Skip to content

Commit 1fb0945

Browse files
michael-simonsmichael-simons
committed
GH-2168 - Treat no attributes annotated with @ConvertedWith as association.
The actuall issue here is caused by `@ConvertedWith` not indicating at all times that a property isn't an association: The moment there is a converter (either through the Spring context or `@ConvertedWith`) it can't be an association anymore. The exception `Required identifier property not found for class` was thrown because SDN treated the annotated property as association. This is fixed now. The commit adds tests for this and also adds an example how to use composite or discrete property convertes. However, in alignment with previous SDN+OGM behavior, a prefixless decomposition of properties into maps and composition back from them is not possible. This closes #2168.
1 parent fcf767a commit 1fb0945

File tree

8 files changed

+352
-1
lines changed

8 files changed

+352
-1
lines changed

‎src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.springframework.data.mapping.model.AnnotationBasedPersistentProperty;
2828
import org.springframework.data.mapping.model.Property;
2929
import org.springframework.data.mapping.model.SimpleTypeHolder;
30+
import org.springframework.data.neo4j.core.convert.ConvertWith;
3031
import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyConverter;
3132
import org.springframework.data.neo4j.core.schema.CompositeProperty;
3233
import org.springframework.data.neo4j.core.schema.Relationship;
@@ -71,7 +72,7 @@ final class DefaultNeo4jPersistentProperty extends AnnotationBasedPersistentProp
7172

7273
Class<?> targetType = getActualType();
7374
return !(simpleTypeHolder.isSimpleType(targetType) || this.mappingContext.hasCustomWriteTarget(targetType)
74-
|| isAnnotationPresent(TargetNode.class) || isComposite());
75+
|| isAnnotationPresent(TargetNode.class) || isComposite() || isAnnotationPresent(ConvertWith.class));
7576
});
7677

