[Java] DTOs respect the optional package field in the schema definiti…

…on (#1022)

* [Java] DTOs respect the optional package field in the schema definition

* [Java] checkstyle

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/TargetCodeGeneratorLoader.java

@@ -52,20 +52,24 @@ public CodeGenerator newInstance(final Ir ir, final String outputDir)
         {
             final JavaOutputManager outputManager = new JavaOutputManager(outputDir, ir.applicableNamespace());
 
+            final boolean shouldSupportTypesPackageNames = Boolean.getBoolean(TYPES_PACKAGE_OVERRIDE);
             final JavaGenerator codecGenerator = new JavaGenerator(
                 ir,
                 System.getProperty(JAVA_ENCODING_BUFFER_TYPE, JAVA_DEFAULT_ENCODING_BUFFER_TYPE),
                 System.getProperty(JAVA_DECODING_BUFFER_TYPE, JAVA_DEFAULT_DECODING_BUFFER_TYPE),
                 Boolean.getBoolean(JAVA_GROUP_ORDER_ANNOTATION),
                 Boolean.getBoolean(JAVA_GENERATE_INTERFACES),
                 Boolean.getBoolean(DECODE_UNKNOWN_ENUM_VALUES),
-                Boolean.getBoolean(TYPES_PACKAGE_OVERRIDE),
+                shouldSupportTypesPackageNames,
                 precedenceChecks(),
                 outputManager);
 
             if (Boolean.getBoolean(JAVA_GENERATE_DTOS))
             {
-                final JavaDtoGenerator dtoGenerator = new JavaDtoGenerator(ir, outputManager);
+                final JavaDtoGenerator dtoGenerator = new JavaDtoGenerator(
+                    ir,
+                    shouldSupportTypesPackageNames,
+                    outputManager);
                 return () ->
                 {
                     codecGenerator.generate();

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppDtoGenerator.java

@@ -41,7 +41,7 @@
 import static uk.co.real_logic.sbe.ir.GenerationUtil.collectVarData;
 
 /**
- * DTO generator for the CSharp programming language.
+ * DTO generator for the C++ programming language.
  */
 public class CppDtoGenerator implements CodeGenerator
 {
@@ -52,7 +52,7 @@ public class CppDtoGenerator implements CodeGenerator
     private final OutputManager outputManager;
 
     /**
-     * Create a new C# DTO {@link CodeGenerator}.
+     * Create a new C++ DTO {@link CodeGenerator}.
      *
      * @param ir            for the messages and types.
      * @param outputManager for generating the DTOs to.

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaDtoGenerator.java

@@ -24,12 +24,14 @@
 import uk.co.real_logic.sbe.ir.Token;
 import org.agrona.LangUtil;
 import org.agrona.Verify;
-import org.agrona.generation.OutputManager;
+import org.agrona.generation.DynamicPackageOutputManager;
 
 import java.io.IOException;
 import java.io.Writer;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.function.Predicate;
 
 import static uk.co.real_logic.sbe.generation.Generators.toLowerFirstChar;
@@ -49,23 +51,48 @@ public class JavaDtoGenerator implements CodeGenerator
     private static final String BASE_INDENT = "";
 
     private final Ir ir;
-    private final OutputManager outputManager;
+    private final DynamicPackageOutputManager outputManager;
+    private final boolean shouldSupportTypesPackageNames;
+    private final Set<String> packageNameByTypes = new HashSet<>();
 
     /**
-     * Create a new C# DTO {@link CodeGenerator}.
+     * Create a new Java DTO {@link CodeGenerator}.
      *
-     * @param ir            for the messages and types.
-     * @param outputManager for generating the DTOs to.
+     * @param ir                                for the messages and types.
+     * @param shouldSupportTypesPackageNames    generator support for types in their own package.
+     * @param outputManager                     for generating the DTOs to.
      */
-    public JavaDtoGenerator(final Ir ir, final OutputManager outputManager)
+    public JavaDtoGenerator(
+        final Ir ir,
+        final boolean shouldSupportTypesPackageNames,
+        final DynamicPackageOutputManager outputManager)
     {
+        this.shouldSupportTypesPackageNames = shouldSupportTypesPackageNames;
         Verify.notNull(ir, "ir");
         Verify.notNull(outputManager, "outputManager");
 
         this.ir = ir;
         this.outputManager = outputManager;
     }
 
+    /**
+     * Fetch the type's explicit package - if set and should be supported.
+     *
+     * @param token the 0-th token of the type.
+     * @param ir    the intermediate representation.
+     * @return the overridden package name of the type if set and supported, or {@link Ir#applicableNamespace()}.
+     */
+    private String fetchTypesPackageName(final Token token, final Ir ir)
+    {
+        if (shouldSupportTypesPackageNames && token.packageName() != null)
+        {
+            outputManager.setPackageName(token.packageName());
+            return token.packageName();
+        }
+
+        return ir.applicableNamespace();
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -110,9 +137,10 @@ public void generate() throws IOException
             generateDisplay(classBuilder, encoderClassName, "computeEncodedLength()",
                 BASE_INDENT + INDENT);
 
+            final String packageName = fetchTypesPackageName(msgToken, ir);
             try (Writer out = outputManager.createOutput(dtoClassName))
             {
-                out.append(generateDtoFileHeader(ir.applicableNamespace()));
+                out.append(generateDtoFileHeader(packageName));
                 out.append("import org.agrona.DirectBuffer;\n");
                 out.append("import org.agrona.MutableDirectBuffer;\n");
                 out.append("import org.agrona.concurrent.UnsafeBuffer;\n\n");
@@ -1582,6 +1610,19 @@ private static String formatDtoClassName(final String name)
 
     private void generateDtosForTypes() throws IOException
     {
+        if (shouldSupportTypesPackageNames)
+        {
+            for (final List<Token> tokens : ir.types())
+            {
+                final String packageName = tokens.get(0).packageName();
+
+                if (packageName != null)
+                {
+                    packageNameByTypes.add(packageName);
+                }
+            }
+        }
+
         for (final List<Token> tokens : ir.types())
         {
             switch (tokens.get(0).signal())
@@ -1607,10 +1648,11 @@ private void generateComposite(final List<Token> tokens) throws IOException
         final String encoderClassName = encoderName(name);
         final String decoderClassName = decoderName(name);
 
+        final String packageName = fetchTypesPackageName(tokens.get(0), ir);
         try (Writer out = outputManager.createOutput(className))
         {
             final List<Token> compositeTokens = tokens.subList(1, tokens.size() - 1);
-            out.append(generateDtoFileHeader(ir.applicableNamespace()));
+            out.append(generateDtoFileHeader(packageName));
             out.append("import org.agrona.DirectBuffer;\n");
             out.append("import org.agrona.MutableDirectBuffer;\n");
             out.append("import org.agrona.concurrent.UnsafeBuffer;\n\n");
@@ -1638,10 +1680,11 @@ private void generateChoiceSet(final List<Token> tokens) throws IOException
         final String encoderClassName = encoderName(name);
         final String decoderClassName = decoderName(name);
 
+        final String packageName = fetchTypesPackageName(tokens.get(0), ir);
         try (Writer out = outputManager.createOutput(className))
         {
             final List<Token> setTokens = tokens.subList(1, tokens.size() - 1);
-            out.append(generateDtoFileHeader(ir.applicableNamespace()));
+            out.append(generateDtoFileHeader(packageName));
             out.append(generateDocumentation(BASE_INDENT, tokens.get(0)));
 
             final ClassBuilder classBuilder = new ClassBuilder(className, BASE_INDENT, "public final");
@@ -1795,13 +1838,35 @@ private void generateCompositePropertyElements(
         }
     }
 
-    private static CharSequence generateDtoFileHeader(final String packageName)
+    private static StringBuilder generateImportStatements(final Set<String> packages, final String currentPackage)
+    {
+        final StringBuilder importStatements = new StringBuilder();
+
+        for (final String candidatePackage : packages)
+        {
+            if (!candidatePackage.equals(currentPackage))
+            {
+                importStatements.append("import ").append(candidatePackage).append(".*;\n");
+            }
+        }
+
+        if (!importStatements.isEmpty())
+        {
+            importStatements.append("\n\n");
+        }
+
+        return importStatements;
+    }
+
+    private CharSequence generateDtoFileHeader(final String packageName)
     {
         final StringBuilder sb = new StringBuilder();
 
         sb.append("/* Generated SBE (Simple Binary Encoding) message DTO */\n");
         sb.append("package ").append(packageName).append(";\n\n");
 
+        sb.append(generateImportStatements(packageNameByTypes, packageName));
+
         return sb;
     }
 

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaDtos.java

@@ -20,6 +20,8 @@
 import uk.co.real_logic.sbe.generation.TargetCodeGenerator;
 import uk.co.real_logic.sbe.ir.Ir;
 
+import static uk.co.real_logic.sbe.SbeTool.TYPES_PACKAGE_OVERRIDE;
+
 /**
  * {@link CodeGenerator} factory for Java DTOs.
  */
@@ -30,6 +32,9 @@ public class JavaDtos implements TargetCodeGenerator
      */
     public CodeGenerator newInstance(final Ir ir, final String outputDir)
     {
-        return new JavaDtoGenerator(ir, new JavaOutputManager(outputDir, ir.applicableNamespace()));
+        return new JavaDtoGenerator(
+            ir,
+            Boolean.getBoolean(TYPES_PACKAGE_OVERRIDE),
+            new JavaOutputManager(outputDir, ir.applicableNamespace()));
     }
 }

sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java

@@ -87,7 +87,7 @@ void javaDtoEncodeShouldBeTheInverseOfDtoDecode(
                     outputManager)
                     .generate();
 
-                new JavaDtoGenerator(encodedMessage.ir(), outputManager)
+                new JavaDtoGenerator(encodedMessage.ir(), false, outputManager)
                     .generate();
             }
             catch (final Exception generationException)

sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/JavaGeneratorTest.java

@@ -601,6 +601,42 @@ void shouldCreateTypesInSamePackageIfSupportDisabled() throws Exception
         }
     }
 
+    @Test
+    void dtosShouldReferenceTypesInDifferentPackages() throws Exception
+    {
+        try (InputStream in = Tests.getLocalResource("explicit-package-test-schema.xml"))
+        {
+            final ParserOptions options = ParserOptions.builder().stopOnError(true).build();
+            final MessageSchema schema = parse(in, options);
+            final IrGenerator irg = new IrGenerator();
+            ir = irg.generate(schema);
+
+            outputManager.clear();
+            outputManager.setPackageName(ir.applicableNamespace());
+
+            final JavaGenerator generator = new JavaGenerator(
+                ir, BUFFER_NAME, READ_ONLY_BUFFER_NAME, false, false, false, true, outputManager
+            );
+            generator.generate();
+
+            final JavaDtoGenerator javaDtoGenerator = new JavaDtoGenerator(ir, true, outputManager);
+            javaDtoGenerator.generate();
+
+            final Map<String, CharSequence> sources = outputManager.getSources();
+            assertNotNull(sources.get("test.message.schema.TestMessageDto"));
+            assertNotNull(sources.get("test.message.schema.MessageHeaderDto"));
+            assertNotNull(sources.get("test.message.schema.common.CarDto"));
+            assertNotNull(sources.get("test.message.schema.common.EngineDto"));
+            assertNotNull(sources.get("outside.schema.FuelSpecDto"));
+            assertNotNull(sources.get("outside.schema.DaysDto"));
+            assertNotNull(sources.get("outside.schema.FuelTypeDto"));
+
+            assertNotNull(compile("test.message.schema.TestMessageDto"));
+            assertNotNull(compile("test.message.schema.common.CarDto"));
+            assertNotNull(compile("outside.schema.FuelSpecDto"));
+        }
+    }
+
     @ParameterizedTest
     @CsvSource(
         {