diff --git a/core/src/main/java/dev/morphia/mapping/InstanceCreatorFactoryImpl.java b/core/src/main/java/dev/morphia/mapping/InstanceCreatorFactoryImpl.java index c5d25a639e5..aee96dbf53a 100644 --- a/core/src/main/java/dev/morphia/mapping/InstanceCreatorFactoryImpl.java +++ b/core/src/main/java/dev/morphia/mapping/InstanceCreatorFactoryImpl.java @@ -12,17 +12,12 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - /** * @hidden * @morphia.internal */ @MorphiaInternal public class InstanceCreatorFactoryImpl implements InstanceCreatorFactory { - private static final Logger LOG = LoggerFactory.getLogger(InstanceCreatorFactoryImpl.class); - private final EntityModel model; private Function creator; @@ -45,15 +40,10 @@ public MorphiaInstanceCreator create(Conversions conversions) { ConstructorCreator.Plan plan = new ConstructorCreator.Plan(model, constructor); creator = (c) -> new ConstructorCreator(plan, c); } else { - LOG.info("using old creator approach: " + model.getType().getName()); - try { - Constructor declared = model.getType().getDeclaredConstructor(); - creator = (c) -> new NoArgCreator(declared); - } catch (NoSuchMethodException e) { - Constructor full = ConstructorCreator.getFullConstructor(model); - ConstructorCreator.Plan plan = new ConstructorCreator.Plan(model, full); - creator = (c) -> new ConstructorCreator(plan, c); - } + // bestConstructor already picks a no-arg constructor when there is one + Constructor full = ConstructorCreator.getFullConstructor(model); + ConstructorCreator.Plan plan = new ConstructorCreator.Plan(model, full); + creator = (c) -> new ConstructorCreator(plan, c); } } diff --git a/core/src/main/java/dev/morphia/mapping/NoArgCreator.java b/core/src/main/java/dev/morphia/mapping/NoArgCreator.java deleted file mode 100644 index 82aefcd86cf..00000000000 --- a/core/src/main/java/dev/morphia/mapping/NoArgCreator.java +++ /dev/null @@ -1,57 +0,0 @@ -package dev.morphia.mapping; - -import java.lang.reflect.Constructor; - -import dev.morphia.annotations.internal.MorphiaInternal; -import dev.morphia.mapping.codec.MorphiaInstanceCreator; -import dev.morphia.mapping.codec.pojo.PropertyModel; -import dev.morphia.sofia.Sofia; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - -/** - * @hidden - * @morphia.internal - */ -@MorphiaInternal -public class NoArgCreator implements MorphiaInstanceCreator { - private Object instance; - private final Constructor noArgsConstructor; - - /** - * Creates the creator - * - * @param noArgsConstructor the constructor - */ - @SuppressFBWarnings("EI_EXPOSE_REP2") - public NoArgCreator(Constructor noArgsConstructor) { - this.noArgsConstructor = noArgsConstructor; - this.noArgsConstructor.setAccessible(true); - } - - @Override - public void set(Object value, PropertyModel model) { - model.getAccessor().set(instance(), value); - } - - private Object instance() { - if (instance == null) { - try { - instance = noArgsConstructor.newInstance(); - } catch (ReflectiveOperationException e) { - throw new MappingException(Sofia.instantiationProblem(noArgsConstructor.getDeclaringClass().getName(), e.getMessage()), e); - } - } - return instance; - } - - @Override - public Object getInstance() { - return instance(); - } - - @Override - public boolean isEagerInstanceSafe() { - return true; - } -} diff --git a/core/src/main/java/dev/morphia/mapping/internal/ConstructorCreator.java b/core/src/main/java/dev/morphia/mapping/internal/ConstructorCreator.java index be500cfc365..388b8c078cf 100644 --- a/core/src/main/java/dev/morphia/mapping/internal/ConstructorCreator.java +++ b/core/src/main/java/dev/morphia/mapping/internal/ConstructorCreator.java @@ -8,6 +8,7 @@ import java.util.Map; import java.util.Objects; import java.util.TreeMap; +import java.util.regex.Pattern; import com.mongodb.lang.Nullable; @@ -39,11 +40,17 @@ */ @MorphiaInternal public class ConstructorCreator implements MorphiaInstanceCreator { + private static final Pattern UNNAMED_PARAMETER = Pattern.compile("arg[0-9]+"); + private static final Object[] NO_PARAMETERS = new Object[0]; + private final Plan plan; private final Conversions conversions; private final Object[] parameters; - private final List pendingModels = new ArrayList<>(); - private final List pendingValues = new ArrayList<>(); + // properties the constructor doesn't take, set once it has run; allocated only if a value arrives before then + @Nullable + private List pendingModels; + @Nullable + private List pendingValues; private Object instance; /** @@ -63,7 +70,7 @@ public ConstructorCreator(EntityModel model, Constructor constructor, Convers public ConstructorCreator(Plan plan, Conversions conversions) { this.plan = plan; this.conversions = conversions; - this.parameters = plan.zeroValues.clone(); + this.parameters = plan.types.length == 0 ? NO_PARAMETERS : plan.zeroValues.clone(); } /** @@ -93,7 +100,7 @@ public Plan(EntityModel model, Constructor constructor) { types[i] = parameter.getType(); zeroValues[i] = zeroValue(parameter); String name = getParameterName(parameter); - if (name.matches("arg[0-9]+")) { + if (UNNAMED_PARAMETER.matcher(name).matches()) { throw new MappingException(Sofia.unnamedConstructorParameter(model.getType().getName())); } if (positions.put(name, i) != null) { @@ -138,8 +145,10 @@ public Object getInstance() { if (instance == null) { try { instance = plan.constructor.newInstance(parameters); - for (int i = 0; i < pendingModels.size(); i++) { - pendingModels.get(i).setValue(instance, pendingValues.get(i)); + if (pendingModels != null) { + for (int i = 0; i < pendingModels.size(); i++) { + pendingModels.get(i).setValue(instance, pendingValues.get(i)); + } } } catch (Exception e) { throw new MappingException(Sofia.cannotInstantiate(plan.model.getType().getName(), e.getMessage()), e); @@ -193,6 +202,10 @@ private static boolean namesMatchProperties(EntityModel model, Constructor co public void set(@Nullable Object value, PropertyModel model) { if (instance != null) { model.setValue(instance, value); + } else if (plan.types.length == 0) { + // a no-arg constructor needs none of the values, so construct now and set each value as it arrives instead + // of buffering them all + model.setValue(getInstance(), value); } else { Integer position = plan.positions.get(model.getName()); if (position != null) { @@ -200,6 +213,10 @@ public void set(@Nullable Object value, PropertyModel model) { } else { // only properties the constructor doesn't take need setting after construction. re-setting the others would // overwrite whatever the constructor did with the value and, for final fields, write them reflectively (JEP 500). + if (pendingModels == null) { + pendingModels = new ArrayList<>(); + pendingValues = new ArrayList<>(); + } pendingModels.add(model); pendingValues.add(value); } diff --git a/core/src/test/java/dev/morphia/test/mapping/internal/ConstructorCreatorTest.java b/core/src/test/java/dev/morphia/test/mapping/internal/ConstructorCreatorTest.java index b05bc8f798c..6e4cd611eaa 100644 --- a/core/src/test/java/dev/morphia/test/mapping/internal/ConstructorCreatorTest.java +++ b/core/src/test/java/dev/morphia/test/mapping/internal/ConstructorCreatorTest.java @@ -95,6 +95,22 @@ public void testBestConstructor() { Assertions.assertEquals(0, constructor.getParameterCount()); } + @Test + public void noArgConstructorSetsValuesAsTheyArrive() { + EntityModel model = getMapper().map(Default.class).get(0); + MorphiaInstanceCreator creator = model.getInstanceCreator(getMapper().getConversions()); + Assertions.assertTrue(creator.isEagerInstanceSafe()); + + creator.set("named", model.getProperty("name")); + Default instance = (Default) creator.getInstance(); + Assertions.assertEquals("named", instance.name); + + // a value that needs converting still goes through Conversions on this path + creator.set(42.0, model.getProperty("count")); + Assertions.assertSame(instance, creator.getInstance()); + Assertions.assertEquals(42, instance.count); + } + @Test public void creatorsDoNotShareState() { EntityModel model = getMapper().map(Address.class).get(0); diff --git a/design/mapping-game-plan.md b/design/mapping-game-plan.md index 9d4363415d6..32c5c9985e0 100644 --- a/design/mapping-game-plan.md +++ b/design/mapping-game-plan.md @@ -210,7 +210,7 @@ JMH numbers on both mappers. - Optional: send a known-rejected type straight to the fallback, avoiding a throw and stack trace for each drifted field. -### 3.2 No-arg creator fast path (#2, A2/B4) +### ~~3.2 No-arg creator fast path (#2, A2/B4)~~ - When the chosen constructor has no parameters, construct the instance first and set values through `model.setValue`, which keeps `Conversions`. Allocate pending lists lazily. - Delete the dead `NoArgCreator` branch and its `LOG.info`. Delete `NoArgCreator` itself, which also skips