Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checking for possible composite converters in GraphEntityMapper.writeProperty #933

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Name> {

@Override
public Map<String, ?> 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<String, String>();
map.put("name", value.toString());
map.put("partialName1", value.partialName1);
map.put("partialName2", value.partialName2);

return map;
}

@Override
public Name toEntityAttribute(Map<String, ?> value) {
var name = new Name();
name.partialName1 = String.valueOf(value.get("partialName1"));
name.partialName2 = String.valueOf(value.get("partialName2"));

return name;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
}
}