From 364c9a0bc4954c41ced154d0fca0660ece5b8c0c Mon Sep 17 00:00:00 2001 From: Christian Banse Date: Wed, 25 Jan 2023 14:31:13 +0100 Subject: [PATCH] Checking for possible composite converters in `GraphEntityMapper.writeProperty` This allows to resolve conflicts when a property is using a composite converter. Fixes #932 --- .../neo4j/ogm/context/GraphEntityMapper.java | 2 + .../gh932/EntityWithCompositeConverter.java | 132 ++++++++++++++++++ .../types/properties/PropertiesTest.java | 41 +++++- 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 neo4j-ogm-tests/neo4j-ogm-integration-tests/src/test/java/org/neo4j/ogm/domain/gh932/EntityWithCompositeConverter.java diff --git a/core/src/main/java/org/neo4j/ogm/context/GraphEntityMapper.java b/core/src/main/java/org/neo4j/ogm/context/GraphEntityMapper.java index 9c0c08445..c63890671 100644 --- a/core/src/main/java/org/neo4j/ogm/context/GraphEntityMapper.java +++ b/core/src/main/java/org/neo4j/ogm/context/GraphEntityMapper.java @@ -286,6 +286,8 @@ private void writeProperty(ClassInfo classInfo, Object instance, Property if (writer == null) { logger.debug("Unable to find property: {} on class: {} for writing", property.getKey(), classInfo.name()); + } else if (writer.isComposite()) { + logger.info("Property {} is already handled by a CompositeAttributeConverter", property.getKey()); } else { Object value = property.getValue(); // merge iterable / arrays and co-erce to the correct attribute type diff --git a/neo4j-ogm-tests/neo4j-ogm-integration-tests/src/test/java/org/neo4j/ogm/domain/gh932/EntityWithCompositeConverter.java b/neo4j-ogm-tests/neo4j-ogm-integration-tests/src/test/java/org/neo4j/ogm/domain/gh932/EntityWithCompositeConverter.java new file mode 100644 index 000000000..b7303fb1d --- /dev/null +++ b/neo4j-ogm-tests/neo4j-ogm-integration-tests/src/test/java/org/neo4j/ogm/domain/gh932/EntityWithCompositeConverter.java @@ -0,0 +1,132 @@ +/* + * Copyright (c) 2002-2022 "Neo4j," + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * 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 + * + * http://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.neo4j.ogm.domain.gh932; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import org.neo4j.ogm.annotation.GeneratedValue; +import org.neo4j.ogm.annotation.Id; +import org.neo4j.ogm.annotation.NodeEntity; +import org.neo4j.ogm.annotation.typeconversion.Convert; +import org.neo4j.ogm.typeconversion.CompositeAttributeConverter; + +/** + * @author Christian Banse + */ +@NodeEntity +public class EntityWithCompositeConverter { + + @Id + @GeneratedValue + private Long id; + + @Convert(value = Converter.class) + private Name name; + + public Name getName() { + return name; + } + + public void setName(Name name) { + this.name = name; + } + + public Long getId() { + return this.id; + } + + @Override public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + EntityWithCompositeConverter that = (EntityWithCompositeConverter) o; + return Objects.equals(id, that.id) && Objects.equals(name, that.name); + } + + @Override + public int hashCode() { + return Objects.hash(id, name); + } + + public static class Name { + private String partialName1; + private String partialName2; + + public String getPartialName1() { + return partialName1; + } + + public String getPartialName2() { + return partialName2; + } + + public void setPartialName1(String partialName1) { + this.partialName1 = partialName1; + } + + public void setPartialName2(String partialName2) { + this.partialName2 = partialName2; + } + + @Override + public String toString() { + return partialName1 + "." + partialName2; + } + + @Override public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + Name name = (Name) o; + return Objects.equals(partialName1, name.partialName1) && Objects.equals(partialName2, name.partialName2); + } + + @Override + public int hashCode() { + return Objects.hash(partialName1, partialName2); + } + } + + public static class Converter implements CompositeAttributeConverter { + + @Override + public Map toGraphProperties(Name value) { + // We want to persist the partial properties of the name as well as a "toString" representation of the whole name as "name" (which "conflicts" with the "name" field on the class). + var map = new HashMap(); + map.put("name", value.toString()); + map.put("partialName1", value.partialName1); + map.put("partialName2", value.partialName2); + + return map; + } + + @Override + public Name toEntityAttribute(Map value) { + var name = new Name(); + name.partialName1 = String.valueOf(value.get("partialName1")); + name.partialName2 = String.valueOf(value.get("partialName2")); + + return name; + } + } +} diff --git a/neo4j-ogm-tests/neo4j-ogm-integration-tests/src/test/java/org/neo4j/ogm/persistence/types/properties/PropertiesTest.java b/neo4j-ogm-tests/neo4j-ogm-integration-tests/src/test/java/org/neo4j/ogm/persistence/types/properties/PropertiesTest.java index 4fdb5f1f7..c64470471 100644 --- a/neo4j-ogm-tests/neo4j-ogm-integration-tests/src/test/java/org/neo4j/ogm/persistence/types/properties/PropertiesTest.java +++ b/neo4j-ogm-tests/neo4j-ogm-integration-tests/src/test/java/org/neo4j/ogm/persistence/types/properties/PropertiesTest.java @@ -29,15 +29,23 @@ import java.util.Iterator; import java.util.Map; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; + import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.neo4j.ogm.context.GraphEntityMapper; +import org.neo4j.ogm.domain.gh932.EntityWithCompositeConverter; import org.neo4j.ogm.domain.properties.SomeNode; import org.neo4j.ogm.domain.properties.User; import org.neo4j.ogm.exception.core.MappingException; import org.neo4j.ogm.session.Session; import org.neo4j.ogm.session.SessionFactory; +import org.neo4j.ogm.testutil.LoggerRule; import org.neo4j.ogm.testutil.TestContainersTestBase; +import org.slf4j.LoggerFactory; /** * @author Frantisek Hartman @@ -49,9 +57,12 @@ public class PropertiesTest extends TestContainersTestBase { private Session session; + @RegisterExtension + private final LoggerRule loggerRule = new LoggerRule(); + @BeforeAll public static void init() { - sessionFactory = new SessionFactory(getDriver(), User.class.getName(), SomeNode.class.getName()); + sessionFactory = new SessionFactory(getDriver(), User.class.getName(), SomeNode.class.getName(), EntityWithCompositeConverter.class.getName()); } @BeforeEach @@ -460,4 +471,32 @@ void manualConversionShouldSupportPropertiesWithouthPrefix() { assertThat(user.getMyProperties()) .containsOnlyKeys("prop1", "prop2"); } + + // GH-932 + @Test + void shouldResolveNameConflictInCompositeConverter() { + + // ensure the right branch in the condition is hit by verifying the log message + Logger logger = (Logger) LoggerFactory.getLogger(GraphEntityMapper.class); + Level originalLevel = logger.getLevel(); + logger.setLevel(Level.INFO); + + try { + EntityWithCompositeConverter e = new EntityWithCompositeConverter(); + EntityWithCompositeConverter.Name n = new EntityWithCompositeConverter.Name(); + n.setPartialName1("some"); + n.setPartialName2("entity"); + e.setName(n); + session.save(e); + session.clear(); + + var loaded = session.load(EntityWithCompositeConverter.class, e.getId()); + assertThat(loaded).isEqualTo(e); + + assertThat(loggerRule.getFormattedMessages()) + .containsSequence("Property name is already handled by a CompositeAttributeConverter"); + } finally { + logger.setLevel(originalLevel); + } + } }