From 1b66182a0615dff28d7b7d2005beea7793f3a67a Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Mon, 31 Jan 2022 19:10:43 +0100 Subject: [PATCH] GH-2451 - Apply property filter correctly to composite properties too. The changes defers the decomposition of the property into single values as late as possible, so that property filter can be left unchanged but now works on the actual attribute names. This fixes #2451. --- .../data/neo4j/core/NamedParameters.java | 25 ++++++- .../mapping/DefaultNeo4jEntityConverter.java | 3 +- .../neo4j/core/mapping/MapValueWrapper.java | 42 ++++++++++++ .../ImperativeCompositePropertiesIT.java | 33 ++++++++- .../ReactiveCompositePropertiesIT.java | 38 +++++++++++ .../integration/imperative/ProjectionIT.java | 36 ++++++++-- .../issues/gh2451/WidgetEntity.java | 68 +++++++++++++++++++ .../issues/gh2451/WidgetProjection.java | 30 ++++++++ .../issues/gh2451/WidgetRepository.java | 28 ++++++++ 9 files changed, 294 insertions(+), 9 deletions(-) create mode 100644 src/main/java/org/springframework/data/neo4j/core/mapping/MapValueWrapper.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetEntity.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetProjection.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetRepository.java diff --git a/src/main/java/org/springframework/data/neo4j/core/NamedParameters.java b/src/main/java/org/springframework/data/neo4j/core/NamedParameters.java index 7f268ca1b..cfa4ac6f6 100644 --- a/src/main/java/org/springframework/data/neo4j/core/NamedParameters.java +++ b/src/main/java/org/springframework/data/neo4j/core/NamedParameters.java @@ -23,6 +23,9 @@ import java.util.stream.Collectors; import org.apiguardian.api.API; import org.neo4j.cypherdsl.core.Cypher; +import org.neo4j.driver.Value; +import org.springframework.data.neo4j.core.mapping.Constants; +import org.springframework.data.neo4j.core.mapping.MapValueWrapper; import org.springframework.lang.Nullable; /** @@ -53,6 +56,7 @@ final class NamedParameters { * @param value The value of the new parameter * @throws IllegalStateException when a parameter with the given name already exists */ + @SuppressWarnings("unchecked") void add(String name, @Nullable Object value) { if (this.parameters.containsKey(name)) { @@ -61,7 +65,26 @@ final class NamedParameters { "Duplicate parameter name: '%s' already in the list of named parameters with value '%s'. New value would be '%s'", name, previousValue == null ? "null" : previousValue.toString(), value == null ? "null" : value.toString())); } - this.parameters.put(name, value); + + if (Constants.NAME_OF_PROPERTIES_PARAM.equals(name) && value != null) { + this.parameters.put(name, unwrapMapValueWrapper((Map) value)); + } else { + this.parameters.put(name, value); + } + } + + private static Map unwrapMapValueWrapper(Map properties) { + + Map newProperties = new HashMap<>(properties.size()); + properties.forEach((k, v) -> { + if (v instanceof MapValueWrapper) { + Value mapValue = ((MapValueWrapper) v).getMapValue(); + mapValue.keys().forEach(k2 -> newProperties.put(k2, mapValue.get(k2))); + } else { + newProperties.put(k, v); + } + }); + return newProperties; } /** diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java index 4a42126fc..3895b20ea 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java @@ -195,7 +195,8 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { final Value value = conversionService.writeValue(propertyAccessor.getProperty(p), p.getTypeInformation(), p.getOptionalConverter()); if (p.isComposite()) { - value.keys().forEach(k -> properties.put(k, value.get(k))); + properties.put(p.getPropertyName(), new MapValueWrapper(value)); + //value.keys().forEach(k -> properties.put(k, value.get(k))); } else { properties.put(p.getPropertyName(), value); } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/MapValueWrapper.java b/src/main/java/org/springframework/data/neo4j/core/mapping/MapValueWrapper.java new file mode 100644 index 000000000..970e3afa8 --- /dev/null +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/MapValueWrapper.java @@ -0,0 +1,42 @@ +/* + * Copyright 2011-2022 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 org.apiguardian.api.API; +import org.neo4j.driver.Value; + +/** + * A wrapper or marker for a Neo4j {@link org.neo4j.driver.internal.value.MapValue} that needs to be unwrapped when used + * for properties. + * This class exists solely for projection / filtering purposes: It allows the {@link DefaultNeo4jEntityConverter} to keep + * the composite properties together as long as possible (in the form of above's {@code MapValue}. Thus, the key in the + * {@link Constants#NAME_OF_PROPERTIES_PARAM} fits the filter so that we can continue filtering after binding. + * + * @author Michael J. Simons + */ +@API(status = API.Status.INTERNAL, since = "6.1") +public final class MapValueWrapper { + + private final Value mapValue; + + MapValueWrapper(Value mapValue) { + this.mapValue = mapValue; + } + + public Value getMapValue() { + return mapValue; + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/conversion_imperative/ImperativeCompositePropertiesIT.java b/src/test/java/org/springframework/data/neo4j/integration/conversion_imperative/ImperativeCompositePropertiesIT.java index 826b57b95..3b1103ba9 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/conversion_imperative/ImperativeCompositePropertiesIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/conversion_imperative/ImperativeCompositePropertiesIT.java @@ -17,6 +17,8 @@ package org.springframework.data.neo4j.integration.conversion_imperative; import static org.assertj.core.api.Assertions.assertThat; +import java.util.Collections; + import org.junit.jupiter.api.Test; import org.neo4j.driver.Driver; import org.neo4j.driver.Record; @@ -27,6 +29,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.data.neo4j.config.AbstractNeo4jConfig; import org.springframework.data.neo4j.core.DatabaseSelectionProvider; +import org.springframework.data.neo4j.core.Neo4jTemplate; import org.springframework.data.neo4j.core.convert.Neo4jConversions; import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager; @@ -41,8 +44,6 @@ import org.springframework.data.neo4j.test.Neo4jIntegrationTest; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.EnableTransactionManagement; -import java.util.Collections; - /** * @author Michael J. Simons * @soundtrack Die Toten Hosen - Learning English, Lesson Two @@ -106,6 +107,34 @@ class ImperativeCompositePropertiesIT extends CompositePropertiesITBase { } } + public interface ThingProjection { + + ThingWithCompositeProperties.SomeOtherDTO getSomeOtherDTO(); + } + + @Test // GH-2451 + void compositePropertiesShouldBeFilterableEvenOnNonMapTypes(@Autowired Repository repository, @Autowired Neo4jTemplate template) { + + Long id = createNodeWithCompositeProperties(); + ThingWithCompositeProperties thing = repository.findById(id).get(); + thing.setDatesWithTransformedKey(Collections.singletonMap("Test", null)); + thing.setSomeDatesByEnumA(Collections.singletonMap(ThingWithCompositeProperties.EnumA.VALUE_AA, null)); + thing.setSomeOtherDTO(null); + template.saveAs(thing, ThingProjection.class); + + try (Session session = driver.session()) { + Record r = session.readTransaction(tx -> tx.run("MATCH (t:CompositeProperties) WHERE id(t) = $id RETURN t", + Collections.singletonMap("id", id)).single()); + Node n = r.get("t").asNode(); + assertThat(n.asMap()) + .containsKeys( + "someDatesByEnumA.VALUE_AA", + "datesWithTransformedKey.test" + ) + .doesNotContainKeys("dto.x", "dto.y", "dto.z"); + } + } + public interface Repository extends Neo4jRepository { } diff --git a/src/test/java/org/springframework/data/neo4j/integration/conversion_reactive/ReactiveCompositePropertiesIT.java b/src/test/java/org/springframework/data/neo4j/integration/conversion_reactive/ReactiveCompositePropertiesIT.java index 4a1f1bc69..196e7add8 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/conversion_reactive/ReactiveCompositePropertiesIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/conversion_reactive/ReactiveCompositePropertiesIT.java @@ -17,7 +17,11 @@ package org.springframework.data.neo4j.integration.conversion_reactive; import static org.assertj.core.api.Assertions.assertThat; +import org.neo4j.driver.Record; +import org.neo4j.driver.Session; +import org.neo4j.driver.types.Node; import org.springframework.data.neo4j.core.ReactiveDatabaseSelectionProvider; +import org.springframework.data.neo4j.core.ReactiveNeo4jTemplate; import org.springframework.data.neo4j.core.convert.Neo4jConversions; import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; import org.springframework.data.neo4j.core.transaction.ReactiveNeo4jTransactionManager; @@ -106,6 +110,40 @@ class ReactiveCompositePropertiesIT extends CompositePropertiesITBase { .verifyComplete(); } + public interface ThingProjection { + + ThingWithCompositeProperties.SomeOtherDTO getSomeOtherDTO(); + } + + @Test // GH-2451 + void compositePropertiesShouldBeFilterableEvenOnNonMapTypes(@Autowired Repository repository, @Autowired ReactiveNeo4jTemplate template) { + + Long id = createNodeWithCompositeProperties(); + repository.findById(id) + .map(thing -> { + thing.setDatesWithTransformedKey(Collections.singletonMap("Test", null)); + thing.setSomeDatesByEnumA(Collections.singletonMap(ThingWithCompositeProperties.EnumA.VALUE_AA, null)); + thing.setSomeOtherDTO(null); + return thing; + }) + .flatMap(thing -> template.saveAs(thing, ThingProjection.class)) + .as(StepVerifier::create) + .expectNextCount(1L) + .verifyComplete(); + + try (Session session = driver.session()) { + Record r = session.readTransaction(tx -> tx.run("MATCH (t:CompositeProperties) WHERE id(t) = $id RETURN t", + Collections.singletonMap("id", id)).single()); + Node n = r.get("t").asNode(); + assertThat(n.asMap()) + .containsKeys( + "someDatesByEnumA.VALUE_AA", + "datesWithTransformedKey.test" + ) + .doesNotContainKeys("dto.x", "dto.y", "dto.z"); + } + } + public interface Repository extends ReactiveNeo4jRepository { } diff --git a/src/test/java/org/springframework/data/neo4j/integration/imperative/ProjectionIT.java b/src/test/java/org/springframework/data/neo4j/integration/imperative/ProjectionIT.java index fe98780b4..aa5894afb 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/imperative/ProjectionIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/imperative/ProjectionIT.java @@ -18,8 +18,8 @@ package org.springframework.data.neo4j.integration.imperative; import static org.assertj.core.api.Assertions.assertThat; import java.util.AbstractMap; +import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; @@ -48,13 +48,16 @@ import org.springframework.data.neo4j.core.DatabaseSelectionProvider; import org.springframework.data.neo4j.core.Neo4jTemplate; import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager; +import org.springframework.data.neo4j.integration.issues.gh2451.WidgetEntity; +import org.springframework.data.neo4j.integration.issues.gh2451.WidgetProjection; +import org.springframework.data.neo4j.integration.issues.gh2451.WidgetRepository; import org.springframework.data.neo4j.integration.shared.common.DepartmentEntity; -import org.springframework.data.neo4j.integration.shared.common.PersonDepartmentQueryResult; -import org.springframework.data.neo4j.integration.shared.common.PersonEntity; import org.springframework.data.neo4j.integration.shared.common.NamesOnly; import org.springframework.data.neo4j.integration.shared.common.NamesOnlyDto; import org.springframework.data.neo4j.integration.shared.common.NamesWithSpELCity; import org.springframework.data.neo4j.integration.shared.common.Person; +import org.springframework.data.neo4j.integration.shared.common.PersonDepartmentQueryResult; +import org.springframework.data.neo4j.integration.shared.common.PersonEntity; import org.springframework.data.neo4j.integration.shared.common.PersonSummary; import org.springframework.data.neo4j.integration.shared.common.ProjectionTest1O1; import org.springframework.data.neo4j.integration.shared.common.ProjectionTestLevel1; @@ -133,6 +136,9 @@ class ProjectionIT { projectionTestRootId = result.get(0).asLong(); projectionTestLevel1Id = result.get(1).asLong(); projectionTest1O1Id = result.get(2).asLong(); + + transaction.run("create (w:Widget {code: 'Window1', label: 'yyy'})").consume(); + transaction.commit(); bookmarkCapture.seedWith(session.lastBookmark()); } @@ -365,6 +371,22 @@ class ProjectionIT { .satisfies(ProjectionIT::projectedEntities); } + @Test // GH-2451 + void compositePropertiesShouldBeIncludedInProjections(@Autowired WidgetRepository repository, + @Autowired Neo4jTemplate template) { + + String code = "Window1"; + WidgetEntity window = repository.findByCode(code).get(); + window.setLabel("changed"); + window.getAdditionalFields().put("key1", "value1"); + + template.saveAs(window, WidgetProjection.class); + + window = repository.findByCode(code).get(); + assertThat(window.getLabel()).isEqualTo("changed"); + assertThat(window.getAdditionalFields()).containsEntry("key1", "value1"); + } + private static void projectedEntities(PersonDepartmentQueryResult personAndDepartment) { assertThat(personAndDepartment.getPerson()).extracting(PersonEntity::getId).isEqualTo("p1"); assertThat(personAndDepartment.getPerson()).extracting(PersonEntity::getEmail).isEqualTo("p1@dep1.org"); @@ -479,7 +501,7 @@ class ProjectionIT { } @Configuration - @EnableNeo4jRepositories(considerNestedRepositories = true) + @EnableNeo4jRepositories(considerNestedRepositories = true, basePackageClasses = {ProjectionIT.class, WidgetEntity.class}) @EnableTransactionManagement static class Config extends AbstractNeo4jConfig { @@ -495,7 +517,11 @@ class ProjectionIT { @Override protected Collection getMappingBasePackages() { - return Collections.singletonList(DepartmentEntity.class.getPackage().getName()); + + List packages = new ArrayList<>(); + packages.add(DepartmentEntity.class.getPackage().getName()); + packages.add(WidgetEntity.class.getPackage().getName()); + return packages; } @Override diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetEntity.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetEntity.java new file mode 100644 index 000000000..c83bd34af --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetEntity.java @@ -0,0 +1,68 @@ +/* + * Copyright 2011-2022 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.gh2451; + +import java.util.HashMap; +import java.util.Map; + +import org.springframework.data.neo4j.core.schema.CompositeProperty; +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; +import org.springframework.data.neo4j.core.schema.Node; + +/** + * @author Michael J. Simons + */ +@Node("Widget") +public class WidgetEntity { + + @GeneratedValue + @Id + private Long id; + private String code; + private String label; + + @CompositeProperty + private Map additionalFields = new HashMap<>(); + + public Long getId() { + return id; + } + + public String getCode() { + return code; + } + + public void setCode(String code) { + this.code = code; + } + + public String getLabel() { + return label; + } + + public void setLabel(String label) { + this.label = label; + } + + public Map getAdditionalFields() { + return additionalFields; + } + + public void setAdditionalFields(Map additionalFields) { + this.additionalFields = additionalFields; + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetProjection.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetProjection.java new file mode 100644 index 000000000..e9d4b03f2 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetProjection.java @@ -0,0 +1,30 @@ +/* + * Copyright 2011-2022 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.gh2451; + +import java.util.Map; + +/** + * @author Michael J. Simons + */ +public interface WidgetProjection { + + String getCode(); + + String getLabel(); + + Map getAdditionalFields(); +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetRepository.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetRepository.java new file mode 100644 index 000000000..b49b37a45 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2451/WidgetRepository.java @@ -0,0 +1,28 @@ +/* + * Copyright 2011-2022 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.gh2451; + +import java.util.Optional; + +import org.springframework.data.neo4j.repository.Neo4jRepository; + +/** + * @author Michael J. Simons + */ +public interface WidgetRepository extends Neo4jRepository { + + Optional findByCode(String code); +}