7778
this.customConversion = Lazy.of(() -> {

‎src/main/java/org/springframework/data/neo4j/core/schema/CompositeProperty.java

+2
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@
6464
@API(status = API.Status.STABLE, since = "6.0")
6565
public @interface CompositeProperty {
6666

67+
String UNSET = new String("");
68+
6769
/**
6870
* @return A converter that allows to store arbitrary objects as decomposed maps on nodes and relationships. The
6971
* default converter allows only maps as composite properties.

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2168/DomainObject.java

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.issues.gh2168;
17+
18+
import lombok.Getter;
19+
import lombok.Setter;
20+
21+
import org.springframework.data.neo4j.core.convert.ConvertWith;
22+
import org.springframework.data.neo4j.core.schema.CompositeProperty;
23+
import org.springframework.data.neo4j.core.schema.GeneratedValue;
24+
import org.springframework.data.neo4j.core.schema.Id;
25+
import org.springframework.data.neo4j.core.schema.Node;
26+
import org.springframework.data.neo4j.core.schema.Property;
27+
28+
/**
29+
* @author Michael J. Simons
30+
*/
31+
@Node
32+
@Getter
33+
@Setter
34+
public class DomainObject {
35+
36+
@Id
37+
@Property
38+
@GeneratedValue(GeneratedValueStrategy.class)
39+
private String id;
40+
41+
@CompositeProperty(converter = UnrelatedObjectCompositePropertyConverter.class)
42+
private UnrelatedObject storedAsMultipleProperties = new UnrelatedObject();
43+
44+
@ConvertWith(converter = UnrelatedObjectPropertyConverter.class)
45+
private UnrelatedObject storedAsSingleProperty = new UnrelatedObject();
46+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2168/GeneratedValueStrategy.java

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.issues.gh2168;
17+
18+
import java.util.concurrent.ThreadLocalRandom;
19+
20+
import org.springframework.data.neo4j.core.schema.IdGenerator;
21+
22+
/**
23+
* @author Michael J. Simons
24+
*/
25+
public final class GeneratedValueStrategy implements IdGenerator<String> {
26+
27+
@Override
28+
public String generateId(String primaryLabel, Object entity) {
29+
return "Please use something that one randomly create conflicting ids :) " +
30+
ThreadLocalRandom.current().nextLong();
31+
}
32+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2168/Gh2168IT.java

+134
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.issues.gh2168;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import java.util.Collections;
21+
import java.util.Optional;
22+
23+
import org.junit.jupiter.api.BeforeAll;
24+
import org.junit.jupiter.api.Test;
25+
import org.neo4j.driver.Driver;
26+
import org.neo4j.driver.Session;
27+
import org.neo4j.driver.Transaction;
28+
import org.neo4j.driver.types.Node;
29+
import org.springframework.beans.factory.annotation.Autowired;
30+
import org.springframework.context.annotation.Bean;
31+
import org.springframework.context.annotation.Configuration;
32+
import org.springframework.data.neo4j.config.AbstractNeo4jConfig;
33+
import org.springframework.data.neo4j.repository.Neo4jRepository;
34+
import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
35+
import org.springframework.data.neo4j.test.Neo4jExtension;
36+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
37+
import org.springframework.transaction.annotation.EnableTransactionManagement;
38+
39+
/**
40+
* @author Michael J. Simons
41+
*/
42+
@Neo4jIntegrationTest
43+
public class Gh2168IT {
44+
45+
protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
46+
47+
@BeforeAll
48+
protected static void setupData() {
49+
try (Transaction transaction = neo4jConnectionSupport.getDriver().session().beginTransaction()) {
50+
transaction.run("MATCH (n) detach delete n");
51+
transaction.run("CREATE (:DomainObject{id: 'A'})");
52+
transaction.commit();
53+
}
54+
}
55+
56+
@Test // GH-2168
57+
void findByIdShouldWork(@Autowired DomainObjectRepository domainObjectRepository) {
58+
59+
Optional<DomainObject> optionalResult = domainObjectRepository.findById("A");
60+
assertThat(optionalResult)
61+
.map(DomainObject::getId)
62+
.hasValue("A");
63+
}
64+
65+
@Test // GH-2168
66+
void compositePropertyCustomConverterDefaultPrefixShouldWork(
67+
@Autowired DomainObjectRepository repository,
68+
@Autowired Driver driver
69+
) {
70+
71+
DomainObject domainObject = new DomainObject();
72+
domainObject.setStoredAsMultipleProperties(new UnrelatedObject(true, 4711L));
73+
domainObject = repository.save(domainObject);
74+
75+
try (Session session = driver.session()) {
76+
Node node = session
77+
.run("MATCH (n:DomainObject {id: $id}) RETURN n", Collections.singletonMap("id", domainObject.getId()))
78+
.single().get(0).asNode();
79+
assertThat(node.get("storedAsMultipleProperties.aBooleanValue").asBoolean()).isTrue();
80+
assertThat(node.get("storedAsMultipleProperties.aLongValue").asLong()).isEqualTo(4711L);
81+
}
82+
83+
domainObject = repository.findById(domainObject.getId()).get();
84+
assertThat(domainObject.getStoredAsMultipleProperties())
85+
.satisfies(t -> {
86+
assertThat(t.isABooleanValue()).isTrue();
87+
assertThat(t.getALongValue()).isEqualTo(4711L);
88+
});
89+
}
90+
91+
92+
93+
// That test and the underlying mapping cause the original issue to fail, as `@ConvertWith` was missing for non-simple
94+
// types in the lookup that checked whether something is an association or not
95+
@Test // GH-2168
96+
void propertyCustomConverterDefaultPrefixShouldWork(
97+
@Autowired DomainObjectRepository repository,
98+
@Autowired Driver driver
99+
) {
100+
101+
DomainObject domainObject = new DomainObject();
102+
domainObject.setStoredAsSingleProperty(new UnrelatedObject(true, 4711L));
103+
domainObject = repository.save(domainObject);
104+
105+
try (Session session = driver.session()) {
106+
Node node = session
107+
.run("MATCH (n:DomainObject {id: $id}) RETURN n", Collections.singletonMap("id", domainObject.getId()))
108+
.single().get(0).asNode();
109+
assertThat(node.get("storedAsSingleProperty").asString()).isEqualTo("true;4711");
110+
}
111+
112+
domainObject = repository.findById(domainObject.getId()).get();
113+
assertThat(domainObject.getStoredAsSingleProperty())
114+
.satisfies(t -> {
115+
assertThat(t.isABooleanValue()).isTrue();
116+
assertThat(t.getALongValue()).isEqualTo(4711L);
117+
});
118+
}
119+
120+
interface DomainObjectRepository extends Neo4jRepository<DomainObject, String> {
121+
}
122+
123+
@Configuration
124+
@EnableTransactionManagement
125+
@EnableNeo4jRepositories(considerNestedRepositories = true)
126+
static class Config extends AbstractNeo4jConfig {
127+
128+
@Bean
129+
public Driver driver() {
130+
131+
return neo4jConnectionSupport.getDriver();
132+
}
133+
}
134+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2168/UnrelatedObject.java

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.issues.gh2168;
17+
18+
import lombok.Getter;
19+
import lombok.Setter;
20+
21+
/**
22+
* @author Michael J. Simons
23+
*/
24+
@Getter
25+
@Setter
26+
public class UnrelatedObject {
27+
28+
private boolean aBooleanValue;
29+
private Long aLongValue;
30+
31+
public UnrelatedObject() {
32+
aLongValue = 0L;
33+
}
34+
35+
public UnrelatedObject(boolean aBooleanValue, Long aLongValue) {
36+
this.aBooleanValue = aBooleanValue;
37+
this.aLongValue = aLongValue;
38+
}
39+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2168/UnrelatedObjectCompositePropertyConverter.java

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.issues.gh2168;
17+
18+
import java.util.HashMap;
19+
import java.util.Map;
20+
import java.util.Optional;
21+
22+
import org.neo4j.driver.Value;
23+
import org.neo4j.driver.Values;
24+
import org.springframework.data.neo4j.core.convert.Neo4jConversionService;
25+
import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyToMapConverter;
26+
27+
/**
28+
* @author Michael J. Simons
29+
*/
30+
public final class UnrelatedObjectCompositePropertyConverter implements Neo4jPersistentPropertyToMapConverter<String, UnrelatedObject> {
31+
32+
private static final String A_BOOLEAN_VALUE = "aBooleanValue";
33+
private static final String A_LONG_VALUE = "aLongValue";
34+
35+
@Override
36+
public Map<String, Value> decompose(UnrelatedObject property, Neo4jConversionService neo4jConversionService) {
37+
Map<String, Value> values = new HashMap<>();
38+
values.put(A_BOOLEAN_VALUE, Values.value(property.isABooleanValue()));
39+
values.put(A_LONG_VALUE, Values.value(property.getALongValue()));
40+
return values;
41+
}
42+
43+
@Override
44+
public UnrelatedObject compose(Map<String, Value> source, Neo4jConversionService neo4jConversionService) {
45+
boolean aBooleanValue = Optional.ofNullable(source.get(A_BOOLEAN_VALUE))
46+
.map(Value::asBoolean)
47+
.orElse(false);
48+
49+
Long aLongValue = Optional.ofNullable(source.get(A_LONG_VALUE))
50+
.map(Value::asLong)
51+
.orElse(0L);
52+
53+
return new UnrelatedObject(aBooleanValue, aLongValue);
54+
}
55+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2168/UnrelatedObjectPropertyConverter.java

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.issues.gh2168;
17+
18+
import org.neo4j.driver.Value;
19+
import org.neo4j.driver.Values;
20+
import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyConverter;
21+
22+
/**
23+
* @author Michael J. Simons
24+
*/
25+
public final class UnrelatedObjectPropertyConverter implements Neo4jPersistentPropertyConverter<UnrelatedObject> {
26+
27+
@Override
28+
public Value write(UnrelatedObject source) {
29+
30+
return Values.value(source.isABooleanValue() + ";" + source.getALongValue());
31+
}
32+
33+
@Override
34+
public UnrelatedObject read(Value source) {
35+
36+
String[] concatenatedValues = source.asString().split(";");
37+
if (concatenatedValues.length == 2) {
38+
return new UnrelatedObject(Boolean.parseBoolean(concatenatedValues[0]), Long.parseLong(concatenatedValues[1]));
39+
}
40+
return null;
41+
}
42+
}

0 commit comments

Comments
 (0)