From c6cf23956fc3d3c116dd68f4d2f692ce4a820ac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20=C5=81abaj?= Date: Mon, 3 Jul 2023 10:46:23 +0200 Subject: [PATCH] Fixes Randgalt/record-builder/#153 (#154) --- pom.xml | 7 ++ .../recordbuilder/core/RecordBuilder.java | 18 +++++ .../processor/CollectionBuilderUtils.java | 67 ++++++++++++++--- .../processor/RecordBuilderProcessor.java | 17 +++++ record-builder-test/pom.xml | 6 ++ .../test/UnmodifiableCollectionsRecord.java | 29 ++++++++ .../TestUnmodifiableCollectionsBuilder.java | 72 +++++++++++++++++++ 7 files changed, 205 insertions(+), 11 deletions(-) create mode 100644 record-builder-test/src/main/java/io/soabase/recordbuilder/test/UnmodifiableCollectionsRecord.java create mode 100644 record-builder-test/src/test/java/io/soabase/recordbuilder/test/TestUnmodifiableCollectionsBuilder.java diff --git a/pom.xml b/pom.xml index 6d5834ae..46face1a 100644 --- a/pom.xml +++ b/pom.xml @@ -58,6 +58,7 @@ 1.12.1 5.5.2 + 3.24.2 7.2 2.0.1.Final 6.0.20.Final @@ -151,6 +152,12 @@ ${junit-jupiter-version} + + org.assertj + assertj-core + ${assertj-core.version} + + javax.validation validation-api diff --git a/record-builder-core/src/main/java/io/soabase/recordbuilder/core/RecordBuilder.java b/record-builder-core/src/main/java/io/soabase/recordbuilder/core/RecordBuilder.java index e5209633..34c6c0a6 100644 --- a/record-builder-core/src/main/java/io/soabase/recordbuilder/core/RecordBuilder.java +++ b/record-builder-core/src/main/java/io/soabase/recordbuilder/core/RecordBuilder.java @@ -183,9 +183,27 @@ * {@link java.util.Map} and {@link java.util.Collection}. When the record is built, any components of these * types are passed through an added shim method that uses the corresponding immutable collection (e.g. * {@code List.copyOf(o)}) or an empty immutable collection if the component is {@code null}. + * + * @see #useUnmodifiableCollections() */ boolean useImmutableCollections() default false; + /** + * Adds special handling for record components of type: {@link java.util.List}, {@link java.util.Set}, + * {@link java.util.Map} and {@link java.util.Collection}. When the record is built, any components of these + * types are passed through an added shim method that uses the corresponding unmodifiable collection (e.g. + * {@code Collections.unmodifiableList(o)}) or an empty immutable collection if the component is {@code null}. + * + *

