Skip to content

Commit bddfecc

Browse files
GH-1903 - Ensure that duplicated private properties must be made transient in a domain class hierarchy.
See #1903.
1 parent cf20e51 commit bddfecc

File tree

3 files changed

+308
-0
lines changed

3 files changed

+308
-0
lines changed

src/test/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntityTest.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.junit.jupiter.api.Test;
3030
import org.junit.jupiter.params.ParameterizedTest;
3131
import org.junit.jupiter.params.provider.ValueSource;
32+
import org.springframework.data.annotation.Transient;
3233
import org.springframework.data.mapping.MappingException;
3334
import org.springframework.data.neo4j.core.schema.DynamicLabels;
3435
import org.springframework.data.neo4j.core.schema.GeneratedValue;
@@ -69,6 +70,24 @@ void failsOnMultipleDuplicatedProperties() {
6970
.withMessage("Duplicate definition of properties [foo, name] in entity class "
7071
+ "org.springframework.data.neo4j.core.mapping.DefaultNeo4jPersistentEntityTest$EntityWithMultipleDuplicatedProperties.");
7172
}
73+
74+
@Test // GH-1903
75+
void failsOnMultipleInheritedDuplicatedProperties() {
76+
assertThatIllegalStateException()
77+
.isThrownBy(() -> new Neo4jMappingContext().getPersistentEntity(
78+
EntityWithInheritedMultipleDuplicatedProperties.class))
79+
.withMessage("Duplicate definition of property [name] in entity class "
80+
+ "org.springframework.data.neo4j.core.mapping.DefaultNeo4jPersistentEntityTest$EntityWithInheritedMultipleDuplicatedProperties.");
81+
}
82+
83+
@Test // GH-1903
84+
void doesNotFailOnTransientInheritedDuplicatedProperties() {
85+
Neo4jPersistentEntity<?> persistentEntity = new Neo4jMappingContext().getPersistentEntity(
86+
EntityWithNotInheritedTransientProperties.class);
87+
88+
assertThat(persistentEntity).isNotNull();
89+
assertThat(persistentEntity.getPersistentProperty("name")).isNotNull();
90+
}
7291
}
7392

7493
@Nested
@@ -356,6 +375,34 @@ private static class EntityWithMultipleDuplicatedProperties {
356375
@Property("foo") private String thisToo;
357376
}
358377

