From 35256f736cf36d36e1a9fe682ef87fd45c29e7d1 Mon Sep 17 00:00:00 2001 From: evanchooly Date: Wed, 14 Oct 2026 23:04:08 -0400 Subject: [PATCH] Compare entity and property models by identity (plan item 2.7) PropertyModel.hashCode included its lazily specialized codec, and EntityModel.hashCode hashed its property models, so both hashes changed the first time an entity was encoded, stranding them in hash-based collections. Both overrides are removed, so the models compare by identity, which is what every caller relies on. --- .../mapping/codec/pojo/EntityModel.java | 27 ------------------- .../mapping/codec/pojo/PropertyModel.java | 24 ----------------- .../codec/harness/CodecHarnessTest.java | 20 ++++++++++++++ design/mapping-game-plan.md | 2 +- 4 files changed, 21 insertions(+), 52 deletions(-) diff --git a/core/src/main/java/dev/morphia/mapping/codec/pojo/EntityModel.java b/core/src/main/java/dev/morphia/mapping/codec/pojo/EntityModel.java index 5b832cca80f..b913849d031 100644 --- a/core/src/main/java/dev/morphia/mapping/codec/pojo/EntityModel.java +++ b/core/src/main/java/dev/morphia/mapping/codec/pojo/EntityModel.java @@ -528,33 +528,6 @@ protected void initializeListeners() { } } - @Override - public int hashCode() { - return Objects.hash(annotations, propertyModelsByName, propertyModelsByMappedName, creatorFactory, - discriminatorEnabled, discriminatorKey(), discriminator(), getType(), collectionName(), listeners); - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (!(o instanceof EntityModel)) { - return false; - } - final EntityModel that = (EntityModel) o; - return discriminatorEnabled == that.discriminatorEnabled - && Objects.equals(annotations, that.annotations) - && Objects.equals(propertyModelsByName, that.propertyModelsByName) - && Objects.equals(propertyModelsByMappedName, that.propertyModelsByMappedName) - && Objects.equals(creatorFactory, that.creatorFactory) - && Objects.equals(discriminatorKey(), that.discriminatorKey()) - && Objects.equals(discriminator(), that.discriminator()) - && Objects.equals(getType(), that.getType()) - && Objects.equals(collectionName(), that.collectionName()) - && Objects.equals(listeners, that.listeners); - } - @Override public String toString() { String properties = propertyModelsByName.values().stream() diff --git a/core/src/main/java/dev/morphia/mapping/codec/pojo/PropertyModel.java b/core/src/main/java/dev/morphia/mapping/codec/pojo/PropertyModel.java index 91072ab0b08..21411ea3bd2 100644 --- a/core/src/main/java/dev/morphia/mapping/codec/pojo/PropertyModel.java +++ b/core/src/main/java/dev/morphia/mapping/codec/pojo/PropertyModel.java @@ -23,7 +23,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.StringJoiner; @@ -174,12 +173,6 @@ Codec getCodec() { return codec; } - @Override - public int hashCode() { - return Objects.hash(getName(), getTypeData(), getMappedName(), codec, getAccessor(), serialization, - annotationMap.values(), getNormalizedType()); - } - /** * @param document the Document get the value from * @return the value from first mapping of this field @@ -295,23 +288,6 @@ public boolean hasAnnotation(Class type) { return annotationMap.containsKey(type); } - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (!(o instanceof PropertyModel that)) { - return false; - } - return getName().equals(that.getName()) - && getTypeData().equals(that.getTypeData()) - && getMappedName().equals(that.getMappedName()) - && Objects.equals(codec, that.codec) - && getAccessor().equals(that.getAccessor()) - && serialization.equals(that.serialization) - && Objects.equals(getNormalizedType(), that.getNormalizedType()); - } - /** * @param datastore * @return the custom codec to use if set or null diff --git a/core/src/test/java/dev/morphia/test/mapping/codec/harness/CodecHarnessTest.java b/core/src/test/java/dev/morphia/test/mapping/codec/harness/CodecHarnessTest.java index 20f989e0c4f..3014e3c136d 100644 --- a/core/src/test/java/dev/morphia/test/mapping/codec/harness/CodecHarnessTest.java +++ b/core/src/test/java/dev/morphia/test/mapping/codec/harness/CodecHarnessTest.java @@ -26,6 +26,7 @@ import dev.morphia.mapping.DiscriminatorFunction; import dev.morphia.mapping.MapperType; import dev.morphia.mapping.codec.pojo.EntityModel; +import dev.morphia.mapping.codec.pojo.PropertyModel; import dev.morphia.mapping.codec.pojo.critter.CritterEntityModel; import dev.morphia.test.mapping.codec.harness.models.HarnessArrays; import dev.morphia.test.mapping.codec.harness.models.HarnessCallbackCircle; @@ -206,6 +207,25 @@ private static void assertCallbacks(String message, HarnessCallbackShape shape, message + ": @PrePersist, @PostPersist, @PreLoad, @PostLoad counts"); } + @Test + void modelHashesAreStableAcrossEncoding() { + for (MapperType mapper : MapperType.values()) { + MorphiaDatastore datastore = datastore(Variant.DEFAULTS, mapper); + EntityModel model = datastore.getMapper().getEntityModel(HarnessScalars.class); + Set models = new HashSet<>(Set.of(model)); + Set properties = new HashSet<>(model.getProperties()); + + // encoding specializes each property's codec, which used to change its hash + Codec codec = codec(datastore, HarnessScalars.class); + decode(codec, encode(codec, HarnessScalars.sample(), COLLECTIBLE)); + + assertTrue(models.contains(model), mapper + ": the model's hash changed"); + for (PropertyModel property : model.getProperties()) { + assertTrue(properties.contains(property), mapper + ": the hash of " + property.getName() + " changed"); + } + } + } + @Test void readsLoadOnlyProperties() { // @LoadOnly values are never written, so the fixtures can't show that they are read diff --git a/design/mapping-game-plan.md b/design/mapping-game-plan.md index c06ca13b99a..5de9f9f86b2 100644 --- a/design/mapping-game-plan.md +++ b/design/mapping-game-plan.md @@ -174,7 +174,7 @@ structural item covers are moved to that item (see Phases 4 and 5). - Write the test first (`List`, where `Shape` has `@PrePersist`). - Fix it by delegating subtypes *before* running callbacks in `LifecycleEncoder`/`LifecycleDecoder`. -### 2.7 `PropertyModel.hashCode` includes the lazy codec (D5) +### ~~2.7 `PropertyModel.hashCode` includes the lazy codec (D5)~~ - Delete the `equals`/`hashCode` overrides on `EntityModel` and `PropertyModel`, so both compare by identity. This also removes the deep `equals` cost on `CopyOnWriteArraySet.add` and in `EntityEncoder`. - Audit tests for any that rely on structural equality.