+ * For backward compatibility, when {@link #useImmutableCollections()} returns {@code true}, this property is + * ignored. + * + * @see #useImmutableCollections() + * + * @since 37 + */ + boolean useUnmodifiableCollections() default false; + /** * When enabled, collection types ({@code List}, {@code Set} and {@code Map}) are handled specially. The setters * for these types now create an internal collection and items are added to that collection. Additionally, diff --git a/record-builder-processor/src/main/java/io/soabase/recordbuilder/processor/CollectionBuilderUtils.java b/record-builder-processor/src/main/java/io/soabase/recordbuilder/processor/CollectionBuilderUtils.java index 775a53ac..5e1ac190 100644 --- a/record-builder-processor/src/main/java/io/soabase/recordbuilder/processor/CollectionBuilderUtils.java +++ b/record-builder-processor/src/main/java/io/soabase/recordbuilder/processor/CollectionBuilderUtils.java @@ -26,6 +26,7 @@ class CollectionBuilderUtils { private final boolean useImmutableCollections; + private final boolean useUnmodifiableCollections; private final boolean addSingleItemCollectionBuilders; private final boolean addClassRetainedGenerated; private final String listShimName; @@ -50,10 +51,12 @@ class CollectionBuilderUtils { private static final Class mapType = Map.class; private static final Class setType = Set.class; private static final Class collectionType = Collection.class; + private static final Class collectionsType = Collections.class; private static final TypeName listTypeName = TypeName.get(listType); private static final TypeName mapTypeName = TypeName.get(mapType); private static final TypeName setTypeName = TypeName.get(setType); private static final TypeName collectionTypeName = TypeName.get(collectionType); + private static final TypeName collectionsTypeName = TypeName.get(collectionsType); private static final TypeVariableName tType = TypeVariableName.get("T"); private static final TypeVariableName kType = TypeVariableName.get("K"); @@ -79,6 +82,7 @@ class CollectionBuilderUtils { CollectionBuilderUtils(List recordComponents, RecordBuilder.Options metaData) { useImmutableCollections = metaData.useImmutableCollections(); + useUnmodifiableCollections = !useImmutableCollections && metaData.useUnmodifiableCollections(); addSingleItemCollectionBuilders = metaData.addSingleItemCollectionBuilders(); addClassRetainedGenerated = metaData.addClassRetainedGenerated(); @@ -154,8 +158,8 @@ yield singleItemsMetaDataWithWildType(parameterizedTypeName, collectionClass, wi } boolean isImmutableCollection(RecordClassType component) { - return useImmutableCollections && (isList(component) || isMap(component) || isSet(component) - || component.rawTypeName().equals(collectionTypeName)); + return (useImmutableCollections || useUnmodifiableCollections) + && (isList(component) || isMap(component) || isSet(component) || isCollection(component)); } boolean isList(RecordClassType component) { @@ -170,8 +174,12 @@ boolean isSet(RecordClassType component) { return component.rawTypeName().equals(setTypeName); } + private boolean isCollection(RecordClassType component) { + return component.rawTypeName().equals(collectionTypeName); + } + void addShimCall(CodeBlock.Builder builder, RecordClassType component) { - if (useImmutableCollections) { + if (useImmutableCollections || useUnmodifiableCollections) { if (isList(component)) { needsListShim = true; needsListMutableMaker = true; @@ -184,7 +192,7 @@ void addShimCall(CodeBlock.Builder builder, RecordClassType component) { needsSetShim = true; needsSetMutableMaker = true; builder.add("$L($L)", setShimName, component.name()); - } else if (component.rawTypeName().equals(collectionTypeName)) { + } else if (isCollection(component)) { needsCollectionShim = true; builder.add("$L($L)", collectionShimName, component.name()); } else { @@ -202,7 +210,7 @@ String shimName(RecordClassType component) { return mapShimName; } else if (isSet(component)) { return setShimName; - } else if (component.rawTypeName().equals(collectionTypeName)) { + } else if (isCollection(component)) { return collectionShimName; } else { throw new IllegalArgumentException(component + " is not a supported collection type"); @@ -222,7 +230,7 @@ String mutableMakerName(RecordClassType component) { } void addShims(TypeSpec.Builder builder) { - if (!useImmutableCollections) { + if (!useImmutableCollections && !useUnmodifiableCollections) { return; } @@ -242,7 +250,7 @@ void addShims(TypeSpec.Builder builder) { } void addMutableMakers(TypeSpec.Builder builder) { - if (!useImmutableCollections) { + if (!useImmutableCollections && !useUnmodifiableCollections) { return; } @@ -298,7 +306,8 @@ private String disambiguateGeneratedMethodName(List recordCompo private MethodSpec buildShimMethod(String name, TypeName mainType, Class abstractType, ParameterizedTypeName parameterizedType, TypeVariableName... typeVariables) { - var code = CodeBlock.of("return (o != null) ? $T.copyOf(o) : $T.of()", mainType, mainType); + var code = buildShimMethodBody(mainType, parameterizedType); + TypeName[] wildCardTypeArguments = parameterizedType.typeArguments.stream().map(WildcardTypeName::subtypeOf) .toList().toArray(new TypeName[0]); var extendedParameterizedType = ParameterizedTypeName.get(ClassName.get(abstractType), wildCardTypeArguments); @@ -307,6 +316,29 @@ private MethodSpec buildShimMethod(String name, TypeName mainType, Class abst .returns(parameterizedType).addParameter(extendedParameterizedType, "o").addStatement(code).build(); } + private CodeBlock buildShimMethodBody(TypeName mainType, ParameterizedTypeName parameterizedType) { + if (!useUnmodifiableCollections) { + return CodeBlock.of("return (o != null) ? $T.copyOf(o) : $T.of()", mainType, mainType); + } + + if (mainType.equals(listTypeName)) { + return CodeBlock.of("return (o != null) ? $T.<$T>unmodifiableList(($T) o) : $T.<$T>emptyList()", + collectionsTypeName, tType, parameterizedType, collectionsTypeName, tType); + } + + if (mainType.equals(setTypeName)) { + return CodeBlock.of("return (o != null) ? $T.<$T>unmodifiableSet(($T) o) : $T.<$T>emptySet()", + collectionsTypeName, tType, parameterizedType, collectionsTypeName, tType); + } + + if (mainType.equals(mapTypeName)) { + return CodeBlock.of("return (o != null) ? $T.<$T, $T>unmodifiableMap(($T) o) : $T.<$T, $T>emptyMap()", + collectionsTypeName, kType, vType, parameterizedType, collectionsTypeName, kType, vType); + } + + throw new IllegalStateException("Cannot build shim method for" + mainType); + } + private MethodSpec buildMutableMakerMethod(String name, String mutableCollectionType, ParameterizedTypeName parameterizedType, TypeVariableName... typeVariables) { var nullCase = CodeBlock.of("if (o == null) return new $L<>()", mutableCollectionType); @@ -340,12 +372,25 @@ private TypeSpec buildMutableCollectionSubType(String className, ClassName mutab } private MethodSpec buildCollectionsShimMethod() { - var code = CodeBlock.builder().add("if (o instanceof Set) {\n").indent() - .addStatement("return $T.copyOf(o)", setTypeName).unindent().addStatement("}") - .addStatement("return (o != null) ? $T.copyOf(o) : $T.of()", listTypeName, listTypeName).build(); + var code = buildCollectionShimMethodBody(); return MethodSpec.methodBuilder(collectionShimName).addAnnotation(generatedRecordBuilderAnnotation) .addModifiers(Modifier.PRIVATE, Modifier.STATIC).addTypeVariable(tType) .returns(parameterizedCollectionType).addParameter(parameterizedCollectionType, "o").addCode(code) .build(); } + + private CodeBlock buildCollectionShimMethodBody() { + if (!useUnmodifiableCollections) { + return CodeBlock.builder().add("if (o instanceof Set) {\n").indent() + .addStatement("return $T.copyOf(o)", setTypeName).unindent().addStatement("}") + .addStatement("return (o != null) ? $T.copyOf(o) : $T.of()", listTypeName, listTypeName).build(); + } + + return CodeBlock.builder().beginControlFlow("if (o instanceof $T)", listType) + .addStatement("return $T.<$T>unmodifiableList(($T) o)", collectionsTypeName, tType, + parameterizedListType) + .endControlFlow().beginControlFlow("if (o instanceof $T)", setType) + .addStatement("return $T.<$T>unmodifiableSet(($T) o)", collectionsTypeName, tType, parameterizedSetType) + .endControlFlow().addStatement("return $T.<$T>emptyList()", collectionsTypeName, tType).build(); + } } diff --git a/record-builder-processor/src/main/java/io/soabase/recordbuilder/processor/RecordBuilderProcessor.java b/record-builder-processor/src/main/java/io/soabase/recordbuilder/processor/RecordBuilderProcessor.java index 322867ce..c3381095 100644 --- a/record-builder-processor/src/main/java/io/soabase/recordbuilder/processor/RecordBuilderProcessor.java +++ b/record-builder-processor/src/main/java/io/soabase/recordbuilder/processor/RecordBuilderProcessor.java @@ -164,6 +164,9 @@ private void processRecordInterface(TypeElement element, boolean addRecordBuilde "RecordInterface only valid for interfaces.", element); return; } + + validateMetaData(metaData, element); + var internalProcessor = new InternalRecordInterfaceProcessor(processingEnv, element, addRecordBuilder, metaData, packageName, fromTemplate); if (!internalProcessor.isValid()) { @@ -184,11 +187,25 @@ private void processRecordBuilder(TypeElement record, RecordBuilder.Options meta record); return; } + + validateMetaData(metaData, record); + var internalProcessor = new InternalRecordBuilderProcessor(processingEnv, record, metaData, packageName); writeRecordBuilderJavaFile(record, internalProcessor.packageName(), internalProcessor.builderClassType(), internalProcessor.builderType(), metaData); } + private void validateMetaData(RecordBuilder.Options metaData, Element record) { + var useImmutableCollections = metaData.useImmutableCollections(); + var useUnmodifiableCollections = metaData.useUnmodifiableCollections(); + + if (useImmutableCollections && useUnmodifiableCollections) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.MANDATORY_WARNING, + "Options.useUnmodifiableCollections property is ignored as Options.useImmutableCollections is set to true", + record); + } + } + private void writeRecordBuilderJavaFile(TypeElement record, String packageName, ClassType builderClassType, TypeSpec builderType, RecordBuilder.Options metaData) { // produces the Java file diff --git a/record-builder-test/pom.xml b/record-builder-test/pom.xml index 9e1907de..90210377 100644 --- a/record-builder-test/pom.xml +++ b/record-builder-test/pom.xml @@ -64,6 +64,12 @@ junit-jupiter test + + + org.assertj + assertj-core + test + diff --git a/record-builder-test/src/main/java/io/soabase/recordbuilder/test/UnmodifiableCollectionsRecord.java b/record-builder-test/src/main/java/io/soabase/recordbuilder/test/UnmodifiableCollectionsRecord.java new file mode 100644 index 00000000..6242938f --- /dev/null +++ b/record-builder-test/src/main/java/io/soabase/recordbuilder/test/UnmodifiableCollectionsRecord.java @@ -0,0 +1,29 @@ +/* + * Copyright 2019 The original author or authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.soabase.recordbuilder.test; + +import io.soabase.recordbuilder.core.RecordBuilder; + +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Set; + +@RecordBuilder +@RecordBuilder.Options(useUnmodifiableCollections = true) +record UnmodifiableCollectionsRecord(List aList, Set orderedSet, Map orderedMap, + Collection aCollection) { +} diff --git a/record-builder-test/src/test/java/io/soabase/recordbuilder/test/TestUnmodifiableCollectionsBuilder.java b/record-builder-test/src/test/java/io/soabase/recordbuilder/test/TestUnmodifiableCollectionsBuilder.java new file mode 100644 index 00000000..93f32967 --- /dev/null +++ b/record-builder-test/src/test/java/io/soabase/recordbuilder/test/TestUnmodifiableCollectionsBuilder.java @@ -0,0 +1,72 @@ +/* + * Copyright 2019 The original author or authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.soabase.recordbuilder.test; + +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; + +import static java.util.Map.entry; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class TestUnmodifiableCollectionsBuilder { + + @Test + void shouldWrapCollectionsWithUnmodifiableView() { + // given + var list = new ArrayList(); + list.add(2); + list.add(1); + list.add(0); + + var orderedSet = new LinkedHashSet(); + orderedSet.add("C"); + orderedSet.add("B"); + orderedSet.add("A"); + + var orderedMap = new LinkedHashMap(); + orderedMap.put("C", 2); + orderedMap.put("B", 1); + orderedMap.put("A", 0); + + var collection = new HashSet(); + collection.add("C"); + collection.add("B"); + collection.add("A"); + + // when + var record = UnmodifiableCollectionsRecordBuilder.builder().aList(list).orderedSet(orderedSet) + .orderedMap(orderedMap).aCollection(collection).build(); + + // then + assertAll(() -> assertThrows(UnsupportedOperationException.class, () -> record.aList().add(9)), + () -> assertThat(record.aList()).containsExactly(2, 1, 0), + () -> assertThrows(UnsupportedOperationException.class, () -> record.orderedSet().add("newElement")), + () -> assertThat(record.orderedSet()).containsExactly("C", "B", "A"), + () -> assertThrows(UnsupportedOperationException.class, () -> record.orderedMap().put("newElement", 9)), + () -> assertThat(record.orderedMap()).containsExactly(entry("C", 2), entry("B", 1), entry("A", 0)), + () -> assertThrows(UnsupportedOperationException.class, () -> record.aCollection().add("newElement")), + () -> assertThat(record.aCollection()).containsExactlyInAnyOrder("C", "B", "A")); + } +}