378+
private abstract static class BaseClassWithPrivatePropertyUnsafe {
379+
380+
@Id @GeneratedValue
381+
private Long id;
382+
383+
private String name;
384+
}
385+
386+
@Node
387+
private static class EntityWithInheritedMultipleDuplicatedProperties extends BaseClassWithPrivatePropertyUnsafe {
388+
389+
private String name;
390+
}
391+
392+
private abstract static class BaseClassWithPrivatePropertySafe {
393+
394+
@Id @GeneratedValue
395+
private Long id;
396+
397+
private @Transient String name;
398+
}
399+
400+
@Node
401+
private static class EntityWithNotInheritedTransientProperties extends BaseClassWithPrivatePropertySafe {
402+
403+
private String name;
404+
}
405+
359406
@Node("a")
360407
private static class EntityWithSingleLabel {
361408
@Id private Long id;
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
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+
17+
package org.springframework.data.neo4j.integration.imperative
18+
19+
import org.assertj.core.api.Assertions.assertThat
20+
import org.junit.jupiter.api.BeforeAll
21+
import org.junit.jupiter.api.Test
22+
import org.neo4j.driver.Driver
23+
import org.springframework.beans.factory.annotation.Autowired
24+
import org.springframework.context.annotation.Bean
25+
import org.springframework.context.annotation.Configuration
26+
import org.springframework.data.neo4j.config.AbstractNeo4jConfig
27+
import org.springframework.data.neo4j.core.Neo4jTemplate
28+
import org.springframework.data.neo4j.core.findById
29+
import org.springframework.data.neo4j.integration.shared.common.ConcreteDataNodeWithAbstractKotlinBase
30+
import org.springframework.data.neo4j.integration.shared.common.ConcreteDataNodeWithOpenKotlinBase
31+
import org.springframework.data.neo4j.integration.shared.common.ConcreteNodeWithAbstractKotlinBase
32+
import org.springframework.data.neo4j.integration.shared.common.ConcreteNodeWithOpenKotlinBase
33+
import org.springframework.data.neo4j.test.Neo4jExtension
34+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest
35+
import org.springframework.transaction.annotation.EnableTransactionManagement
36+
37+
/**
38+
* @author Michael J. Simons
39+
* @soundtrack Genesis - Invisible Touch
40+
*/
41+
@Neo4jIntegrationTest
42+
class KotlinInheritanceIT @Autowired constructor(
43+
private val template: Neo4jTemplate,
44+
private val driver: Driver
45+
) {
46+
47+
companion object {
48+
@JvmStatic
49+
private lateinit var neo4jConnectionSupport: Neo4jExtension.Neo4jConnectionSupport
50+
51+
@BeforeAll
52+
@JvmStatic
53+
fun clearDatabase(@Autowired driver: Driver) {
54+
driver.session().use { session ->
55+
session.run("MATCH (n) DETACH DELETE n")
56+
}
57+
}
58+
}
59+
60+
@Test // GH-1903
61+
fun mappingWithAbstractBaseClassShouldWork() {
62+
63+
var existingId: Long?
64+
driver.session().use { session ->
65+
session.beginTransaction().use { tx ->
66+
existingId = tx.run("CREATE (t:AbstractKotlinBase:ConcreteNodeWithAbstractKotlinBase {name: 'Foo', anotherProperty: 'Bar'}) RETURN id(t) AS id")
67+
.single()["id"].asLong()
68+
tx.commit()
69+
}
70+
}
71+
72+
val existingThing = template.findById<ConcreteNodeWithAbstractKotlinBase>(existingId!!)!!
73+
74+
assertThat(existingThing.name).isEqualTo("Foo")
75+
assertThat(existingThing.anotherProperty).isEqualTo("Bar")
76+
77+
val thing = template.save(ConcreteNodeWithAbstractKotlinBase("onBase", "onDependent"));
78+
assertThat(thing.id).isNotNull()
79+
assertThat(thing.name).isEqualTo("onBase")
80+
assertThat(thing.anotherProperty).isEqualTo("onDependent")
81+
82+
val cnt = driver.session().use { session ->
83+
session.readTransaction { tx ->
84+
tx.run("MATCH (t:AbstractKotlinBase:ConcreteNodeWithAbstractKotlinBase) WHERE id(t) = \$id RETURN count(t)", mapOf("id" to thing.id)).single()[0].asLong()
85+
}
86+
}
87+
assertThat(cnt).isEqualTo(1L)
88+
}
89+
90+
@Test // GH-1903
91+
fun mappingWithDataExtendingAbstractBaseClassShouldWork() {
92+
93+
var existingId: Long?
94+
driver.session().use { session ->
95+
session.beginTransaction().use { tx ->
96+
existingId = tx.run("CREATE (t:AbstractKotlinBase:ConcreteDataNodeWithAbstractKotlinBase {name: 'Foo', anotherProperty: 'Bar'}) RETURN id(t) AS id")
97+
.single()["id"].asLong()
98+
tx.commit()
99+
}
100+
}
101+
102+
val existingThing = template.findById<ConcreteDataNodeWithAbstractKotlinBase>(existingId!!)!!
103+
104+
assertThat(existingThing.name).isEqualTo("Foo")
105+
assertThat(existingThing.anotherProperty).isEqualTo("Bar")
106+
107+
val thing = template.save(ConcreteDataNodeWithAbstractKotlinBase("onBase", "onDependent"));
108+
assertThat(thing.id).isNotNull()
109+
assertThat(thing.name).isEqualTo("onBase")
110+
assertThat(thing.anotherProperty).isEqualTo("onDependent")
111+
112+
val cnt = driver.session().use { session ->
113+
session.readTransaction { tx ->
114+
tx.run("MATCH (t:AbstractKotlinBase:ConcreteDataNodeWithAbstractKotlinBase) WHERE id(t) = \$id RETURN count(t)", mapOf("id" to thing.id)).single()[0].asLong()
115+
}
116+
}
117+
assertThat(cnt).isEqualTo(1L)
118+
}
119+
120+
@Test // GH-1903
121+
fun mappingWithOpenBaseClassShouldWork() {
122+
123+
var existingId: Long =
124+
driver.session().use { session ->
125+
session.writeTransaction { tx ->
126+
tx.run("CREATE (t:OpenKotlinBase:ConcreteNodeWithOpenKotlinBase {name: 'Foo', anotherProperty: 'Bar'}) RETURN id(t) AS id")
127+
.single()["id"].asLong()
128+
}
129+
}
130+
131+
val existingThing = template.findById<ConcreteNodeWithOpenKotlinBase>(existingId!!)!!
132+
133+
assertThat(existingThing.name).isEqualTo("Foo")
134+
assertThat(existingThing.anotherProperty).isEqualTo("Bar")
135+
136+
val thing = template.save(ConcreteNodeWithOpenKotlinBase("onBase", "onDependent"));
137+
assertThat(thing.id).isNotNull()
138+
assertThat(thing.name).isEqualTo("onBase")
139+
assertThat(thing.anotherProperty).isEqualTo("onDependent")
140+
141+
// Note: The open base class used here is not abstract, there fore labels are not inherited
142+
val cnt = driver.session().use { session ->
143+
session.readTransaction { tx ->
144+
tx.run("MATCH (t:ConcreteNodeWithOpenKotlinBase) WHERE NOT t:OpenKotlinBase AND id(t) = \$id RETURN count(t)", mapOf("id" to thing.id)).single()[0].asLong()
145+
}
146+
}
147+
assertThat(cnt).isEqualTo(1L)
148+
}
149+
150+
@Test // GH-1903
151+
fun mappingWithDataExtendingOpenBaseClassShouldWork() {
152+
153+
var existingId: Long =
154+
driver.session().use { session ->
155+
session.writeTransaction { tx ->
156+
tx.run("CREATE (t:OpenKotlinBase:ConcreteDataNodeWithOpenKotlinBase {name: 'Foo', anotherProperty: 'Bar'}) RETURN id(t) AS id")
157+
.single()["id"].asLong()
158+
}
159+
}
160+
161+
val existingThing = template.findById<ConcreteDataNodeWithOpenKotlinBase>(existingId!!)!!
162+
163+
assertThat(existingThing.name).isEqualTo("Foo")
164+
assertThat(existingThing.anotherProperty).isEqualTo("Bar")
165+
166+
val thing = template.save(ConcreteDataNodeWithOpenKotlinBase("onBase", "onDependent"));
167+
assertThat(thing.id).isNotNull()
168+
assertThat(thing.name).isEqualTo("onBase")
169+
assertThat(thing.anotherProperty).isEqualTo("onDependent")
170+
171+
// Note: The open base class used here is not abstract, there fore labels are not inherited
172+
val cnt = driver.session().use { session ->
173+
session.readTransaction { tx ->
174+
tx.run("MATCH (t:ConcreteDataNodeWithOpenKotlinBase) WHERE NOT t:OpenKotlinBase AND id(t) = \$id RETURN count(t)", mapOf("id" to thing.id)).single()[0].asLong()
175+
}
176+
}
177+
assertThat(cnt).isEqualTo(1L)
178+
}
179+
180+
@Configuration
181+
@EnableTransactionManagement
182+
open class MyConfig : AbstractNeo4jConfig() {
183+
@Bean
184+
override fun driver(): Driver {
185+
return neo4jConnectionSupport.driver
186+
}
187+
}
188+
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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.shared.common
17+
18+
import org.springframework.data.annotation.Transient
19+
import org.springframework.data.neo4j.core.schema.GeneratedValue
20+
import org.springframework.data.neo4j.core.schema.Id
21+
import org.springframework.data.neo4j.core.schema.Node
22+
23+
/**
24+
* @author Michael J. Simons
25+
* @soundtrack Genesis - Invisible Touch
26+
*/
27+
@Node
28+
abstract class AbstractKotlinBase(open val name: String) {
29+
30+
@Id
31+
@GeneratedValue
32+
var id: Long? = null
33+
}
34+
35+
/**
36+
* This class passes the name to the abstract base class but does not define a property named name for its own.
37+
*/
38+
@Node
39+
class ConcreteNodeWithAbstractKotlinBase(name: String, val anotherProperty: String) : AbstractKotlinBase(name) {
40+
}
41+
42+
/**
43+
* This class passes the name to the abstract base class and does define a property named name for its own. This property
44+
* must be made transient (another option is to do this in the base class, which is actually preferred. This is not done
45+
* here as the base class defines the property for a non-data class as well)
46+
*/
47+
@Node
48+
data class ConcreteDataNodeWithAbstractKotlinBase(override @Transient val name: String, val anotherProperty: String) : AbstractKotlinBase(name) {
49+
}
50+
51+
@Node
52+
open class OpenKotlinBase(open val name: String?) {
53+
54+
@Id
55+
@GeneratedValue
56+
var id: Long? = null
57+
}
58+
59+
/**
60+
* This class passes the name to the open base class but does not define a property named name for its own.
61+
*/
62+
@Node
63+
class ConcreteNodeWithOpenKotlinBase(name: String, val anotherProperty: String) : OpenKotlinBase(name) {
64+
}
65+
66+
/**
67+
* This class passes the name to the open base class and does define a property named name for its own. This property
68+
* must be made transient (another option is to do this in the base class, which is actually preferred. This is not done
69+
* here as the base class defines the property for a non-data class as well)
70+
*/
71+
@Node
72+
data class ConcreteDataNodeWithOpenKotlinBase(override @Transient val name: String, val anotherProperty: String) : OpenKotlinBase(name) {
73+
}

0 commit comments

Comments
 (0)