From 1fc02cd6e1b27f5fa2049831b31b7227c54c561e Mon Sep 17 00:00:00 2001 From: fh-ms Date: Thu, 30 Apr 2026 14:40:18 +0200 Subject: [PATCH 1/5] Fix isUnchangedStructure comparing Similarity to member directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PersistenceLegacyTypeMappingResult.isUnchangedStructure compared mapping.get(legacyMember) — a Similarity — directly against currentMember via !=, so the check was always true and the method always returned false on the first iteration. As a result the unchanged-structure fast path in PersistenceLegacyTypeHandlerCreator never fired and every legacy handler went through the translating path, even when the layout was actually unchanged. Extract targetElement() from the similarity (with a null guard) before comparing, matching the existing pattern in PersistenceLegacyTypeHandlerCreator.deriveEnumOrdinalMapping. --- .../types/PersistenceLegacyTypeMappingResult.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java index 1bbdefc2..8f18eabd 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java @@ -109,13 +109,14 @@ public static boolean isUnchangedStructure( { final PersistenceTypeDefinitionMember legacyMember = legacy.next() ; final PersistenceTypeDefinitionMember currentMember = current.next(); - + // all legacy members must be directly mapped to their order-wise corresponding current member. - if(mapping.get(legacyMember) != currentMember) + final Similarity match = mapping.get(legacyMember); + if(match == null || match.targetElement() != currentMember) { return false; } - + // and the types must be the same, of course. Member names are sound and smoke. if(!legacyMember.typeName().equals(currentMember.typeName())) { From 0f01a94393099faa01496cd73494401d1840ab1d Mon Sep 17 00:00:00 2001 From: fh-ms Date: Thu, 30 Apr 2026 15:18:45 +0200 Subject: [PATCH 2/5] Avoid NPE for primitive-definition members in isUnchangedStructure --- .../types/PersistenceLegacyTypeMappingResult.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java index 8f18eabd..21b24c82 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java @@ -17,6 +17,7 @@ import static org.eclipse.serializer.util.X.notNull; import java.util.Iterator; +import java.util.Objects; import org.eclipse.serializer.collections.EqHashEnum; import org.eclipse.serializer.collections.XUtilsCollection; @@ -118,7 +119,18 @@ public static boolean isUnchangedStructure( } // and the types must be the same, of course. Member names are sound and smoke. - if(!legacyMember.typeName().equals(currentMember.typeName())) + // workaround: primitive-definition members carry a null typeName; compare their bit-layout definition instead. + // TODO: find cleaner solution + if(legacyMember.isPrimitiveDefinition() && currentMember.isPrimitiveDefinition()) + { + if(!((PersistenceTypeDescriptionMemberPrimitiveDefinition)legacyMember).primitiveDefinition().equals( + ((PersistenceTypeDescriptionMemberPrimitiveDefinition)currentMember).primitiveDefinition() + )) + { + return false; + } + } + else if(!Objects.equals(legacyMember.typeName(), currentMember.typeName())) { return false; } From 0b44e3ce7ba287aec4cadfd619d5d101cad0da77 Mon Sep 17 00:00:00 2001 From: fh-ms Date: Thu, 30 Apr 2026 16:11:27 +0200 Subject: [PATCH 3/5] Use equalsStructure for member comparison in isUnchangedStructure --- .../PersistenceLegacyTypeMappingResult.java | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java index 21b24c82..5a831d7e 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java @@ -17,7 +17,6 @@ import static org.eclipse.serializer.util.X.notNull; import java.util.Iterator; -import java.util.Objects; import org.eclipse.serializer.collections.EqHashEnum; import org.eclipse.serializer.collections.XUtilsCollection; @@ -118,19 +117,10 @@ public static boolean isUnchangedStructure( return false; } - // and the types must be the same, of course. Member names are sound and smoke. - // workaround: primitive-definition members carry a null typeName; compare their bit-layout definition instead. - // TODO: find cleaner solution - if(legacyMember.isPrimitiveDefinition() && currentMember.isPrimitiveDefinition()) - { - if(!((PersistenceTypeDescriptionMemberPrimitiveDefinition)legacyMember).primitiveDefinition().equals( - ((PersistenceTypeDescriptionMemberPrimitiveDefinition)currentMember).primitiveDefinition() - )) - { - return false; - } - } - else if(!Objects.equals(legacyMember.typeName(), currentMember.typeName())) + // the persistent structure of the two members must match; equalsStructure handles + // every member kind (regular fields, primitive definitions, enum constants, complex + // generic fields) via polymorphic dispatch. + if(!legacyMember.equalsStructure(currentMember)) { return false; } From 9126bd1ab42ac6108aef560e9be9177397403801 Mon Sep 17 00:00:00 2001 From: fh-ms Date: Thu, 30 Apr 2026 16:56:34 +0200 Subject: [PATCH 4/5] Add equalsLayout for name-agnostic member comparison isUnchangedStructure now uses equalsLayout instead of equalsStructure, so refactoring-mapping renames still take the unchanged-structure fast path. equalsLayout mirrors equalsStructure on each subtype but drops the member-name check; equalsStructure is left unchanged. --- .../PersistenceLegacyTypeMappingResult.java | 10 +-- .../PersistenceTypeDescriptionMember.java | 61 ++++++++++++++++++- ...enceTypeDescriptionMemberEnumConstant.java | 9 +++ ...eDescriptionMemberFieldGenericComplex.java | 12 ++++ ...ptionMemberFieldGenericVariableLength.java | 9 +++ ...eDescriptionMemberPrimitiveDefinition.java | 12 ++++ 6 files changed, 107 insertions(+), 6 deletions(-) diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java index 5a831d7e..a0d21d33 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceLegacyTypeMappingResult.java @@ -117,10 +117,12 @@ public static boolean isUnchangedStructure( return false; } - // the persistent structure of the two members must match; equalsStructure handles - // every member kind (regular fields, primitive definitions, enum constants, complex - // generic fields) via polymorphic dispatch. - if(!legacyMember.equalsStructure(currentMember)) + // the persistent layout of the two members must match. equalsLayout handles every + // member kind (regular fields, primitive definitions, enum constants, complex generic + // fields) via polymorphic dispatch and intentionally ignores member names: the + // mapping/order check above already establishes identity, so a renamed-but-otherwise- + // unchanged member still qualifies for the unchanged-structure fast path. + if(!legacyMember.equalsLayout(currentMember)) { return false; } diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMember.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMember.java index 60ffbfe1..ac0a2d97 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMember.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMember.java @@ -129,16 +129,37 @@ public default boolean equalsDescription(final PersistenceTypeDescriptionMember /** * Structure means equal order of members by type name and simple name.
* Not qualifier, since that is only required for intra-type field identification - * + * * @param other the description to compare to * @return if this and the other description's structure are equal - * + * * @see #equalDescription(PersistenceTypeDescriptionMember, PersistenceTypeDescriptionMember) */ public default boolean equalsStructure(final PersistenceTypeDescriptionMember other) { return equalTypeAndName(this, other); } + + /** + * Like {@link #equalsStructure(PersistenceTypeDescriptionMember)} but ignores the member's + * simple name. Compares only the persistent layout (type name; for primitive-definition members + * the bit-layout descriptor; for complex generic fields the nested members compared again by + * layout). + *

+ * Used by the "unchanged structure" fast path in legacy-handler creation, where the identity + * correspondence between legacy and current members is already established by the refactoring + * mapping and only the on-disk layout needs to be confirmed — e.g. a renamed field whose + * type and offset are unchanged still produces the same persisted bytes. + * + * @param other the description to compare to. + * @return if this and the other description's persistent layout are equal. + * + * @see #equalLayout(PersistenceTypeDescriptionMember, PersistenceTypeDescriptionMember) + */ + public default boolean equalsLayout(final PersistenceTypeDescriptionMember other) + { + return other != null && Objects.equals(this.typeName(), other.typeName()); + } /** * Tests whether two members have the same {@link #typeName()} and {@link #name()}. The comparison @@ -203,6 +224,25 @@ public static boolean equalStructure( // must delegate to the implementation since complex fields must deep-check their nested fields return m1 == m2 || m1 != null && m1.equalsStructure(m2); } + + /** + * Static null-safe counterpart to {@link #equalsLayout(PersistenceTypeDescriptionMember)}. + * Delegation to the instance method is required so that complex (nested) member descriptions + * can deep-check their nested members by layout. + * + * @param m1 the first member. + * @param m2 the second member. + * + * @return {@code true} if both members have equal persistent layout. + */ + public static boolean equalLayout( + final PersistenceTypeDescriptionMember m1, + final PersistenceTypeDescriptionMember m2 + ) + { + // must delegate to the implementation since complex fields must deep-check their nested fields + return m1 == m2 || m1 != null && m1.equalsLayout(m2); + } /** * Static counterpart to {@link #equalsDescription(PersistenceTypeDescriptionMember)} that handles a @@ -529,6 +569,23 @@ public static boolean equalStructures( return equalMembers(members1, members2, PersistenceTypeDescriptionMember::equalStructure); } + /** + * Pairwise compares two member sequences using + * {@link #equalLayout(PersistenceTypeDescriptionMember, PersistenceTypeDescriptionMember)}. + * + * @param members1 the first sequence. + * @param members2 the second sequence. + * + * @return {@code true} if the sequences are pairwise equal in persistent layout. + */ + public static boolean equalLayouts( + final XGettingSequence members1, + final XGettingSequence members2 + ) + { + return equalMembers(members1, members2, PersistenceTypeDescriptionMember::equalLayout); + } + /** * Pairwise iterates two member sequences in lock-step and applies the passed {@link Equalator} to * every pair. The iteration intentionally proceeds while either iterator has elements, padding diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberEnumConstant.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberEnumConstant.java index e7b8b9f5..e37ac0b6 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberEnumConstant.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberEnumConstant.java @@ -49,6 +49,15 @@ public default boolean equalsStructure(final PersistenceTypeDescriptionMember ot && equalName(this, (PersistenceTypeDescriptionMemberEnumConstant)other) ; } + + @Override + public default boolean equalsLayout(final PersistenceTypeDescriptionMember other) + { + // enum-constant entries have no layout footprint (length 0); identity lives in name(). + // Callers that pair members by mapping/order already establish identity correspondence, + // so layout equality here only confirms both members are enum-constant entries. + return other instanceof PersistenceTypeDescriptionMemberEnumConstant; + } @Override public default boolean equalsDescription(final PersistenceTypeDescriptionMember member) diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberFieldGenericComplex.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberFieldGenericComplex.java index 212ad916..a876f59f 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberFieldGenericComplex.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberFieldGenericComplex.java @@ -71,6 +71,18 @@ public default boolean equalsStructure(final PersistenceTypeDescriptionMember ot ) ; } + + @Override + public default boolean equalsLayout(final PersistenceTypeDescriptionMember other) + { + return PersistenceTypeDescriptionMemberFieldGenericVariableLength.super.equalsLayout(other) + && other instanceof PersistenceTypeDescriptionMemberFieldGenericComplex + && PersistenceTypeDescriptionMember.equalLayouts( + this.members(), + ((PersistenceTypeDescriptionMemberFieldGenericComplex)other).members() + ) + ; + } /** * Type-specific overload of diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberFieldGenericVariableLength.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberFieldGenericVariableLength.java index 3153ade0..b4964be8 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberFieldGenericVariableLength.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberFieldGenericVariableLength.java @@ -47,6 +47,15 @@ public default boolean equalsStructure(final PersistenceTypeDescriptionMember ot && PersistenceTypeDescriptionMemberFieldGeneric.super.equalsStructure(other) ; } + + @Override + public default boolean equalsLayout(final PersistenceTypeDescriptionMember other) + { + // the type check is the only specific thing here. + return other instanceof PersistenceTypeDescriptionMemberFieldGenericVariableLength + && PersistenceTypeDescriptionMemberFieldGeneric.super.equalsLayout(other) + ; + } /** * Type-specific overload of diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberPrimitiveDefinition.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberPrimitiveDefinition.java index 5da9cbdc..51fa1300 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberPrimitiveDefinition.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMemberPrimitiveDefinition.java @@ -65,6 +65,18 @@ public default boolean equalsDescription(final PersistenceTypeDescriptionMember && equalDescription(this, (PersistenceTypeDescriptionMemberPrimitiveDefinition)member) ; } + + @Override + public default boolean equalsLayout(final PersistenceTypeDescriptionMember other) + { + // primitive-definition members carry a null typeName; identity lives in primitiveDefinition(). + return other instanceof PersistenceTypeDescriptionMemberPrimitiveDefinition + && Objects.equals( + this.primitiveDefinition(), + ((PersistenceTypeDescriptionMemberPrimitiveDefinition)other).primitiveDefinition() + ) + ; + } /** * Tests whether two primitive-definition entries record the same {@link #primitiveDefinition()} From d265172beb8d391299f97a1b126e92d49ce54031 Mon Sep 17 00:00:00 2001 From: fh-ms Date: Thu, 30 Apr 2026 18:46:03 +0200 Subject: [PATCH 5/5] Fix javadoc --- .../persistence/types/PersistenceTypeDescriptionMember.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMember.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMember.java index ac0a2d97..b2cf0eb4 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMember.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/PersistenceTypeDescriptionMember.java @@ -133,7 +133,7 @@ public default boolean equalsDescription(final PersistenceTypeDescriptionMember * @param other the description to compare to * @return if this and the other description's structure are equal * - * @see #equalDescription(PersistenceTypeDescriptionMember, PersistenceTypeDescriptionMember) + * @see #equalStructure(PersistenceTypeDescriptionMember, PersistenceTypeDescriptionMember) */ public default boolean equalsStructure(final PersistenceTypeDescriptionMember other) {