diff --git a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/model/metamodel/FieldMappingValidator.java b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/model/metamodel/FieldMappingValidator.java index ec8e24835..d832a5a5c 100644 --- a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/model/metamodel/FieldMappingValidator.java +++ b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/model/metamodel/FieldMappingValidator.java @@ -122,11 +122,16 @@ private static void validateLexicalFormAttribute(AbstractAttribute attribu private static void validateSimpleLiteralField(AbstractAttribute attribute) { final Class fieldType = getBindableType(attribute); - if (attribute.isSimpleLiteral() && (!String.class.isAssignableFrom(fieldType) && !Enum.class.isAssignableFrom( - fieldType) && !attribute.getConverter().supportsAxiomValueType(String.class))) { - throw new InvalidFieldMappingException( - attribute.getJavaField(),"simpleLiteral mapping can only be used on fields of type String or Enum or using a suitable converter."); + + if (!attribute.isSimpleLiteral() || String.class.isAssignableFrom(fieldType) || Enum.class.isAssignableFrom(fieldType)) { + return; } + + if(attribute.getConverter() != null && attribute.getConverter().supportsAxiomValueType(String.class)) { + return; + } + + throw new InvalidFieldMappingException(attribute.getJavaField(),"simpleLiteral mapping can only be used on fields of type String or Enum or using a suitable converter."); } private static Class getBindableType(AbstractAttribute attribute) { diff --git a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/CloneBuilder.java b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/CloneBuilder.java index edac51a27..3725bea2f 100644 --- a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/CloneBuilder.java +++ b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/CloneBuilder.java @@ -33,32 +33,14 @@ import cz.cvut.kbss.jopa.sessions.util.CloneConfiguration; import cz.cvut.kbss.jopa.sessions.util.CloneRegistrationDescriptor; import cz.cvut.kbss.jopa.utils.EntityPropertiesUtils; -import cz.cvut.kbss.ontodriver.model.LangString; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.lang.reflect.Field; -import java.math.BigDecimal; -import java.math.BigInteger; -import java.net.URI; -import java.net.URL; -import java.time.Duration; -import java.time.Instant; -import java.time.LocalDate; -import java.time.LocalDateTime; -import java.time.LocalTime; -import java.time.OffsetDateTime; -import java.time.OffsetTime; -import java.time.Period; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; import java.util.Collection; -import java.util.Date; import java.util.Map; import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; +import java.util.Optional; /** * Builds clones used in transactions for tracking changes. @@ -67,8 +49,6 @@ public class CloneBuilder { private static final Logger LOG = LoggerFactory.getLogger(CloneBuilder.class); - private static final Set> IMMUTABLE_TYPES = getImmutableTypes(); - // Contains entities that are already cloned, so that we don't clone them again private final RepositoryMap visitedEntities; @@ -136,6 +116,7 @@ private Object buildCloneImpl(Object cloneOwner, Field clonedField, Object origi } final Class cls = original.getClass(); final boolean managed = isTypeManaged(cls); + final boolean hasBuilder = instanceHasBuilder(original); final Descriptor descriptor = cloneConfiguration.getDescriptor(); if (managed) { final Object visitedClone = getVisitedEntity(descriptor, original); @@ -151,7 +132,7 @@ private Object buildCloneImpl(Object cloneOwner, Field clonedField, Object origi final LoadStateDescriptor loadState = cloneLoadStateDescriptor(original, clone); uow.getLoadStateRegistry().put(clone, loadState); } - if (!builder.populatesAttributes() && !isImmutable(cls)) { + if (!builder.populatesAttributes() && (managed || hasBuilder)) { populateAttributes(original, clone, cloneConfiguration); } return clone; @@ -189,33 +170,29 @@ private void populateAttributes(Object original, Object clone, CloneConfiguratio continue; } else { final Class origValueClass = origVal.getClass(); - if (isImmutable(origValueClass)) { - // The field is an immutable type - clonedValue = origVal; - } else if (IndirectWrapperHelper.requiresIndirectWrapper(origVal)) { + if (IndirectWrapperHelper.requiresIndirectWrapper(origVal)) { final Descriptor fieldDescriptor = getFieldDescriptor(f, originalClass, configuration.getDescriptor()); // Collection or Map clonedValue = getInstanceBuilder(origVal).buildClone(clone, f, origVal, CloneConfiguration.withDescriptor(fieldDescriptor) .forPersistenceContext(configuration.isForPersistenceContext()) .addPostRegisterHandlers(configuration.getPostRegister())); - } else { + } else if(isTypeManaged(origValueClass)) { // Otherwise, we have a relationship, and we need to clone its target as well if (isOriginalInUoW(origVal)) { // If the reference is already managed clonedValue = uow.getCloneForOriginal(origVal); } else { - if (isTypeManaged(origValueClass)) { final Descriptor fieldDescriptor = getFieldDescriptor(f, originalClass, configuration.getDescriptor()); clonedValue = getVisitedEntity(configuration.getDescriptor(), origVal); if (clonedValue == null) { clonedValue = uow.registerExistingObject(origVal, new CloneRegistrationDescriptor(fieldDescriptor).postCloneHandlers(configuration.getPostRegister())); } - } else { - clonedValue = buildClone(origVal, configuration); - } } + } else { + // We assume that the value is immutable + clonedValue = origVal; } } EntityPropertiesUtils.setFieldValue(f, clone, clonedValue); @@ -234,33 +211,6 @@ private Descriptor getFieldDescriptor(Field field, Class entityClass, Descrip return entityDescriptor.getAttributeDescriptor(fieldSpec); } - /** - * Check if the given class is an immutable type. - *

- * Objects of immutable types do not have to be cloned, because they cannot be modified. - *

- * Note that this method does not do any sophisticated verification, it just checks if the specified class - * corresponds to a small set of predefined conditions, e.g. primitive class, enum, String. - * - * @param cls the class to check - * @return Whether the class represents immutable objects - */ - static boolean isImmutable(Class cls) { - return cls.isPrimitive() || cls.isEnum() || IMMUTABLE_TYPES.contains(cls); - } - - /** - * Checks if the specified object is immutable. - *

- * {@code null} is considered immutable, otherwise, this method just calls {@link #isImmutable(Class)}. - * - * @param object The instance to check - * @return immutability status - */ - static boolean isImmutable(Object object) { - return object == null || isImmutable(object.getClass()); - } - /** * Merges the changes on clone into the original object. * @@ -272,16 +222,16 @@ public void mergeChanges(ObjectChangeSet changeSet) { try { for (ChangeRecord change : changeSet.getChanges()) { Field f = change.getAttribute().getJavaField(); - if (isImmutable(f.getType())) { - EntityPropertiesUtils.setFieldValue(f, original, change.getNewValue()); - continue; - } + Object origVal = EntityPropertiesUtils.getFieldValue(f, original); Object newVal = change.getNewValue(); - if (newVal == null) { + + if(newVal == null) { EntityPropertiesUtils.setFieldValue(f, original, null); - } else { + } else if(isTypeManaged(f.getType()) || instanceHasBuilder(newVal)) { getInstanceBuilder(newVal).mergeChanges(f, original, origVal, newVal); + } else { + EntityPropertiesUtils.setFieldValue(f, original, newVal); } loadStateDescriptor.setLoaded((FieldSpecification) change.getAttribute(), LoadState.LOADED); } @@ -305,6 +255,8 @@ AbstractInstanceBuilder getInstanceBuilder(Object toClone) { return builders.getBuilder(toClone); } + boolean instanceHasBuilder(Object toClone) { return builders.hasBuilder(toClone); } + boolean isTypeManaged(Class cls) { return uow.isEntityType(cls); } @@ -341,72 +293,48 @@ public void removeVisited(Object instance, Descriptor descriptor) { visitedEntities.remove(descriptor, instance); } - private static Set> getImmutableTypes() { - return Stream.of(Boolean.class, - Character.class, - Byte.class, - Short.class, - Integer.class, - Long.class, - Float.class, - Double.class, - BigInteger.class, - BigDecimal.class, - Void.class, - String.class, - URI.class, - URL.class, - LocalDate.class, - LocalTime.class, - LocalDateTime.class, - ZonedDateTime.class, - OffsetDateTime.class, - OffsetTime.class, - ZoneOffset.class, - Instant.class, - Duration.class, - Period.class, - LangString.class).collect(Collectors.toSet()); - } private final class Builders { private final AbstractInstanceBuilder defaultBuilder; - private final ManagedInstanceBuilder managedInstanceBuilder; - private final AbstractInstanceBuilder dateBuilder; + private final AbstractInstanceBuilder managedInstanceBuilder; private final AbstractInstanceBuilder multilingualStringBuilder; // Lists and Sets - private AbstractInstanceBuilder collectionBuilder; - private AbstractInstanceBuilder mapBuilder; + private final AbstractInstanceBuilder collectionBuilder; + private final AbstractInstanceBuilder mapBuilder; private Builders() { this.defaultBuilder = new DefaultInstanceBuilder(CloneBuilder.this, uow); this.managedInstanceBuilder = new ManagedInstanceBuilder(CloneBuilder.this, uow); - this.dateBuilder = new DateInstanceBuilder(CloneBuilder.this, uow); this.multilingualStringBuilder = new MultilingualStringInstanceBuilder(CloneBuilder.this, uow); + this.mapBuilder = new MapInstanceBuilder(CloneBuilder.this, uow); + this.collectionBuilder = new CollectionInstanceBuilder(CloneBuilder.this, uow); } - private AbstractInstanceBuilder getBuilder(Object toClone) { - if (toClone instanceof Date) { - return dateBuilder; - } - if (toClone instanceof MultilingualString) { - return multilingualStringBuilder; - } - if (toClone instanceof Map) { - if (mapBuilder == null) { - this.mapBuilder = new MapInstanceBuilder(CloneBuilder.this, uow); - } - return mapBuilder; - } else if (toClone instanceof Collection) { - if (collectionBuilder == null) { - this.collectionBuilder = new CollectionInstanceBuilder(CloneBuilder.this, uow); - } - return collectionBuilder; - } else if (isTypeManaged(toClone.getClass())) { - return managedInstanceBuilder; - } else { - return defaultBuilder; + private Optional getAbstractInstanceBuilder(Object toClone) { + if(toClone instanceof MultilingualString) { + return Optional.of(multilingualStringBuilder); + } else if(toClone instanceof Map) { + return Optional.of(mapBuilder); + } else if(toClone instanceof Collection) { + return Optional.of(collectionBuilder); } + + return Optional.empty(); + } + + private boolean hasBuilder(Object toClone) { + return getAbstractInstanceBuilder(toClone).isPresent(); + } + + private AbstractInstanceBuilder getBuilder(Object toClone) { + return getAbstractInstanceBuilder(toClone) + .orElseGet(() -> { + if (toClone != null && isTypeManaged(toClone.getClass())) { + return managedInstanceBuilder; + } + + return defaultBuilder; + }); } } } diff --git a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/CollectionInstanceBuilder.java b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/CollectionInstanceBuilder.java index c5636826d..014188816 100644 --- a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/CollectionInstanceBuilder.java +++ b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/CollectionInstanceBuilder.java @@ -145,10 +145,7 @@ private void cloneCollectionContent(Object cloneOwner, Field field, Collection buildInstanceOfSpecialCollection(Object cloneOwner, Field field, Collection container, CloneConfiguration configuration) { if (arrayAsListClass.isInstance(container)) { @@ -173,8 +171,7 @@ private Collection buildInstanceOfSpecialCollection(Object cloneOwner, Field return arrayList; } else if (singletonListClass.isInstance(container) || singletonSetClass.isInstance(container)) { final Object element = container.iterator().next(); - final Object elementClone = CloneBuilder.isImmutable(element) ? element : - cloneCollectionElement(cloneOwner, field, element, configuration); + final Object elementClone = cloneCollectionElement(cloneOwner, field, element, configuration); final Collection result = CollectionFactory.createDefaultCollection(singletonListClass.isInstance(container) ? CollectionType.LIST : CollectionType.SET); result.add(elementClone); return result; diff --git a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/DateInstanceBuilder.java b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/DateInstanceBuilder.java deleted file mode 100644 index 772a750b2..000000000 --- a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/DateInstanceBuilder.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * JOPA - * Copyright (C) 2024 Czech Technical University in Prague - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3.0 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. - */ -package cz.cvut.kbss.jopa.sessions; - -import cz.cvut.kbss.jopa.sessions.util.CloneConfiguration; -import cz.cvut.kbss.jopa.utils.EntityPropertiesUtils; - -import java.lang.reflect.Field; -import java.util.Date; - -class DateInstanceBuilder extends AbstractInstanceBuilder { - - DateInstanceBuilder(CloneBuilder builder, UnitOfWork uow) { - super(builder, uow); - } - - @Override - Object buildClone(Object cloneOwner, Field field, Object original, CloneConfiguration config) { - if (original == null) { - return null; - } - final Date orig = (Date) original; - return new Date(orig.getTime()); - } - - @Override - void mergeChanges(Field field, Object target, Object originalValue, Object cloneValue) { - EntityPropertiesUtils.setFieldValue(field, target, cloneValue); - } - - @Override - boolean populatesAttributes() { - return true; - } -} diff --git a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/DefaultInstanceBuilder.java b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/DefaultInstanceBuilder.java index 690dfed4f..a557495d5 100644 --- a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/DefaultInstanceBuilder.java +++ b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/DefaultInstanceBuilder.java @@ -51,10 +51,13 @@ class DefaultInstanceBuilder extends AbstractInstanceBuilder { */ @Override Object buildClone(Object cloneOwner, Field field, Object original, CloneConfiguration config) { - if (CloneBuilder.isImmutable(original)) { + final Class javaClass = original.getClass(); + + // Assume the original is immutable and there is no other InstanceBuilder + if (!builder.isTypeManaged(javaClass)) { return original; } - final Class javaClass = original.getClass(); + Object newInstance = buildNewInstanceUsingDefaultConstructor(javaClass); if (newInstance == null) { final Field[] fields = javaClass.getDeclaredFields(); diff --git a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/MapInstanceBuilder.java b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/MapInstanceBuilder.java index 741a099db..d7febbe15 100644 --- a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/MapInstanceBuilder.java +++ b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/MapInstanceBuilder.java @@ -107,10 +107,8 @@ Object buildClone(Object cloneOwner, Field field, Object original, CloneConfigur private Map buildSingletonClone(Object cloneOwner, Field field, Map orig, CloneConfiguration configuration) { Entry e = orig.entrySet().iterator().next(); - Object key = CloneBuilder.isImmutable(e.getKey()) ? e.getKey() : - cloneObject(cloneOwner, field, e.getKey(), configuration); - Object value = CloneBuilder.isImmutable(e.getValue()) ? e.getValue() : - cloneObject(cloneOwner, field, e.getValue(), configuration); + Object key = cloneObject(cloneOwner, field, e.getKey(), configuration); + Object value = cloneObject(cloneOwner, field, e.getValue(), configuration); if ((value instanceof Collection || value instanceof Map) && !(value instanceof ChangeTrackingIndirectCollection)) { value = uow.createIndirectCollection(value, cloneOwner, field); } @@ -123,24 +121,9 @@ private void cloneMapContent(Object cloneOwner, Field field, Map source, return; } final Map m = (Map) target; - final Entry tmp = source.entrySet().iterator().next(); - // Note: If we encounter null -> null mapping first, the whole map will be treated as immutable type map, which can be incorrect - final boolean keyPrimitive = CloneBuilder.isImmutable(tmp.getKey()); - final boolean valuePrimitive = CloneBuilder.isImmutable(tmp.getValue()); for (Entry e : source.entrySet()) { - Object key; - Object value; - if (keyPrimitive) { - if (valuePrimitive) { - m.putAll(source); - break; - } - key = e.getKey(); - value = cloneObject(cloneOwner, field, e.getValue(), configuration); - } else { - key = cloneObject(cloneOwner, field, e.getKey(), configuration); - value = valuePrimitive ? e.getValue() : cloneObject(cloneOwner, field, e.getValue(), configuration); - } + Object key = cloneObject(cloneOwner, field, e.getKey(), configuration); + Object value = cloneObject(cloneOwner, field, e.getValue(), configuration); m.put(key, value); } } @@ -151,8 +134,10 @@ private Object cloneObject(Object owner, Field field, Object obj, CloneConfigura clone = null; } else if (builder.isTypeManaged(obj.getClass())) { clone = uow.registerExistingObject(obj, new CloneRegistrationDescriptor(configuration.getDescriptor()).postCloneHandlers(configuration.getPostRegister())); - } else { + } else if (builder.instanceHasBuilder(obj)) { clone = builder.buildClone(owner, field, obj, configuration.getDescriptor()); + } else { + return obj; // assume it is immutable } return clone; } diff --git a/jopa-impl/src/test/java/cz/cvut/kbss/jopa/sessions/DateInstanceBuilderTest.java b/jopa-impl/src/test/java/cz/cvut/kbss/jopa/sessions/DateInstanceBuilderTest.java deleted file mode 100644 index e1e4724d7..000000000 --- a/jopa-impl/src/test/java/cz/cvut/kbss/jopa/sessions/DateInstanceBuilderTest.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * JOPA - * Copyright (C) 2024 Czech Technical University in Prague - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3.0 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. - */ -package cz.cvut.kbss.jopa.sessions; - -import cz.cvut.kbss.jopa.environment.OWLClassM; -import cz.cvut.kbss.jopa.model.descriptors.Descriptor; -import cz.cvut.kbss.jopa.model.descriptors.EntityDescriptor; -import cz.cvut.kbss.jopa.sessions.util.CloneConfiguration; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import java.lang.reflect.Field; -import java.util.Date; - -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.mock; - -class DateInstanceBuilderTest { - - private OWLClassM entityM; - private Field dateField; - private Descriptor descriptor; - - private final DateInstanceBuilder builder = new DateInstanceBuilder(mock(CloneBuilder.class), mock(UnitOfWork.class)); - - @BeforeEach - void setUp() throws Exception { - entityM = new OWLClassM(); - this.dateField = OWLClassM.getDateAttributeField(); - this.descriptor = new EntityDescriptor(); - } - - @Test - void testBuildClone() { - final Date original = new Date(); - final Object res = builder.buildClone(entityM, dateField, original, new CloneConfiguration(descriptor, false)); - assertInstanceOf(Date.class, res); - assertNotSame(original, res); - assertEquals(original, res); - } - - @Test - void testBuildCloneOfNull() { - final Object res = builder.buildClone(entityM, dateField, null, new CloneConfiguration(descriptor, false)); - assertNull(res); - } - - @Test - void testMergeChanges() { - final Date orig = new Date(); - final Date clone = new Date(System.currentTimeMillis() - 100000); - entityM.setDateAttribute(orig); - builder.mergeChanges(dateField, entityM, orig, clone); - assertEquals(clone, entityM.getDateAttribute()); - } -}