Fixed errors in mono-to-poly and poly-to-mono mapping

holgerknoche · Mar 21, 2024 · 6fad2e8 · 6fad2e8
1 parent 9cb694c
commit 6fad2e8
Show file tree

Hide file tree

Showing 14 changed files with 250 additions and 13 deletions.
diff --git a/...pievolution-core/src/main/java/gutta/apievolution/core/apimodel/provider/ModelMerger.java b/...pievolution-core/src/main/java/gutta/apievolution/core/apimodel/provider/ModelMerger.java
@@ -192,8 +192,16 @@ private void assertUniqueInternalName(UserDefinedType<ProviderApiDefinition> typ
             this.knownTypeNames.add(type.getInternalName());
         }
 
-        private ProviderRecordType convertRecordType(ProviderRecordType inType) {
-            Abstract abstractness = (inType.isAbstract()) ? Abstract.YES : Abstract.NO;
+        private ProviderRecordType convertRecordType(ProviderRecordType inType) {            
+            Abstract abstractness;
+
+            // The type is only abstract if it is abstract in all revisions
+            if (inType.isAbstract()) {
+                 Optional<ProviderRecordType> concretePredecessor = inType.findFirstPredecessorMatching(ProviderRecordType::isConcrete);
+                 abstractness = (concretePredecessor.isPresent()) ? Abstract.NO : Abstract.YES;
+            } else {
+                abstractness = Abstract.NO;
+            }                        
 
             if (inType.isException()) {
                 return this.mergedDefinition.newExceptionType(inType.getPublicName(), inType.getInternalName(), inType.getTypeId(), abstractness,

diff --git a/...olution-core/src/test/java/gutta/apievolution/core/apimodel/provider/ModelMergerTest.java b/...olution-core/src/test/java/gutta/apievolution/core/apimodel/provider/ModelMergerTest.java
@@ -713,8 +713,59 @@ void typeChangeOfOperation() {
                 "}\n";
 
         String actual = new ProviderApiDefinitionPrinter().printApiDefinition(mergedDefinition);
-        assertEquals(expected, actual);
+        assertEquals(expected, actual);       
+    }
+
+    /**
+     * Test case: The abstractness of types in the merged model is handled as expected.
+     */
+    @Test
+    void abstractnessOfTypes() {
+        // Revision 1
+        ProviderApiDefinition revision1 = ProviderApiDefinition.create("test", 0);
+
+        // Define one abstract and two concrete types
+        ProviderRecordType recordType1V1 = revision1.newRecordType("RecordType1", 0);
+        ProviderRecordType recordType2V1 = revision1.newRecordType("RecordType2", noInternalName(), 1, Abstract.YES, noSuperTypes(), noPredecessor());
+        ProviderRecordType recordType3V1 = revision1.newRecordType("RecordType3", 2);
+
+        // Dummy operations to avoid warnings
+        ProviderOperation op1V1 = revision1.newOperation("operation", recordType1V1, recordType2V1);
+        ProviderOperation op2V1 = revision1.newOperation("operation2", recordType3V1, recordType3V1);
+
+        revision1.finalizeDefinition();
+
+        // Revision 2
+        ProviderApiDefinition revision2 = ProviderApiDefinition.create("test", 1);
+
+        // In this revision, one of the concrete types becomes abstract
+        ProviderRecordType recordType1V2 = revision2.newRecordType("RecordType1", noInternalName(), 0, Abstract.YES, noSuperTypes(), recordType1V1);
+        ProviderRecordType recordType2V2 = revision2.newRecordType("RecordType2", noInternalName(), 1, Abstract.YES, noSuperTypes(), recordType2V1);
+        ProviderRecordType recordType3V2 = revision2.newRecordType("RecordType3", noInternalName(), 2, recordType3V1);
 
+        // Dummy operations to avoid warnings
+        revision2.newOperation("operation", noInternalName(), recordType1V2, recordType2V2, op1V1);
+        revision2.newOperation("operation2", noInternalName(), recordType3V2, recordType3V2, op2V1);
+
+        revision2.finalizeDefinition();
+
+        // Create and merge the revision history
+        RevisionHistory revisionHistory = new RevisionHistory(revision1, revision2);
+        ProviderApiDefinition mergedDefinition = new ModelMerger().createMergedDefinition(revisionHistory);
+
+        String expected = "api test [] {\n" + 
+                " record RecordType1(RecordType1) {\n" + 
+                " }\n" + 
+                " abstract record RecordType2(RecordType2) {\n" + 
+                " }\n" + 
+                " record RecordType3(RecordType3) {\n" + 
+                " }\n" + 
+                " operation operation(operation) (RecordType2@revision 0) : RecordType1@revision 0\n" + 
+                " operation operation2(operation2) (RecordType3@revision 0) : RecordType3@revision 0\n" + 
+                "}\n";
+
+        String actual = new ProviderApiDefinitionPrinter().printApiDefinition(mergedDefinition);
+        assertEquals(expected, actual);
     }
 
 }
diff --git a/...at/src/main/java/gutta/apievolution/fixedformat/apimapping/ApiMappingScriptGenerator.java b/...at/src/main/java/gutta/apievolution/fixedformat/apimapping/ApiMappingScriptGenerator.java
@@ -471,12 +471,16 @@ private ApiMappingOperation createMonoToPolyRecordMappingOperation(RecordType<?,
         }
 
         private ApiMappingOperation createPolyToMonoRecordMappingOperation(RecordType<?, ?, ?> recordType) {
-            RecordType<?, ?, ?> sourceType = this.mappingInfoProvider.toSourceType(recordType);
-
-            int sourceTypeId = sourceType.getTypeId();
             RecordTypeEntry typeEntry = this.resolveTypeEntryFor(recordType);
 
-            return new PolyToMonoRecordMappingOperation(sourceTypeId, typeEntry);
+            // The type itself and all its subtypes are mappable 
+            RecordType<?, ?, ?> sourceType = this.mappingInfoProvider.toSourceType(recordType);
+            Set<RecordType<?, ?, ?>> possibleSourceTypes = collectAllConcreteSubtypes(sourceType);
+            Set<Integer> mappableTypeIds = possibleSourceTypes.stream()
+                .map(RecordType::getTypeId)
+                .collect(Collectors.toSet());
+
+            return new PolyToMonoRecordMappingOperation(mappableTypeIds, typeEntry);
         }
 
         private ApiMappingOperation createPolymorphicRecordMappingOperation(RecordType<?, ?, ?> recordType) {

diff --git a/...main/java/gutta/apievolution/fixedformat/apimapping/PolyToMonoRecordMappingOperation.java b/...main/java/gutta/apievolution/fixedformat/apimapping/PolyToMonoRecordMappingOperation.java
@@ -3,15 +3,16 @@
 import gutta.apievolution.fixedformat.objectmapping.Flags;
 
 import java.nio.ByteBuffer;
+import java.util.Set;
 
 class PolyToMonoRecordMappingOperation extends AbstractPolymorphicRecordMappingOperation {
 
-    private final int expectedTypeId;
+    private final Set<Integer> mappableTypeIds;
 
     private final MonomorphicRecordMappingOperation delegate;
 
-    public PolyToMonoRecordMappingOperation(int sourceTypeId, RecordTypeEntry targetTypeEntry) {
-        this.expectedTypeId = sourceTypeId;
+    public PolyToMonoRecordMappingOperation(Set<Integer> mappableTypeIds, RecordTypeEntry targetTypeEntry) {
+        this.mappableTypeIds = mappableTypeIds;
         this.delegate = new MonomorphicRecordMappingOperation(targetTypeEntry);
     }
 
@@ -28,7 +29,7 @@ protected boolean mayBeUnrepresentable() {
     @Override
     protected void mapNonNullValue(ByteBuffer source, ByteBuffer target) {
         int sourceTypeId = source.getInt();
-        if (sourceTypeId != this.expectedTypeId) {
+        if (!this.mappableTypeIds.contains(sourceTypeId)) {
             // If the actual type id does not match the expected one, the value is unrepresentable
             target.put(Flags.IS_UNREPRESENTABLE);
             this.writeNulls(target);

diff --git a/...tion-fixedformat/src/test/java/gutta/apievolution/fixedformat/FixedFormatMappingTest.java b/...tion-fixedformat/src/test/java/gutta/apievolution/fixedformat/FixedFormatMappingTest.java
@@ -13,6 +13,7 @@
 import gutta.apievolution.fixedformat.apimapping.consumer.ConsumerOperationProxy;
 import gutta.apievolution.fixedformat.apimapping.provider.ProviderOperationProxy;
 import gutta.apievolution.fixedformat.consumer.ConsumerEnum;
+import gutta.apievolution.fixedformat.consumer.ConsumerMonoToPolyType;
 import gutta.apievolution.fixedformat.consumer.ConsumerParameter;
 import gutta.apievolution.fixedformat.consumer.ConsumerResult;
 import gutta.apievolution.fixedformat.consumer.ConsumerStructureWithPolyField;
@@ -24,6 +25,8 @@
 import gutta.apievolution.fixedformat.objectmapping.FixedFormatMapper;
 import gutta.apievolution.fixedformat.objectmapping.UnrepresentableValueException;
 import gutta.apievolution.fixedformat.provider.MappableProviderTestException;
+import gutta.apievolution.fixedformat.provider.ProviderMonoToPolySubTypeA;
+import gutta.apievolution.fixedformat.provider.ProviderMonoToPolyType;
 import gutta.apievolution.fixedformat.provider.ProviderParameter;
 import gutta.apievolution.fixedformat.provider.ProviderResult;
 import gutta.apievolution.fixedformat.provider.ProviderStructureWithPolyField;
@@ -150,7 +153,35 @@ void exceptionMapping() {
         assertEquals(1234, thrownException.getExceptionField());
     }
 
-    // TODO Test case for monomorphic to polymorphic conversion and vice versa
+    /**
+     * Test case: Mono-to-poly mapping (parameter) and vice versa (result).
+     */
+    @Test
+    void monoToPolyTypeMapping() {
+        ApiMappingScriptGenerator scriptGenerator = new ApiMappingScriptGenerator();
+        ApiMappingScript consumerToProviderScript = scriptGenerator.generateMappingScript(DEFINITION_RESOLUTION, MappingDirection.CONSUMER_TO_PROVIDER);
+        ApiMappingScript providerToConsumerScript = scriptGenerator.generateMappingScript(DEFINITION_RESOLUTION, MappingDirection.PROVIDER_TO_CONSUMER);
+
+        FixedFormatMapper mapper = new FixedFormatMapper();
+
+        MonoToPolyMappingProviderProxy providerProxy = new MonoToPolyMappingProviderProxy(consumerToProviderScript, providerToConsumerScript, mapper);
+        RequestRouter requestRouter = new RequestRouter(providerProxy);
+
+        MonoToPolyMappingConsumerProxy consumerProxy = new MonoToPolyMappingConsumerProxy(requestRouter, mapper);
+
+        // First variant: Returns the matching type
+        ConsumerMonoToPolyType parameter = new ConsumerMonoToPolyType();
+        parameter.setField1(3456);        
+        ConsumerMonoToPolyType result = consumerProxy.invoke(parameter);        
+        assertEquals(1234, result.getField1());
+
+        // Second variant: A subtype is returned, which makes no difference
+        ConsumerMonoToPolyType parameter2 = new ConsumerMonoToPolyType();
+        // Set the magic value
+        parameter2.setField1(1);        
+        ConsumerMonoToPolyType result2 = consumerProxy.invoke(parameter2);        
+        assertEquals(5678, result2.getField1());
+    }
 
     /**
      * Test case: The provider throws an exception, but the consumer does not expect one. This results in an unrepresentable value.
@@ -286,5 +317,36 @@ protected ProviderResult invokeOperation(ProviderParameter parameter) {
         }
 
     }
+
+    private static class MonoToPolyMappingConsumerProxy extends ConsumerOperationProxy<ConsumerMonoToPolyType, ConsumerMonoToPolyType> {
+
+        public MonoToPolyMappingConsumerProxy(RequestRouter router, FixedFormatMapper mapper) {
+            super("monoToPolyMapping", ConsumerMonoToPolyType.class, ConsumerMonoToPolyType.class, Collections.emptySet(), router, mapper, CHARSET);
+        }
+
+    }
+
+    private static class MonoToPolyMappingProviderProxy extends ProviderOperationProxy<ProviderMonoToPolyType, ProviderMonoToPolyType> {
+
+        public MonoToPolyMappingProviderProxy(ApiMappingScript consumerToProviderScript, ApiMappingScript providerToConsumerScript, FixedFormatMapper mapper) {
+            super("monoToPolyMapping", ProviderMonoToPolyType.class, ProviderMonoToPolyType.class, Collections.emptySet(), consumerToProviderScript, providerToConsumerScript, mapper, CHARSET);
+        }
+
+        @Override
+        protected ProviderMonoToPolyType invokeOperation(ProviderMonoToPolyType parameter) {
+            if (parameter.getField1() == 1) {
+                ProviderMonoToPolySubTypeA result = new ProviderMonoToPolySubTypeA();
+                result.setField1(5678);
+                result.setField2(4321);
+
+                return result;                
+            } else {
+                ProviderMonoToPolyType result = new ProviderMonoToPolyType();
+                result.setField1(1234);
+
+                return result;
+            }
+        }        
+    }
 
 }
diff --git a/...dformat/src/test/java/gutta/apievolution/fixedformat/consumer/ConsumerMonoToPolyType.java b/...dformat/src/test/java/gutta/apievolution/fixedformat/consumer/ConsumerMonoToPolyType.java
@@ -0,0 +1,15 @@
+package gutta.apievolution.fixedformat.consumer;
+
+public class ConsumerMonoToPolyType {
+
+    private Integer field1;
+
+    public Integer getField1() {
+        return this.field1;
+    }
+
+    public void setField1(Integer field1) {
+        this.field1 = field1;
+    }
+
+}
diff --git a/...mat/src/test/java/gutta/apievolution/fixedformat/provider/ProviderMonoToPolySubTypeA.java b/...mat/src/test/java/gutta/apievolution/fixedformat/provider/ProviderMonoToPolySubTypeA.java
@@ -0,0 +1,18 @@
+package gutta.apievolution.fixedformat.provider;
+
+import gutta.apievolution.fixedformat.objectmapping.TypeId;
+
+@TypeId(4)
+public class ProviderMonoToPolySubTypeA extends ProviderMonoToPolyType {
+
+    private Integer field2;
+
+    public Integer getField2() {
+        return this.field2;
+    }
+
+    public void setField2(Integer field2) {
+        this.field2 = field2;
+    }
+
+}
diff --git a/...mat/src/test/java/gutta/apievolution/fixedformat/provider/ProviderMonoToPolySubTypeB.java b/...mat/src/test/java/gutta/apievolution/fixedformat/provider/ProviderMonoToPolySubTypeB.java
@@ -0,0 +1,20 @@
+package gutta.apievolution.fixedformat.provider;
+
+import gutta.apievolution.fixedformat.objectmapping.MaxLength;
+import gutta.apievolution.fixedformat.objectmapping.TypeId;
+
+@TypeId(5)
+public class ProviderMonoToPolySubTypeB extends ProviderMonoToPolyType {
+
+    @MaxLength(10)
+    private String field3;
+
+    public String getField3() {
+        return this.field3;
+    }
+
+    public void setField3(String field3) {
+        this.field3 = field3;
+    }
+
+}
diff --git a/...dformat/src/test/java/gutta/apievolution/fixedformat/provider/ProviderMonoToPolyType.java b/...dformat/src/test/java/gutta/apievolution/fixedformat/provider/ProviderMonoToPolyType.java
@@ -0,0 +1,20 @@
+package gutta.apievolution.fixedformat.provider;
+
+import gutta.apievolution.fixedformat.objectmapping.SubTypes;
+import gutta.apievolution.fixedformat.objectmapping.TypeId;
+
+@TypeId(3)
+@SubTypes({ProviderMonoToPolySubTypeA.class, ProviderMonoToPolySubTypeB.class})
+public class ProviderMonoToPolyType {
+
+    private Integer field1;
+
+    public Integer getField1() {
+        return this.field1;
+    }
+
+    public void setField1(Integer field1) {
+        this.field1 = field1;
+    }
+
+}
diff --git a/gutta-apievolution-fixedformat/src/test/resources/apis/consumer-api.api b/gutta-apievolution-fixedformat/src/test/resources/apis/consumer-api.api
@@ -36,6 +36,10 @@ api test.customer {
         int32 exceptionField
     }
 
+    record MonoToPolyType as ConsumerMonoToPolyType {
+        int32 field1
+    }
+
     operation testOperation(TestParameter): TestResult
 
     operation polyOperation(SuperType): SuperType
@@ -45,5 +49,7 @@ api test.customer {
     operation opWithException(TestParameter): TestResult throws TestException
 
     operation opWithUnmappedException(TestParameter): TestResult
+
+    operation monoToPolyMapping(MonoToPolyType): MonoToPolyType
 
 }
diff --git a/gutta-apievolution-fixedformat/src/test/resources/apis/provider-revision-1.api b/gutta-apievolution-fixedformat/src/test/resources/apis/provider-revision-1.api
@@ -35,7 +35,11 @@ api test.provider {
 
     exception TestException as ProviderTestException {
         int32 exceptionField
-    }    
+    }
+
+    record MonoToPolyType as ProviderMonoToPolyType {
+        int32 field1
+    }
 
     operation testOperation(TestParameter): TestResult
 
@@ -46,5 +50,7 @@ api test.provider {
     operation opWithException(TestParameter): TestResult throws TestException
 
     operation opWithUnmappedException(TestParameter): TestResult throws TestException
+
+    operation monoToPolyMapping(MonoToPolyType): MonoToPolyType
 
 }
diff --git a/gutta-apievolution-fixedformat/src/test/resources/apis/provider-revision-2.api b/gutta-apievolution-fixedformat/src/test/resources/apis/provider-revision-2.api
@@ -17,6 +17,20 @@ api test.provider {
         TestEnum[10] resultList
     }
 
+    abstract record MonoToPolyType as ProviderMonoToPolyType {
+        int32 field1
+    }
+
+    record MonoToPolySubTypeA extends MonoToPolyType as ProviderMonoToPolySubTypeA {
+        int32 field2
+    }
+
+    record MonoToPolySubTypeB extends MonoToPolyType as ProviderMonoToPolySubTypeB {
+        string(10) field3
+    }
+
     operation testOperation(TestParameter): TestResult
+
+    operation monoToPolyMapping(MonoToPolyType): MonoToPolyType
 
 }
diff --git a/gutta-apievolution-json/src/test/resources/apis/consumer-api.api b/gutta-apievolution-json/src/test/resources/apis/consumer-api.api
@@ -32,10 +32,16 @@ api test.customer {
     	int32 fieldB
     }
 
+    exception TestException as ConsumerTestException {
+        int32 exceptionField
+    }    
+
     operation testOperation(TestParameter): TestResult
 
     operation polyOperation(SuperType): SuperType
 
     operation polyOperation2(StructureWithPolyField): StructureWithPolyField
+
+    operation opWithException(TestParameter): TestResult throws TestException
 
 }
diff --git a/gutta-apievolution-json/src/test/resources/apis/provider-revision-1.api b/gutta-apievolution-json/src/test/resources/apis/provider-revision-1.api
@@ -33,10 +33,16 @@ api test.provider {
     	int32 fieldB
     }
 
+    exception TestException as ProviderTestException {
+        int32 exceptionField
+    }
+
     operation testOperation(TestParameter): TestResult
 
     operation polyOperation(SuperType): SuperType
 
     operation polyOperation2(StructureWithPolyField): StructureWithPolyField
+
+    operation opWithException(TestParameter): TestResult throws TestException
 
 }