Merge pull request #139 from CJSCommonPlatform/fix-builder-optionals

Fix builder optional arguments

CHANGELOG.md

@@ -5,6 +5,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Changed
+- Builders field set method of Optional take argument of actual type rather than Optional of type
+- Builders construct object with Optional.empty() if no value set for field
+
 ## [1.7.3] - 2019-00-19
 ### Changed
 - Upgraded framework-api to version 4.1.0

pojo-generation-core/src/main/java/uk/gov/justice/generation/pojo/plugin/classmodifying/builder/BuilderFieldFactory.java

@@ -1,7 +1,6 @@
 package uk.gov.justice.generation.pojo.plugin.classmodifying.builder;
 
 import static java.util.stream.Collectors.toList;
-import static javax.lang.model.element.Modifier.PRIVATE;
 
 import uk.gov.justice.generation.pojo.dom.Definition;
 import uk.gov.justice.generation.pojo.generators.ClassNameFactory;
@@ -10,29 +9,29 @@
 import java.util.List;
 
 import com.squareup.javapoet.FieldSpec;
-import com.squareup.javapoet.TypeName;
 
 /**
  * Factory for creating the fields in the Builder class
  */
 public class BuilderFieldFactory {
 
+    private final FieldSpecFactory fieldSpecFactory;
+
+    public BuilderFieldFactory(final FieldSpecFactory fieldSpecFactory) {
+        this.fieldSpecFactory = fieldSpecFactory;
+    }
+
     /**
-     * 
      * @param fieldDefinitions The list of {@link Definition}s from which to create fields
      * @param classNameFactory The factory for creating the class name
-     *
      * @return A list of {@link FieldSpec}s of the fields of the Builder
      */
-    public List<FieldSpec> createFields(
-            final List<Definition> fieldDefinitions,
-            final ClassNameFactory classNameFactory,
-            final PluginContext pluginContext) {
+    public List<FieldSpec> createFields(final List<Definition> fieldDefinitions,
+                                        final ClassNameFactory classNameFactory,
+                                        final PluginContext pluginContext) {
+
         return fieldDefinitions.stream()
-                .map(fieldDefinition -> {
-                    final TypeName typeName = classNameFactory.createTypeNameFrom(fieldDefinition, pluginContext);
-                    return FieldSpec.builder(typeName, fieldDefinition.getFieldName(), PRIVATE).build();
-                })
+                .map(fieldDefinition -> fieldSpecFactory.createFieldSpecFor(fieldDefinition, classNameFactory, pluginContext))
                 .collect(toList());
     }
 }

pojo-generation-core/src/main/java/uk/gov/justice/generation/pojo/plugin/classmodifying/builder/BuilderGenerator.java

@@ -153,6 +153,6 @@ private MethodSpec buildMethod(final ClassName pojoClassName, final List<Definit
             return builderMethodFactory.createTheBuildMethodWithAdditionalProperties(fieldDefinitions, pojoClassName);
         }
 
-        return builderMethodFactory.createTheBuildMethod(fieldDefinitions, pojoClassName);
+        return builderMethodFactory.createTheBuildMethod(fieldDefinitions, pojoClassName, pluginContext);
     }
 }

pojo-generation-core/src/main/java/uk/gov/justice/generation/pojo/plugin/classmodifying/builder/BuilderGeneratorFactory.java

