From 74e6bfbcfcb29c3d309d8a13c29cbfa6f399185c Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Thu, 5 Jan 2023 10:56:02 +0100 Subject: [PATCH] GH-2640 - Introduce API for dynamic configuration of transient properties. (#2645) This change introduces the concept of `PersistentPropertyCharacteristics` and `PersistentPropertyCharacteristicsProvider`. The latter can be registered implicitly as a `@Bean` with the mapping context or via a custom mapping context. It allows checking the properties and their owner for indicators like type and so on to treat them as transient or read only. Examples have been added to the `Neo4jMappingContextTest` and `PersistentPropertyCharacteristicsIT`. This closes #2640. --- .../config/Neo4jCdiConfigurationSupport.java | 2 +- .../DefaultNeo4jPersistentProperty.java | 16 +- .../core/mapping/Neo4jMappingContext.java | 112 +++++++++--- .../PersistentPropertyCharacteristics.java | 82 +++++++++ ...istentPropertyCharacteristicsProvider.java | 35 ++++ .../core/mapping/Neo4jMappingContextTest.java | 31 ++++ .../PersistentPropertyCharacteristicsIT.java | 159 ++++++++++++++++++ .../query/Neo4jNestedMapEntityWriterTest.java | 2 +- 8 files changed, 415 insertions(+), 24 deletions(-) create mode 100644 src/main/java/org/springframework/data/neo4j/core/mapping/PersistentPropertyCharacteristics.java create mode 100644 src/main/java/org/springframework/data/neo4j/core/mapping/PersistentPropertyCharacteristicsProvider.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2640/PersistentPropertyCharacteristicsIT.java diff --git a/src/main/java/org/springframework/data/neo4j/config/Neo4jCdiConfigurationSupport.java b/src/main/java/org/springframework/data/neo4j/config/Neo4jCdiConfigurationSupport.java index fca90ee7c..0c4966346 100644 --- a/src/main/java/org/springframework/data/neo4j/config/Neo4jCdiConfigurationSupport.java +++ b/src/main/java/org/springframework/data/neo4j/config/Neo4jCdiConfigurationSupport.java @@ -89,7 +89,7 @@ class Neo4jCdiConfigurationSupport { @Produces @Singleton public Neo4jMappingContext neo4jMappingContext(@SuppressWarnings("CdiInjectionPointsInspection") Driver driver, @Any Instance neo4JConversions) { - return new Neo4jMappingContext(resolve(neo4JConversions), driver.defaultTypeSystem()); + return Neo4jMappingContext.builder().withNeo4jConversions(resolve(neo4JConversions)).withTypeSystem(driver.defaultTypeSystem()).build(); } @Produces @Singleton diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java index f8b627efb..dda9ef714 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java @@ -61,6 +61,8 @@ final class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProp private final Lazy> customConversion; + private final @Nullable PersistentPropertyCharacteristics optionalCharacteristics; + /** * Creates a new {@link AnnotationBasedPersistentProperty}. * @@ -69,7 +71,7 @@ final class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProp * @param simpleTypeHolder type holder */ DefaultNeo4jPersistentProperty(Property property, PersistentEntity owner, - Neo4jMappingContext mappingContext, SimpleTypeHolder simpleTypeHolder) { + Neo4jMappingContext mappingContext, SimpleTypeHolder simpleTypeHolder, @Nullable PersistentPropertyCharacteristics optionalCharacteristics) { super(property, owner, simpleTypeHolder); this.mappingContext = mappingContext; @@ -101,6 +103,8 @@ final class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProp return this.mappingContext.getOptionalCustomConversionsFor(this); }); + + this.optionalCharacteristics = optionalCharacteristics; } @Override @@ -308,7 +312,17 @@ final class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProp @Override public boolean isReadOnly() { + if (optionalCharacteristics != null && optionalCharacteristics.isReadOnly() != null) { + return Boolean.TRUE.equals(optionalCharacteristics.isReadOnly()); + } + Class typeOfAnnotation = org.springframework.data.neo4j.core.schema.Property.class; return isAnnotationPresent(ReadOnlyProperty.class) || (isAnnotationPresent(typeOfAnnotation) && getRequiredAnnotation(typeOfAnnotation).readOnly()); } + + @Override + public boolean isTransient() { + return this.optionalCharacteristics == null || optionalCharacteristics.isTransient() == null ? + super.isTransient() : Boolean.TRUE.equals(optionalCharacteristics.isTransient()); + } } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java index 87bfb5177..6f0765917 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java @@ -66,6 +66,7 @@ import org.springframework.data.neo4j.core.mapping.callback.EventSupport; import org.springframework.data.neo4j.core.schema.IdGenerator; import org.springframework.data.neo4j.core.schema.Node; import org.springframework.data.neo4j.core.schema.PostLoad; +import org.springframework.data.util.Lazy; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.ReflectionUtils; @@ -117,14 +118,94 @@ public final class Neo4jMappingContext extends AbstractMappingContext propertyCharacteristicsProvider; + + /** + * A builder for creating custom instances of a {@link Neo4jMappingContext}. + * @since 6.3.7 + */ + public static class Builder { + + private Neo4jConversions neo4jConversions; + + @Nullable + private TypeSystem typeSystem; + + @Nullable + private PersistentPropertyCharacteristicsProvider persistentPropertyCharacteristicsProvider; + + private Builder() { + this(new Neo4jConversions(), null, null); + } + + private Builder(Neo4jConversions neo4jConversions, @Nullable TypeSystem typeSystem, @Nullable PersistentPropertyCharacteristicsProvider persistentPropertyCharacteristicsProvider) { + this.neo4jConversions = neo4jConversions; + this.typeSystem = typeSystem; + this.persistentPropertyCharacteristicsProvider = persistentPropertyCharacteristicsProvider; + } + + @SuppressWarnings("HiddenField") + public Builder withNeo4jConversions(@Nullable Neo4jConversions neo4jConversions) { + this.neo4jConversions = neo4jConversions; + return this; + } + + @SuppressWarnings("HiddenField") + public Builder withPersistentPropertyCharacteristicsProvider(@Nullable PersistentPropertyCharacteristicsProvider persistentPropertyCharacteristicsProvider) { + this.persistentPropertyCharacteristicsProvider = persistentPropertyCharacteristicsProvider; + return this; + } + + @SuppressWarnings("HiddenField") + public Builder withTypeSystem(@Nullable TypeSystem typeSystem) { + this.typeSystem = typeSystem; + return this; + } + + public Neo4jMappingContext build() { + return new Neo4jMappingContext(this); + } + } + + public static Builder builder() { + return new Builder(); + } + public Neo4jMappingContext() { - this(new Neo4jConversions()); + this(new Builder()); } public Neo4jMappingContext(Neo4jConversions neo4jConversions) { - this(neo4jConversions, null); + this(new Builder(neo4jConversions, null, null)); + } + + /** + * This API is primarily used from inside the CDI extension to configure the type system. This is necessary as + * we don't get notified of the context via {@link #setApplicationContext(ApplicationContext applicationContext)}. + * + * @param neo4jConversions The conversions to be used + * @param typeSystem The current drivers type system. If this is null, we use the default one without accessing the driver. + * @deprecated Use {@link Neo4jMappingContext#builder()} + */ + @API(status = API.Status.INTERNAL, since = "6.0") + @Deprecated + public Neo4jMappingContext(Neo4jConversions neo4jConversions, @Nullable TypeSystem typeSystem) { + this(new Builder(neo4jConversions, typeSystem, null)); + } + + private Neo4jMappingContext(Builder builder) { + + this.conversionService = new DefaultNeo4jConversionService(builder.neo4jConversions); + this.typeSystem = builder.typeSystem == null ? InternalTypeSystem.TYPE_SYSTEM : builder.typeSystem; + this.eventSupport = EventSupport.useExistingCallbacks(this, EntityCallbacks.create()); + + super.setSimpleTypeHolder(builder.neo4jConversions.getSimpleTypeHolder()); + + PersistentPropertyCharacteristicsProvider characteristicsProvider = builder.persistentPropertyCharacteristicsProvider; + this.propertyCharacteristicsProvider = Lazy.of(() -> characteristicsProvider != null || this.beanFactory == null ? + characteristicsProvider : this.beanFactory.getBeanProvider(PersistentPropertyCharacteristicsProvider.class).getIfUnique()); } /** @@ -139,22 +220,6 @@ public final class Neo4jMappingContext extends AbstractMappingContext owner, SimpleTypeHolder simpleTypeHolder) { - return new DefaultNeo4jPersistentProperty(property, owner, this, simpleTypeHolder); + PersistentPropertyCharacteristics optionalCharacteristics = this.propertyCharacteristicsProvider + .getOptional() + .flatMap(provider -> Optional.ofNullable(provider.apply(property, owner))) + .orElse(null); + + return new DefaultNeo4jPersistentProperty(property, owner, this, simpleTypeHolder, optionalCharacteristics); } @Override @@ -278,9 +348,9 @@ public final class Neo4jMappingContext extends AbstractMappingContext getPersistentEntity(TypeInformation typeInformation) { - NodeDescription existingDescription = this.doGetPersistentEntity(typeInformation); + Neo4jPersistentEntity existingDescription = this.doGetPersistentEntity(typeInformation); if (existingDescription != null) { - return (Neo4jPersistentEntity) existingDescription; + return existingDescription; } return super.getPersistentEntity(typeInformation); } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/PersistentPropertyCharacteristics.java b/src/main/java/org/springframework/data/neo4j/core/mapping/PersistentPropertyCharacteristics.java new file mode 100644 index 000000000..984e778ee --- /dev/null +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/PersistentPropertyCharacteristics.java @@ -0,0 +1,82 @@ +/* + * Copyright 2011-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.core.mapping; + +import static org.apiguardian.api.API.Status.STABLE; + +import org.apiguardian.api.API; +import org.springframework.lang.Nullable; + +/** + * The characteristics of a {@link Neo4jPersistentProperty} can diverge from what is by default derived from the annotated + * classes. Diverging characteristics are requested by the {@link Neo4jMappingContext} prior to creating a persistent property. + * Additional providers of characteristics may be registered with the mapping context. + * + * @author Michael J. Simons + * @soundtrack Metallica - Kill 'Em All + * @since 6.3.7 + */ +@API(status = STABLE, since = "6.3.7") +public interface PersistentPropertyCharacteristics { + + /** + * @return {@literal null} to leave the defaults, {@literal true} or {@literal false} otherwise + */ + @Nullable + default Boolean isTransient() { + return null; + } + + /** + * @return {@literal null} to leave the defaults, {@literal true} or {@literal false} otherwise + */ + @Nullable + default Boolean isReadOnly() { + return null; + } + + /** + * @return Characteristics applying the defaults + */ + static PersistentPropertyCharacteristics useDefaults() { + return new PersistentPropertyCharacteristics() { + }; + } + + /** + * @return Characteristics to treat a property as transient + */ + static PersistentPropertyCharacteristics treatAsTransient() { + return new PersistentPropertyCharacteristics() { + @Override + public Boolean isTransient() { + return true; + } + }; + } + + /** + * @return Characteristics to treat a property as transient + */ + static PersistentPropertyCharacteristics treatAsReadOnly() { + return new PersistentPropertyCharacteristics() { + @Override + public Boolean isReadOnly() { + return true; + } + }; + } +} diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/PersistentPropertyCharacteristicsProvider.java b/src/main/java/org/springframework/data/neo4j/core/mapping/PersistentPropertyCharacteristicsProvider.java new file mode 100644 index 000000000..1a828b430 --- /dev/null +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/PersistentPropertyCharacteristicsProvider.java @@ -0,0 +1,35 @@ +/* + * Copyright 2011-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.core.mapping; + +import static org.apiguardian.api.API.Status.STABLE; + +import java.util.function.BiFunction; + +import org.apiguardian.api.API; +import org.springframework.data.mapping.model.Property; + +/** + * An instance of such a provider can be registered as a Spring bean and will be consulted by the {@link Neo4jMappingContext} + * prior to creating and populating {@link Neo4jPersistentProperty persistent properties}. + * + * @author Michael J. Simons + * @soundtrack Metallica - Kill 'Em All + * @since 6.3.7 + */ +@API(status = STABLE, since = "6.3.7") +public interface PersistentPropertyCharacteristicsProvider extends BiFunction, PersistentPropertyCharacteristics> { +} diff --git a/src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java b/src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java index bbefd8277..d67d17a3e 100644 --- a/src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java @@ -634,6 +634,37 @@ class Neo4jMappingContextTest { assertThat(children).containsExactly("B2", "B2a", "B3", "B3a"); } + @Test + void characteristicsShouldBeApplied() { + + Neo4jMappingContext neo4jMappingContext1 = Neo4jMappingContext.builder().withPersistentPropertyCharacteristicsProvider((property, owner) -> { + if (owner.getUnderlyingClass().equals(UserNode.class)) { + if (property.getName().equals("name")) { + return PersistentPropertyCharacteristics.treatAsTransient(); + } else if (property.getName().equals("first_name")) { + return PersistentPropertyCharacteristics.treatAsReadOnly(); + } + } + if (property.getType().equals(ConvertibleType.class)) { + return PersistentPropertyCharacteristics.treatAsTransient(); + } + + return PersistentPropertyCharacteristics.useDefaults(); + }).build(); + Neo4jMappingContext neo4jMappingContext2 = Neo4jMappingContext.builder().build(); + + Neo4jPersistentEntity userEntity = neo4jMappingContext1.getPersistentEntity(UserNode.class); + assertThat(userEntity.getPersistentProperty("name")).isNull(); // Transient properties won't materialize + assertThat(userEntity.getRequiredPersistentProperty("first_name").isTransient()).isFalse(); + assertThat(userEntity.getRequiredPersistentProperty("first_name").isReadOnly()).isTrue(); + + Neo4jPersistentEntity entityWithConvertible = neo4jMappingContext1.getPersistentEntity(EntityWithConvertibleProperty.class); + assertThat(entityWithConvertible.getPersistentProperty("convertibleType")).isNull(); + + entityWithConvertible = neo4jMappingContext2.getPersistentEntity(EntityWithConvertibleProperty.class); + assertThat(entityWithConvertible.getPersistentProperty("convertibleType")).isNotNull(); + } + @Test // GH-2574 void hierarchyMustBeConsistentlyReported() throws ClassNotFoundException { diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2640/PersistentPropertyCharacteristicsIT.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2640/PersistentPropertyCharacteristicsIT.java new file mode 100644 index 000000000..6d134f10d --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2640/PersistentPropertyCharacteristicsIT.java @@ -0,0 +1,159 @@ +/* + * Copyright 2011-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.integration.issues.gh2640; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.UUID; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.neo4j.driver.Driver; +import org.neo4j.driver.Record; +import org.neo4j.driver.Session; +import org.neo4j.driver.Values; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.neo4j.core.DatabaseSelectionProvider; +import org.springframework.data.neo4j.core.Neo4jTemplate; +import org.springframework.data.neo4j.core.mapping.PersistentPropertyCharacteristics; +import org.springframework.data.neo4j.core.mapping.PersistentPropertyCharacteristicsProvider; +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; +import org.springframework.data.neo4j.core.schema.Node; +import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; +import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager; +import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories; +import org.springframework.data.neo4j.test.BookmarkCapture; +import org.springframework.data.neo4j.test.Neo4jExtension; +import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration; +import org.springframework.data.neo4j.test.Neo4jIntegrationTest; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.EnableTransactionManagement; + +/** + * @author Michael J. Simons + */ +@Neo4jIntegrationTest +class PersistentPropertyCharacteristicsIT { + + protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport; + + @BeforeEach + void setupData(@Autowired Driver driver, @Autowired BookmarkCapture bookmarkCapture) { + + try (Session session = driver.session()) { + session.run("MATCH (n) DETACH DELETE n").consume(); + bookmarkCapture.seedWith(session.lastBookmark()); + } + } + + @Test + // GH-2640 + void implicitTransientPropertiesShouldNotBeWritten(@Autowired Driver driver, @Autowired BookmarkCapture bookmarkCapture, @Autowired Neo4jTemplate template) { + + SomeWithImplicitTransientProperties1 node1 = template.save(new SomeWithImplicitTransientProperties1("the name", 1.23)); + SomeWithImplicitTransientProperties2 node2 = template.save(new SomeWithImplicitTransientProperties2(47.11)); + + try (Session session = driver.session(bookmarkCapture.createSessionConfig())) { + List records = session.run("MATCH (n) WHERE n.id IN $ids RETURN n", Values.parameters("ids", Arrays.asList(node1.id.toString(), node2.id.toString()))).list(); + assertThat(records) + .hasSize(2) + .noneMatch(r -> { + Map properties = r.get("n").asNode().asMap(); + return properties.containsKey("foobar") || properties.containsKey("bazbar"); + }); + } + } + + @Node + static class SomeWithImplicitTransientProperties1 { + + @Id + @GeneratedValue + private UUID id; + + private String name; + + private Double foobar; + + SomeWithImplicitTransientProperties1(String name, Double foobar) { + this.name = name; + this.foobar = foobar; + } + } + + @Node + static class SomeWithImplicitTransientProperties2 { + + @Id + @GeneratedValue + private UUID id; + + private Double bazbar; + + SomeWithImplicitTransientProperties2(Double bazbar) { + this.bazbar = bazbar; + } + } + + @Configuration + @EnableTransactionManagement + @EnableNeo4jRepositories(considerNestedRepositories = true) + static class Config extends Neo4jImperativeTestConfiguration { + + @Bean + public BookmarkCapture bookmarkCapture() { + return new BookmarkCapture(); + } + + @Override + public PlatformTransactionManager transactionManager( + Driver driver, DatabaseSelectionProvider databaseNameProvider) { + + BookmarkCapture bookmarkCapture = bookmarkCapture(); + return new Neo4jTransactionManager(driver, databaseNameProvider, + Neo4jBookmarkManager.create(bookmarkCapture)); + } + + @Bean + public PersistentPropertyCharacteristicsProvider persistentPropertyCharacteristicsProvider() { + return (property, owner) -> { + + if (property.getType().equals(Double.class)) { + return PersistentPropertyCharacteristics.treatAsTransient(); + } + + return PersistentPropertyCharacteristics.useDefaults(); + }; + } + + @Bean + public Driver driver() { + + return neo4jConnectionSupport.getDriver(); + } + + @Override + public boolean isCypher5Compatible() { + return neo4jConnectionSupport.isCypher5SyntaxCompatible(); + } + } +} diff --git a/src/test/java/org/springframework/data/neo4j/repository/query/Neo4jNestedMapEntityWriterTest.java b/src/test/java/org/springframework/data/neo4j/repository/query/Neo4jNestedMapEntityWriterTest.java index 14b98c052..94c82db3f 100644 --- a/src/test/java/org/springframework/data/neo4j/repository/query/Neo4jNestedMapEntityWriterTest.java +++ b/src/test/java/org/springframework/data/neo4j/repository/query/Neo4jNestedMapEntityWriterTest.java @@ -75,7 +75,7 @@ class Neo4jNestedMapEntityWriterTest { Neo4jNestedMapEntityWriterTest() { - this.mappingContext = new Neo4jMappingContext(new Neo4jConversions(), InternalTypeSystem.TYPE_SYSTEM); + this.mappingContext = Neo4jMappingContext.builder().withNeo4jConversions(new Neo4jConversions()).build(); this.mappingContext.setInitialEntitySet(new HashSet<>( Arrays.asList( FlatEntity.class,