@@ -13,21 +13,22 @@ public class BuilderGeneratorFactory {
     /**
      * Creates a {@link BuilderGenerator}
      *
-     * @param classDefinition The {@link ClassDefinition} of the outer POJO which will contain
-     *                        the Builder as a static inner class
+     * @param classDefinition  The {@link ClassDefinition} of the outer POJO which will contain the
+     *                         Builder as a static inner class
      * @param classNameFactory A factory for creating the field and method class names
-     *
      * @return a newly constructed {@link BuilderGenerator}
      */
     public BuilderGenerator create(final ClassDefinition classDefinition,
                                    final ClassNameFactory classNameFactory,
                                    final PluginContext pluginContext) {
 
+        final OptionalTypeNameUtil optionalTypeNameUtil = new OptionalTypeNameUtil();
+
         return new BuilderGenerator(
                 classDefinition,
                 classNameFactory,
-                new BuilderFieldFactory(),
-                new BuilderMethodFactory(),
+                new BuilderFieldFactory(new FieldSpecFactory(optionalTypeNameUtil)),
+                new BuilderMethodFactory(classNameFactory, optionalTypeNameUtil),
                 new AdditionalPropertiesDeterminer(),
                 pluginContext
         );

pojo-generation-core/src/main/java/uk/gov/justice/generation/pojo/plugin/classmodifying/builder/BuilderMethodFactory.java

@@ -17,13 +17,23 @@
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.CodeBlock;
 import com.squareup.javapoet.MethodSpec;
+import com.squareup.javapoet.ParameterizedTypeName;
 import com.squareup.javapoet.TypeName;
 
 /**
  * Factory for creating the 'with' and 'build' methods of the POJO's static inner Builder
  */
 public class BuilderMethodFactory {
 
+    private final ClassNameFactory classNameFactory;
+    private final OptionalTypeNameUtil optionalTypeNameUtil;
+
+    public BuilderMethodFactory(final ClassNameFactory classNameFactory,
+                                final OptionalTypeNameUtil optionalTypeNameUtil) {
+        this.classNameFactory = classNameFactory;
+        this.optionalTypeNameUtil = optionalTypeNameUtil;
+    }
+
     public List<MethodSpec> createTheWithMethods(
             final List<Definition> fieldDefinitions,
             final ClassNameFactory classNameFactory,
@@ -60,15 +70,16 @@ public List<MethodSpec> createTheWithMethodsWithAdditionalProperties(
 
     public MethodSpec createTheBuildMethod(
             final List<Definition> fieldDefinitions,
-            final ClassName pojoClassName) {
+            final ClassName pojoClassName,
+            final PluginContext pluginContext) {
 
-        final String params = fieldDefinitions.stream()
-                .map(Definition::getFieldName)
+        final String constructorArguments = fieldDefinitions.stream()
+                .map(definition -> createConstructorArgumentFor(definition, pluginContext))
                 .collect(joining(", "));
 
         return MethodSpec.methodBuilder("build")
                 .addModifiers(PUBLIC)
-                .addStatement("return new $L(" + params + ")", pojoClassName)
+                .addStatement("return new $L(" + constructorArguments + ")", pojoClassName)
                 .returns(pojoClassName)
                 .build();
     }
@@ -90,6 +101,16 @@ public MethodSpec createTheBuildMethodWithAdditionalProperties(
                 .build();
     }
 
+    private String createConstructorArgumentFor(final Definition definition, final PluginContext pluginContext) {
+        final TypeName typeName = classNameFactory.createTypeNameFrom(definition, pluginContext);
+
+        if (optionalTypeNameUtil.isOptionalType(typeName)) {
+            return "Optional.ofNullable(" + definition.getFieldName() + ")";
+        }
+
+        return definition.getFieldName();
+    }
+
     private String createStatementFormatWith(final String params) {
         if (params.isEmpty()) {
             return "return new $L($N)";
@@ -98,14 +119,13 @@ private String createStatementFormatWith(final String params) {
         return "return new $L(" + params + ", $N)";
     }
 
-    private MethodSpec generateWithMethod(
-            final Definition fieldDefinition,
-            final ClassName builderClassName,
-            final ClassNameFactory classNameFactory,
-            final PluginContext pluginContext) {
+    private MethodSpec generateWithMethod(final Definition fieldDefinition,
+                                          final ClassName builderClassName,
+                                          final ClassNameFactory classNameFactory,
+                                          final PluginContext pluginContext) {
 
         final String fieldName = fieldDefinition.getFieldName();
-        final TypeName typeName = classNameFactory.createTypeNameFrom(fieldDefinition, pluginContext);
+        final TypeName typeName = getParameterTypeName(fieldDefinition, classNameFactory, pluginContext);
 
         return methodBuilder("with" + capitalize(fieldName))
                 .addModifiers(PUBLIC)
@@ -118,6 +138,19 @@ private MethodSpec generateWithMethod(
                 .build();
     }
 
+    private TypeName getParameterTypeName(final Definition fieldDefinition,
+                                          final ClassNameFactory classNameFactory,
+                                          final PluginContext pluginContext) {
+
+        final TypeName typeName = classNameFactory.createTypeNameFrom(fieldDefinition, pluginContext);
+
+        if (optionalTypeNameUtil.isOptionalType(typeName)) {
+            return optionalTypeNameUtil.getOptionalTypeFrom((ParameterizedTypeName) typeName);
+        }
+
+        return typeName;
+    }
+
     private MethodSpec generateWithMethodForAdditionalProperties(final ClassName builderClassName) {
 
         return methodBuilder("withAdditionalProperty")

pojo-generation-core/src/main/java/uk/gov/justice/generation/pojo/plugin/classmodifying/builder/FieldSpecFactory.java

@@ -0,0 +1,38 @@
+package uk.gov.justice.generation.pojo.plugin.classmodifying.builder;
+
+import static javax.lang.model.element.Modifier.PRIVATE;
+
+import uk.gov.justice.generation.pojo.dom.Definition;
+import uk.gov.justice.generation.pojo.generators.ClassNameFactory;
+import uk.gov.justice.generation.pojo.plugin.PluginContext;
+
+import com.squareup.javapoet.FieldSpec;
+import com.squareup.javapoet.ParameterizedTypeName;
+import com.squareup.javapoet.TypeName;
+
+public class FieldSpecFactory {
+
+    private final OptionalTypeNameUtil optionalTypeNameUtil;
+
+    public FieldSpecFactory(final OptionalTypeNameUtil optionalTypeNameUtil) {
+        this.optionalTypeNameUtil = optionalTypeNameUtil;
+    }
+
+    public FieldSpec createFieldSpecFor(final Definition fieldDefinition,
+                                        final ClassNameFactory classNameFactory,
+                                        final PluginContext pluginContext) {
+
+        final TypeName typeName = classNameFactory.createTypeNameFrom(fieldDefinition, pluginContext);
+
+        if (optionalTypeNameUtil.isOptionalType(typeName)) {
+            final TypeName parameterizedType = optionalTypeNameUtil.getOptionalTypeFrom((ParameterizedTypeName) typeName);
+            return FieldSpec
+                    .builder(parameterizedType, fieldDefinition.getFieldName(), PRIVATE)
+                    .build();
+        }
+
+        return FieldSpec
+                .builder(typeName, fieldDefinition.getFieldName(), PRIVATE)
+                .build();
+    }
+}

pojo-generation-core/src/main/java/uk/gov/justice/generation/pojo/plugin/classmodifying/builder/OptionalTypeNameUtil.java

@@ -0,0 +1,19 @@
+package uk.gov.justice.generation.pojo.plugin.classmodifying.builder;
+
+import java.util.Objects;
+import java.util.Optional;
+
+import com.squareup.javapoet.ParameterizedTypeName;
+import com.squareup.javapoet.TypeName;
+
+public class OptionalTypeNameUtil {
+
+    public TypeName getOptionalTypeFrom(final ParameterizedTypeName typeName) {
+        return typeName.typeArguments.get(0);
+    }
+
+    public boolean isOptionalType(final TypeName typeName) {
+        return typeName instanceof ParameterizedTypeName
+                && Objects.equals(((ParameterizedTypeName) typeName).rawType.simpleName(), Optional.class.getSimpleName());
+    }
+}

pojo-generation-core/src/test/java/uk/gov/justice/generation/pojo/plugin/classmodifying/builder/BuilderFieldFactoryTest.java

@@ -1,9 +1,6 @@
 package uk.gov.justice.generation.pojo.plugin.classmodifying.builder;
 
-import static com.squareup.javapoet.TypeName.get;
 import static java.util.Arrays.asList;
-import static javax.lang.model.element.Modifier.PRIVATE;
-import static org.hamcrest.CoreMatchers.hasItem;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
@@ -15,15 +12,21 @@
 
 import java.util.List;
 
+import javax.lang.model.element.Modifier;
+
 import com.squareup.javapoet.FieldSpec;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
+import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 
 @RunWith(MockitoJUnitRunner.class)
 public class BuilderFieldFactoryTest {
 
+    @Mock
+    private FieldSpecFactory fieldSpecFactory;
+
     @InjectMocks
     private BuilderFieldFactory builderFieldFactory;
 
@@ -32,32 +35,29 @@ public void shouldGenerateThePrivateFieldsForOurBuilder() throws Exception {
 
         final Definition fieldDefinition_1 = mock(Definition.class);
         final Definition fieldDefinition_2 = mock(Definition.class);
-        final List<Definition> fieldDefinitions = asList(fieldDefinition_1, fieldDefinition_2);
+        final Definition fieldDefinition_3 = mock(Definition.class);
+        final List<Definition> fieldDefinitions = asList(fieldDefinition_1, fieldDefinition_2, fieldDefinition_3);
 
         final ClassNameFactory classNameFactory = mock(ClassNameFactory.class);
         final PluginContext pluginContext = mock(PluginContext.class);
 
-        when(fieldDefinition_1.getFieldName()).thenReturn("fieldDefinition_1");
-        when(fieldDefinition_2.getFieldName()).thenReturn("fieldDefinition_2");
+        final FieldSpec fieldSpec_1 = FieldSpec.builder(String.class, "field_1", Modifier.PUBLIC).build();
+        final FieldSpec fieldSpec_2 = FieldSpec.builder(String.class, "field_2", Modifier.PUBLIC).build();
+        final FieldSpec fieldSpec_3 = FieldSpec.builder(String.class, "field_3", Modifier.PUBLIC).build();
 
-        when(classNameFactory.createTypeNameFrom(fieldDefinition_1, pluginContext)).thenReturn(get(String.class));
-        when(classNameFactory.createTypeNameFrom(fieldDefinition_2, pluginContext)).thenReturn(get(Integer.class));
+        when(fieldSpecFactory.createFieldSpecFor(fieldDefinition_1, classNameFactory, pluginContext)).thenReturn(fieldSpec_1);
+        when(fieldSpecFactory.createFieldSpecFor(fieldDefinition_2, classNameFactory, pluginContext)).thenReturn(fieldSpec_2);
+        when(fieldSpecFactory.createFieldSpecFor(fieldDefinition_3, classNameFactory, pluginContext)).thenReturn(fieldSpec_3);
 
         final List<FieldSpec> fields = builderFieldFactory.createFields(
                 fieldDefinitions,
                 classNameFactory,
                 pluginContext);
 
-        assertThat(fields.size(), is(2));
-
-        assertThat(fields.get(0).name, is("fieldDefinition_1"));
-        assertThat(fields.get(0).modifiers.size(), is(1));
-        assertThat(fields.get(0).modifiers, hasItem(PRIVATE));
-        assertThat(fields.get(0).type.toString(), is("java.lang.String"));
+        assertThat(fields.size(), is(3));
 
-        assertThat(fields.get(1).name, is("fieldDefinition_2"));
-        assertThat(fields.get(1).modifiers.size(), is(1));
-        assertThat(fields.get(1).modifiers, hasItem(PRIVATE));
-        assertThat(fields.get(1).type.toString(), is("java.lang.Integer"));
+        assertThat(fields.get(0), is(fieldSpec_1));
+        assertThat(fields.get(1), is(fieldSpec_2));
+        assertThat(fields.get(2), is(fieldSpec_3));
     }
 }

pojo-generation-core/src/test/java/uk/gov/justice/generation/pojo/plugin/classmodifying/builder/BuilderGeneratorTest.java

@@ -86,7 +86,7 @@ public void shouldGenerateABuilderWithTheCorrectFieldsBuildMethodAndWithMethods(
 
 
         when(additionalPropertiesDeterminer.shouldAddAdditionalProperties(classDefinition, pluginContext)).thenReturn(false);
-        when(builderMethodFactory.createTheBuildMethod(fieldDefinitions, pojoClassName)).thenReturn(buildMethod);
+        when(builderMethodFactory.createTheBuildMethod(fieldDefinitions, pojoClassName, pluginContext)).thenReturn(buildMethod);
 
         final TypeSpec builder = builderGenerator.generate();