From d8bff18b22d82e09beca894465aa2c04a5ecfe85 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Thu, 4 May 2023 16:01:33 +0200 Subject: [PATCH] refactor: Avoid all usage of Neo4j id() function unless required by the users datamodel. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change makes use of the Cypher-DSL dialect capatibilities and uses a String identifier internally for all objects. This will be the `elementId()` on Neo4j 5 and higher and `toString(id())` on Neo4j 4 and lower. Unless explicitly configured by the user through having `@Id @GeneratedValue long id`, we don’t use the `id()` function anymore. This function has been deprecated in Neo4j 5. Users are encouraged to use `@Id @GeneratedValue String id` for assigning the element id as object id or using custom unique identifiers or `@Id @GeneratedValue UUID id` or `@Id @GeneratedValue(value = UUIDStringGenerator.class) String id` for UUIDs. We marked the long ids as deprecated, but don’t have any plans for now to remove them, so that future SDN versions will still be compatible with older Neo4j versions if the data model contains long ids. We opted for using String’s directly to avoid tons of additional instances and rely on the JVMs excellent String caching capatiblities, thus, the `ElementId` interface - not usable so far anyhow - has been removed without current or planned replacement. See #2716 and other tickets, closes #2718. --- etc/migrate-to-element-id.yml | 15 + .../data/neo4j/core/Neo4jTemplate.java | 165 ++++---- .../neo4j/core/ReactiveNeo4jTemplate.java | 126 +++--- .../data/neo4j/core/TemplateSupport.java | 83 +++- .../neo4j/core/convert/AdditionalTypes.java | 14 - .../neo4j/core/mapping/CypherGenerator.java | 159 +++++--- .../mapping/DefaultNeo4jEntityConverter.java | 44 +- .../mapping/DefaultNeo4jPersistentEntity.java | 9 +- .../neo4j/core/mapping/IdDescription.java | 16 +- .../neo4j/core/mapping/IdentitySupport.java | 84 ++-- .../core/mapping/Neo4jPersistentEntity.java | 9 + ...tedRelationshipProcessingStateMachine.java | 95 ++--- .../neo4j/core/mapping/NodeDescription.java | 1 + .../core/mapping/CypherGeneratorTest.java | 32 +- .../core/mapping/Neo4jMappingContextTest.java | 2 +- .../mapping/callback/IdPopulatorTest.java | 2 +- .../imperative/InheritanceMappingIT.java | 7 +- .../integration/imperative/Neo4jClientIT.java | 6 +- .../integration/imperative/RepositoryIT.java | 153 ++++--- .../AbstractElementIdTestBase.java | 69 ++++ .../ImperativeElementIdIT.java | 362 +++++++++++++++++ .../NodeWithGeneratedId1.java} | 40 +- .../pure_element_id/NodeWithGeneratedId2.java | 63 +++ .../pure_element_id/NodeWithGeneratedId3.java | 63 +++ .../pure_element_id/NodeWithGeneratedId4.java | 88 ++++ .../pure_element_id/ReactiveElementIdIT.java | 376 ++++++++++++++++++ .../issues/pure_element_id/RelWithProps.java | 64 +++ .../reactive/ReactiveNeo4jClientIT.java | 35 +- .../reactive/ReactiveRepositoryIT.java | 128 +++--- .../common/AltLikedByPersonRelationship.java | 8 + .../data/neo4j/test/TestIdentitySupport.java} | 25 +- 31 files changed, 1760 insertions(+), 583 deletions(-) create mode 100644 etc/migrate-to-element-id.yml create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/AbstractElementIdTestBase.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ImperativeElementIdIT.java rename src/{main/java/org/springframework/data/neo4j/core/schema/DefaultElementId.java => test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId1.java} (50%) create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId2.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId3.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId4.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ReactiveElementIdIT.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/RelWithProps.java rename src/{main/java/org/springframework/data/neo4j/core/schema/ElementId.java => test/java/org/springframework/data/neo4j/test/TestIdentitySupport.java} (64%) diff --git a/etc/migrate-to-element-id.yml b/etc/migrate-to-element-id.yml new file mode 100644 index 000000000..7d92261ff --- /dev/null +++ b/etc/migrate-to-element-id.yml @@ -0,0 +1,15 @@ + +# Run with +# ./mvnw org.openrewrite.maven:rewrite-maven-plugin:dryRun \ +# -Drewrite.checkstyleDetectionEnabled=false \ +# -Drewrite.configLocation=etc/migrate-to-element-id.yml \ +# -Drewrite.activeRecipes=sdn.elementId.rewriteIdCalls +--- +type: specs.openrewrite.org/v1beta/recipe +name: sdn.elementId.rewriteIdCalls +displayName: Change calls to Functions.id to Functions.elemenetId +recipeList: + - org.openrewrite.java.ChangeMethodName: + methodPattern: org.neo4j.cypherdsl.core.Functions id(..) + newMethodName: elementId + ignoreDefinition: true \ No newline at end of file diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java index f500c0542..77822c1f3 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java @@ -409,23 +409,21 @@ private T saveImpl(T instance, @Nullable Collection propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved); - if (entityMetaData.isUsingInternalIds()) { - propertyAccessor.setProperty(entityMetaData.getRequiredIdProperty(), internalId); - } + TemplateSupport.setGeneratedIdIfNecessary(entityMetaData, propertyAccessor, elementId, newOrUpdatedNode); TemplateSupport.updateVersionPropertyIfPossible(entityMetaData, propertyAccessor, newOrUpdatedNode.get()); if (stateMachine == null) { - stateMachine = new NestedRelationshipProcessingStateMachine(neo4jMappingContext, instance, internalId); + stateMachine = new NestedRelationshipProcessingStateMachine(neo4jMappingContext, instance, elementId); } - stateMachine.markValueAsProcessed(instance, internalId); + stateMachine.markEntityAsProcessed(instance, elementId); processRelations(entityMetaData, propertyAccessor, isEntityNew, stateMachine, binderFunction.filter); T bean = propertyAccessor.getBean(); - stateMachine.markValueAsProcessedAs(instance, bean); + stateMachine.markAsAliased(instance, bean); return bean; } @@ -510,21 +508,21 @@ class Tuple3 { binderFunction = TemplateSupport.createAndApplyPropertyFilter(pps, entityMetaData, binderFunction); List> entityList = entitiesToBeSaved.stream().map(h -> h.modifiedInstance).map(binderFunction) .collect(Collectors.toList()); - Map idToInternalIdMapping = neo4jClient + Map idToInternalIdMapping = neo4jClient .query(() -> renderer.render(cypherGenerator.prepareSaveOfMultipleInstancesOf(entityMetaData))) .bind(entityList).to(Constants.NAME_OF_ENTITY_LIST_PARAM) .fetchAs(Map.Entry.class) - .mappedBy((t, r) -> new AbstractMap.SimpleEntry<>(r.get(Constants.NAME_OF_ID), r.get(Constants.NAME_OF_INTERNAL_ID).asLong())) + .mappedBy((t, r) -> new AbstractMap.SimpleEntry<>(r.get(Constants.NAME_OF_ID), r.get(Constants.NAME_OF_ELEMENT_ID).asString())) .all() .stream() - .collect(Collectors.toMap(m -> (Value) m.getKey(), m -> (Long) m.getValue())); + .collect(Collectors.toMap(m -> (Value) m.getKey(), m -> (String) m.getValue())); // Save related return entitiesToBeSaved.stream().map(t -> { PersistentPropertyAccessor propertyAccessor = entityMetaData.getPropertyAccessor(t.modifiedInstance); Neo4jPersistentProperty idProperty = entityMetaData.getRequiredIdProperty(); Object id = convertIdValues(idProperty, propertyAccessor.getProperty(idProperty)); - Long internalId = idToInternalIdMapping.get(id); + String internalId = idToInternalIdMapping.get(id); return this.processRelations(entityMetaData, propertyAccessor, t.wasNew, new NestedRelationshipProcessingStateMachine(neo4jMappingContext, t.originalInstance, internalId), TemplateSupport.computeIncludePropertyPredicate(pps, entityMetaData)); }).collect(Collectors.toList()); } @@ -757,14 +755,14 @@ private T processNestedRelations( // This avoids the usage of cache but might have significant impact on overall performance if (!isParentObjectNew && !stateMachine.hasProcessedRelationship(fromId, relationshipDescription)) { - List knownRelationshipsIds = new ArrayList<>(); + List knownRelationshipsIds = new ArrayList<>(); if (idProperty != null) { for (Object relatedValueToStore : relatedValuesToStore) { if (relatedValueToStore == null) { continue; } - Long id = (Long) relationshipContext.getRelationshipPropertiesPropertyAccessor(relatedValueToStore).getProperty(idProperty); + Object id = relationshipContext.getRelationshipPropertiesPropertyAccessor(relatedValueToStore).getProperty(idProperty); if (id != null) { knownRelationshipsIds.add(id); } @@ -809,37 +807,30 @@ private T processNestedRelations( ? stateMachine.getProcessedAs(relatedObjectBeforeCallbacksApplied) : eventSupport.maybeCallBeforeBind(relatedObjectBeforeCallbacksApplied); - Long relatedInternalId; + String relatedInternalId; Entity savedEntity = null; // No need to save values if processed if (stateMachine.hasProcessedValue(relatedValueToStore)) { - relatedInternalId = stateMachine.getInternalId(relatedValueToStore); + relatedInternalId = stateMachine.getObjectId(relatedValueToStore); } else { savedEntity = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath); - relatedInternalId = IdentitySupport.getInternalId(savedEntity); - stateMachine.markValueAsProcessed(relatedValueToStore, relatedInternalId); + relatedInternalId = IdentitySupport.getElementId(savedEntity); + stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId); if (relatedValueToStore instanceof MappingSupport.RelationshipPropertiesWithEntityHolder) { Object entity = ((MappingSupport.RelationshipPropertiesWithEntityHolder) relatedValueToStore).getRelatedEntity(); - stateMachine.markValueAsProcessedAs(entity, relatedInternalId); + stateMachine.markAsAliased(entity, relatedInternalId); } } Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty(); PersistentPropertyAccessor targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject); - Object actualRelatedId = targetPropertyAccessor.getProperty(requiredIdProperty); - // if an internal id is used this must be set to link this entity in the next iteration - if (targetEntity.isUsingInternalIds()) { - if (relatedInternalId == null && actualRelatedId != null) { - relatedInternalId = (Long) targetPropertyAccessor.getProperty(requiredIdProperty); - } else if (actualRelatedId == null) { - targetPropertyAccessor.setProperty(requiredIdProperty, relatedInternalId); - } - } + Object possibleInternalLongId = targetPropertyAccessor.getProperty(requiredIdProperty); + relatedInternalId = TemplateSupport.retrieveOrSetRelatedId(targetEntity, targetPropertyAccessor, Optional.ofNullable(savedEntity), relatedInternalId); if (savedEntity != null) { TemplateSupport.updateVersionPropertyIfPossible(targetEntity, targetPropertyAccessor, savedEntity); } - stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean()); - stateMachine.markRelationshipAsProcessed(actualRelatedId == null ? relatedInternalId : actualRelatedId, + stateMachine.markAsAliased(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean()); + stateMachine.markRelationshipAsProcessed(possibleInternalLongId == null ? relatedInternalId : possibleInternalLongId, relationshipDescription.getRelationshipObverse()); Object idValue = idProperty != null @@ -854,42 +845,43 @@ private T processNestedRelations( boolean isNewRelationship = idValue == null; if (relationshipDescription.isDynamic()) { // create new dynamic relationship properties - if (relationshipDescription.hasRelationshipProperties() && isNewRelationship && idProperty != null) { - CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForSingleRelationship( - sourceEntity, relationshipDescription, relatedValueToStore, true); - - List row = Collections.singletonList(properties); - statementHolder = statementHolder.addProperty(Constants.NAME_OF_RELATIONSHIP_LIST_PARAM, row); - - Optional relationshipInternalId = neo4jClient.query(renderer.render(statementHolder.getStatement())) - .bind(convertIdValues(sourceEntity.getRequiredIdProperty(), fromId)) // - .to(Constants.FROM_ID_PARAMETER_NAME) // - .bind(relatedInternalId) // - .to(Constants.TO_ID_PARAMETER_NAME) // - .bind(idValue) // - .to(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM) // - .bindAll(statementHolder.getProperties()) - .fetchAs(Long.class) - .mappedBy(IdentitySupport::getInternalId) - .one(); - - assignIdToRelationshipProperties(relationshipContext, relatedValueToStore, idProperty, relationshipInternalId.get()); - } else { // plain (new or to update) dynamic relationship or dynamic relationships with properties to update - CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForSingleRelationship( - sourceEntity, relationshipDescription, relatedValueToStore, false); - - List row = Collections.singletonList(properties); - statementHolder = statementHolder.addProperty(Constants.NAME_OF_RELATIONSHIP_LIST_PARAM, row); - neo4jClient.query(renderer.render(statementHolder.getStatement())) - .bind(convertIdValues(sourceEntity.getRequiredIdProperty(), fromId)) // - .to(Constants.FROM_ID_PARAMETER_NAME) // - .bind(relatedInternalId) // - .to(Constants.TO_ID_PARAMETER_NAME) // - .bind(idValue) - .to(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM) // - .bindAll(statementHolder.getProperties()) - .run(); - } + if (relationshipDescription.hasRelationshipProperties() && isNewRelationship && idProperty != null) { + CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForSingleRelationship( + sourceEntity, relationshipDescription, relatedValueToStore, true); + + List row = Collections.singletonList(properties); + statementHolder = statementHolder.addProperty(Constants.NAME_OF_RELATIONSHIP_LIST_PARAM, row); + + Optional relationshipInternalId = neo4jClient.query(renderer.render(statementHolder.getStatement())) + .bind(convertIdValues(sourceEntity.getRequiredIdProperty(), fromId)) // + .to(Constants.FROM_ID_PARAMETER_NAME) // + .bind(relatedInternalId) // + .to(Constants.TO_ID_PARAMETER_NAME) // + .bind(idValue) // + .to(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM) // + .bindAll(statementHolder.getProperties()) + .fetchAs(Object.class) + .mappedBy((t, r) -> IdentitySupport.mapperForRelatedIdValues(idProperty).apply(r)) + .one(); + + assignIdToRelationshipProperties(relationshipContext, relatedValueToStore, idProperty, relationshipInternalId.orElseThrow()); + } else { // plain (new or to update) dynamic relationship or dynamic relationships with properties to update + + CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForSingleRelationship( + sourceEntity, relationshipDescription, relatedValueToStore, false); + + List row = Collections.singletonList(properties); + statementHolder = statementHolder.addProperty(Constants.NAME_OF_RELATIONSHIP_LIST_PARAM, row); + neo4jClient.query(renderer.render(statementHolder.getStatement())) + .bind(convertIdValues(sourceEntity.getRequiredIdProperty(), fromId)) // + .to(Constants.FROM_ID_PARAMETER_NAME) // + .bind(relatedInternalId) // + .to(Constants.TO_ID_PARAMETER_NAME) // + .bind(idValue) + .to(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM) // + .bindAll(statementHolder.getProperties()) + .run(); + } } else if (relationshipDescription.hasRelationshipProperties() && isNewRelationship && idProperty != null) { newRelationshipPropertiesRows.add(properties); newRelatedValuesToStore.add(relatedValueToStore); @@ -913,7 +905,6 @@ private T processNestedRelations( relationshipProperty.isDynamicAssociation(), relatedValueToStore, targetPropertyAccessor); - relationshipHandler.handle(relatedValueToStore, relatedObjectBeforeCallbacksApplied, potentiallyRecreatedNewRelatedObject); } // batch operations @@ -937,14 +928,15 @@ private T processNestedRelations( if (!newRelatedValuesToStore.isEmpty()) { CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForImperativeRelationshipsWithPropertiesBatch(true, sourceEntity, relationshipDescription, newRelatedValuesToStore, newRelationshipPropertiesRows); - - List all = new ArrayList<>(neo4jClient.query(renderer.render(statementHolder.getStatement())) + List all = new ArrayList<>(neo4jClient.query(renderer.render(statementHolder.getStatement())) .bindAll(statementHolder.getProperties()) - .fetchAs(Long.class).mappedBy(IdentitySupport::getInternalId).all()); + .fetchAs(Object.class) + .mappedBy((t, r) -> IdentitySupport.mapperForRelatedIdValues(idProperty).apply(r)) + .all()); // assign new ids for (int i = 0; i < all.size(); i++) { - Long aLong = all.get(i); - assignIdToRelationshipProperties(relationshipContext, newRelatedValuesToStore.get(i), idProperty, aLong); + Object anId = all.get(i); + assignIdToRelationshipProperties(relationshipContext, newRelatedValuesToStore.get(i), idProperty, anId); } } } @@ -957,7 +949,12 @@ private T processNestedRelations( return finalSubgraphRoot; } - private void assignIdToRelationshipProperties(NestedRelationshipContext relationshipContext, Object relatedValueToStore, Neo4jPersistentProperty idProperty, Long relationshipInternalId) { + private void assignIdToRelationshipProperties( + NestedRelationshipContext relationshipContext, + Object relatedValueToStore, + Neo4jPersistentProperty idProperty, + Object relationshipInternalId + ) { relationshipContext .getRelationshipPropertiesPropertyAccessor(relatedValueToStore) .setProperty(idProperty, relationshipInternalId); @@ -1171,12 +1168,12 @@ private NodesAndRelationshipsByIdStatementProvider createNodesAndRelationshipsBy Map usedParameters = new HashMap<>(parameters); usedParameters.putAll(rootNodesStatement.getCatalog().getParameters()); - final Collection rootNodeIds = new HashSet<>(neo4jClient + final Collection rootNodeIds = new HashSet<>(neo4jClient .query(renderer.render(rootNodesStatement)) .bindAll(usedParameters) .fetchAs(Value.class).mappedBy((t, r) -> r.get(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE)) .one() - .map(value -> value.asList(Value::asLong)) + .map(value -> value.asList(Value::asString)) .get()); if (rootNodeIds.isEmpty()) { @@ -1184,8 +1181,8 @@ private NodesAndRelationshipsByIdStatementProvider createNodesAndRelationshipsBy return NodesAndRelationshipsByIdStatementProvider.EMPTY; } // load first level relationships - final Set relationshipIds = new HashSet<>(); - final Set relatedNodeIds = new HashSet<>(); + final Set relationshipIds = new HashSet<>(); + final Set relatedNodeIds = new HashSet<>(); for (RelationshipDescription relationshipDescription : entityMetaData.getRelationshipsInHierarchy(queryFragments::includeField)) { @@ -1205,8 +1202,8 @@ private NodesAndRelationshipsByIdStatementProvider createNodesAndRelationshipsBy return new NodesAndRelationshipsByIdStatementProvider(rootNodeIds, relationshipIds, relatedNodeIds, queryFragments); } - private void iterateNextLevel(Collection nodeIds, RelationshipDescription sourceRelationshipDescription, Set relationshipIds, - Set relatedNodeIds, PropertyPathWalkStep currentPathStep) { + private void iterateNextLevel(Collection nodeIds, RelationshipDescription sourceRelationshipDescription, Set relationshipIds, + Set relatedNodeIds, PropertyPathWalkStep currentPathStep) { Neo4jPersistentEntity target = (Neo4jPersistentEntity) sourceRelationshipDescription.getTarget(); @@ -1233,7 +1230,7 @@ private void iterateNextLevel(Collection nodeIds, RelationshipDescription Statement statement = cypherGenerator .prepareMatchOf(target, relationshipDescription, null, - Functions.id(node).in(Cypher.parameter(Constants.NAME_OF_IDS))) + Functions.elementId(node).in(Cypher.parameter(Constants.NAME_OF_IDS))) .returning(cypherGenerator.createGenericReturnStatement()).build(); neo4jClient.query(renderer.render(statement)) @@ -1245,20 +1242,20 @@ private void iterateNextLevel(Collection nodeIds, RelationshipDescription } @NonNull - private Consumer> iterateAndMapNextLevel(Set relationshipIds, - Set relatedNodeIds, + private Consumer> iterateAndMapNextLevel(Set relationshipIds, + Set relatedNodeIds, RelationshipDescription relationshipDescription, PropertyPathWalkStep currentPathStep) { return record -> { @SuppressWarnings("unchecked") - List newRelationshipIds = (List) record.get(Constants.NAME_OF_SYNTHESIZED_RELATIONS); + List newRelationshipIds = (List) record.get(Constants.NAME_OF_SYNTHESIZED_RELATIONS); relationshipIds.addAll(newRelationshipIds); @SuppressWarnings("unchecked") - List newRelatedNodeIds = (List) record.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES); + List newRelatedNodeIds = (List) record.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES); - Set relatedIds = new HashSet<>(newRelatedNodeIds); + Set relatedIds = new HashSet<>(newRelatedNodeIds); // use this list to get down the road // 1. remove already visited ones; relatedIds.removeAll(relatedNodeIds); diff --git a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java index 8f42eb109..12cf223b9 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java @@ -36,6 +36,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; @@ -446,10 +447,11 @@ private Mono saveImpl(T instance, @Nullable Collection propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved); return idMono.doOnNext(newOrUpdatedNode -> { - IdentitySupport.updateInternalId(entityMetaData, propertyAccessor, newOrUpdatedNode); + var elementId = IdentitySupport.getElementId(newOrUpdatedNode); + TemplateSupport.setGeneratedIdIfNecessary(entityMetaData, propertyAccessor, elementId, Optional.of(newOrUpdatedNode)); TemplateSupport.updateVersionPropertyIfPossible(entityMetaData, propertyAccessor, newOrUpdatedNode); - finalStateMachine.markValueAsProcessed(instance, IdentitySupport.getInternalId(newOrUpdatedNode)); - }).map(IdentitySupport::getInternalId) + finalStateMachine.markEntityAsProcessed(instance, elementId); + }).map(IdentitySupport::getElementId) .flatMap(internalId -> processRelations(entityMetaData, propertyAccessor, isNewEntity, finalStateMachine, binderFunction.filter)); }); } @@ -582,15 +584,15 @@ private Flux saveAllImpl(Iterable instances, @Nullable Collection renderer.render(cypherGenerator.prepareSaveOfMultipleInstancesOf(entityMetaData))) .bind(boundedEntityList).to(Constants.NAME_OF_ENTITY_LIST_PARAM) .fetchAs(Tuple2.class) - .mappedBy((t, r) -> Tuples.of(r.get(Constants.NAME_OF_ID), r.get(Constants.NAME_OF_INTERNAL_ID).asLong())) + .mappedBy((t, r) -> Tuples.of(r.get(Constants.NAME_OF_ID), r.get(Constants.NAME_OF_ELEMENT_ID).asString())) .all() - .collectMap(m -> (Value) m.getT1(), m -> (Long) m.getT2()); + .collectMap(m -> (Value) m.getT1(), m -> (String) m.getT2()); }).flatMapMany(idToInternalIdMapping -> Flux.fromIterable(entitiesToBeSaved) .flatMap(t -> { PersistentPropertyAccessor propertyAccessor = entityMetaData.getPropertyAccessor(t.getT3()); Neo4jPersistentProperty idProperty = entityMetaData.getRequiredIdProperty(); Object id = convertIdValues(idProperty, propertyAccessor.getProperty(idProperty)); - Long internalId = idToInternalIdMapping.get(id); + String internalId = idToInternalIdMapping.get(id); return processRelations(entityMetaData, propertyAccessor, t.getT2(), new NestedRelationshipProcessingStateMachine(neo4jMappingContext, t.getT1(), internalId), TemplateSupport.computeIncludePropertyPredicate(pps, entityMetaData)); })) @@ -709,9 +711,9 @@ private Mono createNodesAndRelations return Mono.deferContextual(ctx -> { Class rootClass = entityMetaData.getUnderlyingClass(); - Set rootNodeIds = ctx.get("rootNodes"); - Set processedRelationshipIds = ctx.get("processedRelationships"); - Set processedNodeIds = ctx.get("processedNodes"); + Set rootNodeIds = ctx.get("rootNodes"); + Set processedRelationshipIds = ctx.get("processedRelationships"); + Set processedNodeIds = ctx.get("processedNodes"); return Flux.fromIterable(entityMetaData.getRelationshipsInHierarchy(queryFragments::includeField)) .flatMap(relationshipDescription -> { @@ -723,16 +725,19 @@ private Mono createNodesAndRelations usedParameters.putAll(statement.getCatalog().getParameters()); return neo4jClient.query(renderer.render(statement)) .bindAll(usedParameters) - .fetchAs(TupleOfLongsHolder.class) + .fetchAs(Tuple2.class) .mappedBy((t, r) -> { - Collection rootIds = r.get(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE).asList(Value::asLong); + Collection rootIds = r.get(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE).asList(Value::asString); rootNodeIds.addAll(rootIds); - Collection newRelationshipIds = r.get(Constants.NAME_OF_SYNTHESIZED_RELATIONS).asList(Value::asLong); - Collection newRelatedNodeIds = r.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES).asList(Value::asLong); - return TupleOfLongsHolder.with(Tuples.of(newRelationshipIds, newRelatedNodeIds)); + Collection newRelationshipIds = r.get(Constants.NAME_OF_SYNTHESIZED_RELATIONS).asList(Value::asString); + Collection newRelatedNodeIds = r.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES).asList(Value::asString); + return Tuples.of(newRelationshipIds, newRelatedNodeIds); }) .one() - .map(TupleOfLongsHolder::get) + .map((t) -> { + //noinspection unchecked + return (Tuple2, Collection>) t; + }) .expand(iterateAndMapNextLevel(relationshipDescription, queryFragments, rootClass, PropertyPathWalkStep.empty())); }) .then(Mono.fromSupplier(() -> new NodesAndRelationshipsByIdStatementProvider(rootNodeIds, processedRelationshipIds, processedNodeIds, queryFragments))); @@ -744,23 +749,7 @@ private Mono createNodesAndRelations } - static class TupleOfLongsHolder { - private final Tuple2, Collection> content; - - static TupleOfLongsHolder with(Tuple2, Collection> content) { - return new TupleOfLongsHolder(content); - } - - private TupleOfLongsHolder(Tuple2, Collection> content) { - this.content = content; - } - - Tuple2, Collection> get() { - return content; - } - } - - private Flux, Collection>> iterateNextLevel(Collection relatedNodeIds, + private Flux, Collection>> iterateNextLevel(Collection relatedNodeIds, RelationshipDescription sourceRelationshipDescription, QueryFragments queryFragments, Class rootClass, PropertyPathWalkStep currentPathStep) { @@ -786,43 +775,46 @@ private Flux, Collection>> iterateNextLevel(Collec Statement statement = cypherGenerator .prepareMatchOf(target, relDe, null, - Functions.id(node).in(Cypher.parameter(Constants.NAME_OF_ID))) + Functions.elementId(node).in(Cypher.parameter(Constants.NAME_OF_ID))) .returning(cypherGenerator.createGenericReturnStatement()).build(); return neo4jClient.query(renderer.render(statement)) .bindAll(Collections.singletonMap(Constants.NAME_OF_ID, relatedNodeIds)) - .fetchAs(TupleOfLongsHolder.class) + .fetchAs(Tuple2.class) .mappedBy((t, r) -> { - Collection newRelationshipIds = r.get(Constants.NAME_OF_SYNTHESIZED_RELATIONS).asList(Value::asLong); - Collection newRelatedNodeIds = r.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES).asList(Value::asLong); + Collection newRelationshipIds = r.get(Constants.NAME_OF_SYNTHESIZED_RELATIONS).asList(Value::asString); + Collection newRelatedNodeIds = r.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES).asList(Value::asString); - return TupleOfLongsHolder.with(Tuples.of(newRelationshipIds, newRelatedNodeIds)); + return Tuples.of(newRelationshipIds, newRelatedNodeIds); }) .one() - .map(TupleOfLongsHolder::get) + .map((t) -> { + //noinspection unchecked + return (Tuple2, Collection>) t; + }) .expand(object -> iterateAndMapNextLevel(relDe, queryFragments, rootClass, nextPathStep).apply(object)); }); } @NonNull - private Function, Collection>, - Publisher, Collection>>> iterateAndMapNextLevel( + private Function, Collection>, + Publisher, Collection>>> iterateAndMapNextLevel( RelationshipDescription relationshipDescription, QueryFragments queryFragments, Class rootClass, PropertyPathWalkStep currentPathStep) { return newRelationshipAndRelatedNodeIds -> Flux.deferContextual(ctx -> { - Set relationshipIds = ctx.get("processedRelationships"); - Set processedNodeIds = ctx.get("processedNodes"); + Set relationshipIds = ctx.get("processedRelationships"); + Set processedNodeIds = ctx.get("processedNodes"); - Collection newRelationshipIds = newRelationshipAndRelatedNodeIds.getT1(); - Set tmpProcessedRels = ConcurrentHashMap.newKeySet(newRelationshipIds.size()); + Collection newRelationshipIds = newRelationshipAndRelatedNodeIds.getT1(); + Set tmpProcessedRels = ConcurrentHashMap.newKeySet(newRelationshipIds.size()); tmpProcessedRels.addAll(newRelationshipIds); tmpProcessedRels.removeAll(relationshipIds); relationshipIds.addAll(newRelationshipIds); - Collection newRelatedNodeIds = newRelationshipAndRelatedNodeIds.getT2(); - Set tmpProcessedNodes = ConcurrentHashMap.newKeySet(newRelatedNodeIds.size()); + Collection newRelatedNodeIds = newRelationshipAndRelatedNodeIds.getT2(); + Set tmpProcessedNodes = ConcurrentHashMap.newKeySet(newRelatedNodeIds.size()); tmpProcessedNodes.addAll(newRelatedNodeIds); tmpProcessedNodes.removeAll(processedNodeIds); processedNodeIds.addAll(newRelatedNodeIds); @@ -904,14 +896,14 @@ private Mono processNestedRelations(Neo4jPersistentEntity sourceEntity // This avoids the usage of cache but might have significant impact on overall performance if (!isParentObjectNew && !stateMachine.hasProcessedRelationship(fromId, relationshipDescription)) { - List knownRelationshipsIds = new ArrayList<>(); + List knownRelationshipsIds = new ArrayList<>(); if (idProperty != null) { for (Object relatedValueToStore : relatedValuesToStore) { if (relatedValueToStore == null) { continue; } - Long id = (Long) relationshipContext + Object id = relationshipContext .getRelationshipPropertiesPropertyAccessor(relatedValueToStore) .getProperty(idProperty); if (id != null) { @@ -950,44 +942,38 @@ private Mono processNestedRelations(Neo4jPersistentEntity sourceEntity .flatMap(newRelatedObject -> { Neo4jPersistentEntity targetEntity = neo4jMappingContext.getRequiredPersistentEntity(relatedObjectBeforeCallbacksApplied.getClass()); - Mono, AtomicReference>> queryOrSave; + Mono, AtomicReference>> queryOrSave; if (stateMachine.hasProcessedValue(relatedValueToStore)) { - AtomicReference relatedInternalId = new AtomicReference<>(); - Long possibleValue = stateMachine.getInternalId(relatedValueToStore); + AtomicReference relatedInternalId = new AtomicReference<>(); + String possibleValue = stateMachine.getObjectId(relatedValueToStore); if (possibleValue != null) { relatedInternalId.set(possibleValue); } queryOrSave = Mono.just(Tuples.of(relatedInternalId, new AtomicReference<>())); } else { queryOrSave = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath) - .map(entity -> Tuples.of(new AtomicReference<>(IdentitySupport.getInternalId(entity)), new AtomicReference<>(entity))) - .doOnNext(entity -> { - stateMachine.markValueAsProcessed(relatedValueToStore, entity.getT1().get()); + .map(entity -> Tuples.of(new AtomicReference<>(IdentitySupport.getElementId(entity)), new AtomicReference<>(entity))) + .doOnNext(t -> { + var relatedInternalId = t.getT1().get(); + stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId); if (relatedValueToStore instanceof MappingSupport.RelationshipPropertiesWithEntityHolder) { - Object value = ((MappingSupport.RelationshipPropertiesWithEntityHolder) relatedValueToStore).getRelatedEntity(); - stateMachine.markValueAsProcessedAs(value, entity.getT1().get()); + Object entity = ((MappingSupport.RelationshipPropertiesWithEntityHolder) relatedValueToStore).getRelatedEntity(); + stateMachine.markAsAliased(entity, relatedInternalId); } }); } return queryOrSave.flatMap(idAndEntity -> { - Long relatedInternalId = idAndEntity.getT1().get(); + String relatedInternalId = idAndEntity.getT1().get(); Entity savedEntity = idAndEntity.getT2().get(); Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty(); PersistentPropertyAccessor targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject); - Object actualRelatedId = targetPropertyAccessor.getProperty(requiredIdProperty); - // if an internal id is used this must be set to link this entity in the next iteration - if (targetEntity.isUsingInternalIds()) { - if (relatedInternalId == null && actualRelatedId != null) { - relatedInternalId = (Long) targetPropertyAccessor.getProperty(requiredIdProperty); - } else if (actualRelatedId == null) { - targetPropertyAccessor.setProperty(requiredIdProperty, relatedInternalId); - } - } + Object possibleInternalLongId = targetPropertyAccessor.getProperty(requiredIdProperty); + relatedInternalId = TemplateSupport.retrieveOrSetRelatedId(targetEntity, targetPropertyAccessor, Optional.ofNullable(savedEntity), relatedInternalId); if (savedEntity != null) { TemplateSupport.updateVersionPropertyIfPossible(targetEntity, targetPropertyAccessor, savedEntity); } - stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean()); - stateMachine.markRelationshipAsProcessed(actualRelatedId == null ? relatedInternalId : actualRelatedId, + stateMachine.markAsAliased(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean()); + stateMachine.markRelationshipAsProcessed(possibleInternalLongId == null ? relatedInternalId : possibleInternalLongId, relationshipDescription.getRelationshipObverse()); Object idValue = idProperty != null @@ -1016,8 +1002,8 @@ private Mono processNestedRelations(Neo4jPersistentEntity sourceEntity .bind(idValue) // .to(Constants.NAME_OF_KNOWN_RELATIONSHIP_PARAM) // .bindAll(statementHolder.getProperties()) - .fetchAs(Long.class) - .mappedBy(IdentitySupport::getInternalId) + .fetchAs(Object.class) + .mappedBy((t, r) -> IdentitySupport.mapperForRelatedIdValues(idProperty).apply(r)) .one() .flatMap(relationshipInternalId -> { if (idProperty != null && isNewRelationship) { diff --git a/src/main/java/org/springframework/data/neo4j/core/TemplateSupport.java b/src/main/java/org/springframework/data/neo4j/core/TemplateSupport.java index 2073da0b3..e0679e444 100644 --- a/src/main/java/org/springframework/data/neo4j/core/TemplateSupport.java +++ b/src/main/java/org/springframework/data/neo4j/core/TemplateSupport.java @@ -22,6 +22,8 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.function.BiFunction; import java.util.function.BiPredicate; @@ -45,6 +47,7 @@ import org.springframework.data.neo4j.core.mapping.Constants; import org.springframework.data.neo4j.core.mapping.EntityInstanceWithSource; import org.springframework.data.neo4j.core.mapping.IdDescription; +import org.springframework.data.neo4j.core.mapping.IdentitySupport; import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext; import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity; import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty; @@ -179,10 +182,10 @@ static final class NodesAndRelationshipsByIdStatementProvider { final static NodesAndRelationshipsByIdStatementProvider EMPTY = new NodesAndRelationshipsByIdStatementProvider(Collections.emptySet(), Collections.emptySet(), Collections.emptySet(), new QueryFragments()); - private final Map> parameters = new HashMap<>(3); + private final Map> parameters = new HashMap<>(3); private final QueryFragments queryFragments; - NodesAndRelationshipsByIdStatementProvider(Collection rootNodeIds, Collection relationshipsIds, Collection relatedNodeIds, QueryFragments queryFragments) { + NodesAndRelationshipsByIdStatementProvider(Collection rootNodeIds, Collection relationshipsIds, Collection relatedNodeIds, QueryFragments queryFragments) { this.parameters.put(ROOT_NODE_IDS, rootNodeIds); this.parameters.put(RELATIONSHIP_IDS, relationshipsIds); @@ -207,13 +210,13 @@ Statement toStatement(NodeDescription nodeDescription) { Node relatedNodes = Cypher.anyNode(relatedNodeIds); Relationship relationships = Cypher.anyNode().relationshipBetween(Cypher.anyNode()).named(relationshipIds); return Cypher.match(rootNodes) - .where(Functions.id(rootNodes).in(Cypher.parameter(rootNodeIds))) + .where(Functions.elementId(rootNodes).in(Cypher.parameter(rootNodeIds))) .with(Functions.collect(rootNodes).as(Constants.NAME_OF_ROOT_NODE)) .optionalMatch(relationships) - .where(Functions.id(relationships).in(Cypher.parameter(relationshipIds))) + .where(Functions.elementId(relationships).in(Cypher.parameter(relationshipIds))) .with(Constants.NAME_OF_ROOT_NODE, Functions.collectDistinct(relationships).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS)) .optionalMatch(relatedNodes) - .where(Functions.id(relatedNodes).in(Cypher.parameter(relatedNodeIds))) + .where(Functions.elementId(relatedNodes).in(Cypher.parameter(relatedNodeIds))) .with( Constants.NAME_OF_ROOT_NODE, Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATIONS).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS), @@ -337,6 +340,76 @@ public Map apply(T t) { } } + /** + * Uses the given {@link PersistentPropertyAccessor propertyAccessor} to set the value of the generated id. + * + * @param entityMetaData The type information from SDN + * @param propertyAccessor An accessor tied to a concrete instance + * @param elementId The element id to store + * @param databaseEntity A fallback entity to retrieve the deprecated internal long id + * @param The type of the entity + */ + static void setGeneratedIdIfNecessary( + Neo4jPersistentEntity entityMetaData, + PersistentPropertyAccessor propertyAccessor, + String elementId, + Optional databaseEntity + ) { + if (!entityMetaData.isUsingInternalIds()) { + return; + } + var requiredIdProperty = entityMetaData.getRequiredIdProperty(); + var idPropertyType = requiredIdProperty.getType(); + if (entityMetaData.isUsingDeprecatedInternalId()) { + propertyAccessor.setProperty(requiredIdProperty, databaseEntity.map(IdentitySupport::getInternalId).orElseThrow()); + } else if (idPropertyType.equals(String.class)) { + propertyAccessor.setProperty(requiredIdProperty, elementId); + } else { + throw new IllegalArgumentException("Unsupported generated id property " + idPropertyType); + } + } + + /** + * Retrieves the object id for a related object if no has been found so far or updates the object with the id of a previously + * processed object. + * + * @param entityMetadata Needed for determining the type of ids + * @param propertyAccessor Bound to the currently processed entity + * @param databaseEntity Source for the old neo4j internal id + * @param relatedInternalId The element id or the string version of the old id + * @param The type of the entity + * @return The actual related internal id being used. + */ + static String retrieveOrSetRelatedId( + Neo4jPersistentEntity entityMetadata, + PersistentPropertyAccessor propertyAccessor, + Optional databaseEntity, + @Nullable String relatedInternalId + ) { + if (!entityMetadata.isUsingInternalIds()) { + return Objects.requireNonNull(relatedInternalId); + } + + var requiredIdProperty = entityMetadata.getRequiredIdProperty(); + var current = propertyAccessor.getProperty(requiredIdProperty); + + if (entityMetadata.isUsingDeprecatedInternalId()) { + if (relatedInternalId == null && current != null) { + relatedInternalId = current.toString(); + } else if (current == null) { + long internalId = databaseEntity.map(Entity::id).orElseThrow(); + propertyAccessor.setProperty(requiredIdProperty, internalId); + } + } else { + if (relatedInternalId == null && current != null) { + relatedInternalId = (String) current; + } else if (current == null) { + propertyAccessor.setProperty(requiredIdProperty, relatedInternalId); + } + } + return Objects.requireNonNull(relatedInternalId); + } + private TemplateSupport() { } } diff --git a/src/main/java/org/springframework/data/neo4j/core/convert/AdditionalTypes.java b/src/main/java/org/springframework/data/neo4j/core/convert/AdditionalTypes.java index 7f0173c73..5f3751698 100644 --- a/src/main/java/org/springframework/data/neo4j/core/convert/AdditionalTypes.java +++ b/src/main/java/org/springframework/data/neo4j/core/convert/AdditionalTypes.java @@ -51,7 +51,6 @@ import org.springframework.data.convert.ReadingConverter; import org.springframework.data.convert.WritingConverter; import org.springframework.data.mapping.MappingException; -import org.springframework.data.neo4j.core.schema.ElementId; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -104,24 +103,11 @@ final class AdditionalTypes { hlp.add(ConverterBuilder.reading(Value.class, Entity.class, Value::asEntity)); hlp.add(ConverterBuilder.reading(Value.class, Node.class, Value::asNode)); hlp.add(ConverterBuilder.reading(Value.class, Relationship.class, Value::asRelationship)); - hlp.add(ConverterBuilder.reading(Value.class, ElementId.class, AdditionalTypes::asElementId).andWriting(AdditionalTypes::value)); hlp.add(ConverterBuilder.reading(Value.class, Map.class, Value::asMap).andWriting(AdditionalTypes::value)); CONVERTERS = Collections.unmodifiableList(hlp); } - static ElementId asElementId(Value value) { - return ElementId.of(value.asString()); - } - - static Value value(ElementId elementId) { - if (elementId == null) { - return Values.NULL; - } - - return Values.value(elementId.value()); - } - static Value value(Map map) { return Values.value(map); } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/CypherGenerator.java b/src/main/java/org/springframework/data/neo4j/core/mapping/CypherGenerator.java index 1d7a982e0..adef4c77f 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/CypherGenerator.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/CypherGenerator.java @@ -30,6 +30,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.function.Function; import java.util.function.Predicate; import java.util.function.UnaryOperator; import java.util.regex.Pattern; @@ -41,7 +42,6 @@ import org.neo4j.cypherdsl.core.Conditions; import org.neo4j.cypherdsl.core.Cypher; import org.neo4j.cypherdsl.core.Expression; -import org.neo4j.cypherdsl.core.FunctionInvocation; import org.neo4j.cypherdsl.core.Functions; import org.neo4j.cypherdsl.core.IdentifiableElement; import org.neo4j.cypherdsl.core.MapProjection; @@ -117,7 +117,9 @@ public StatementBuilder.OrderableOngoingReadingAndWith prepareMatchOf(NodeDescri List expressions = new ArrayList<>(); expressions.add(rootNode.getRequiredSymbolicName()); - expressions.add(Functions.id(rootNode).as(Constants.NAME_OF_INTERNAL_ID)); + if (nodeDescription instanceof Neo4jPersistentEntity entity && entity.isUsingDeprecatedInternalId()) { + expressions.add(Functions.id(rootNode).as(Constants.NAME_OF_INTERNAL_ID)); + } expressions.add(Functions.elementId(rootNode).as(Constants.NAME_OF_ELEMENT_ID)); return match(rootNode).where(conditionOrNoCondition(condition)).with(expressions.toArray(IdentifiableElement[]::new)); @@ -131,7 +133,7 @@ public StatementBuilder.OngoingReading prepareMatchOf(NodeDescription nodeDes StatementBuilder.OngoingReadingWithoutWhere match = prepareMatchOfRootNode(rootNode, initialMatchOn); List expressions = new ArrayList<>(); - expressions.add(Functions.collect(Functions.id(rootNode)).as(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE)); + expressions.add(Functions.collect(Functions.elementId(rootNode)).as(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE)); return match .where(conditionOrNoCondition(condition)) @@ -168,9 +170,9 @@ public StatementBuilder.OngoingReading prepareMatchOf(NodeDescription nodeDes relationship = relationship.named(Constants.NAME_OF_SYNTHESIZED_RELATIONS); List expressions = new ArrayList<>(); - expressions.add(Functions.collect(Functions.id(rootNode)).as(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE)); - expressions.add(Functions.collect(Functions.id(targetNode)).as(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)); - expressions.add(Functions.collect(Functions.id(relationship)).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS)); + expressions.add(Functions.collect(Functions.elementId(rootNode)).as(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE)); + expressions.add(Functions.collect(Functions.elementId(targetNode)).as(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)); + expressions.add(Functions.collect(Functions.elementId(relationship)).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS)); return match .where(conditionOrNoCondition(condition)) @@ -341,11 +343,14 @@ public Statement prepareSaveOf(NodeDescription nodeDescription, Statement createIfNew; Statement updateIfExists; - if (((Neo4jPersistentEntity) nodeDescription).hasVersionProperty()) { - Property versionProperty = rootNode.property(((Neo4jPersistentEntity) nodeDescription).getRequiredVersionProperty().getName()); + var neo4jPersistentEntity = (Neo4jPersistentEntity) nodeDescription; + var nodeIdFunction = getNodeIdFunction(neo4jPersistentEntity); + + if (neo4jPersistentEntity.hasVersionProperty()) { + Property versionProperty = rootNode.property(neo4jPersistentEntity.getRequiredVersionProperty().getName()); createIfNew = updateDecorator.apply(optionalMatch(possibleExistingNode) - .where(Functions.id(possibleExistingNode).isEqualTo(idParameter)) + .where(nodeIdFunction.apply(possibleExistingNode).isEqualTo(idParameter)) .with(possibleExistingNode) .where(possibleExistingNode.isNull()) .create(rootNode.withProperties(versionProperty, literalOf(0))) @@ -355,7 +360,7 @@ public Statement prepareSaveOf(NodeDescription nodeDescription, .build(); updateIfExists = updateDecorator.apply(match(rootNode) - .where(Functions.id(rootNode).isEqualTo(idParameter)) + .where(nodeIdFunction.apply(rootNode).isEqualTo(idParameter)) .and(versionProperty.isEqualTo(parameter(Constants.NAME_OF_VERSION_PARAM))) // Initial check .set(versionProperty.to(versionProperty.add(literalOf(1)))) // Acquire lock .with(rootNode) @@ -365,12 +370,12 @@ public Statement prepareSaveOf(NodeDescription nodeDescription, .returning(rootNode).build(); } else { createIfNew = updateDecorator - .apply(optionalMatch(possibleExistingNode).where(Functions.id(possibleExistingNode).isEqualTo(idParameter)) + .apply(optionalMatch(possibleExistingNode).where(nodeIdFunction.apply(possibleExistingNode).isEqualTo(idParameter)) .with(possibleExistingNode).where(possibleExistingNode.isNull()).create(rootNode) .set(rootNode, parameter(Constants.NAME_OF_PROPERTIES_PARAM))) .returning(rootNode).build(); - updateIfExists = updateDecorator.apply(match(rootNode).where(Functions.id(rootNode).isEqualTo(idParameter)) + updateIfExists = updateDecorator.apply(match(rootNode).where(nodeIdFunction.apply(rootNode).isEqualTo(idParameter)) .mutate(rootNode, parameter(Constants.NAME_OF_PROPERTIES_PARAM))).returning(rootNode).build(); } @@ -391,15 +396,18 @@ public Statement prepareSaveOfMultipleInstancesOf(NodeDescription nodeDescrip String nameOfIdProperty = idDescription.getOptionalGraphPropertyName() .orElseThrow(() -> new MappingException("External id does not correspond to a graph property")); + List expressions = new ArrayList<>(); + if (nodeDescription instanceof Neo4jPersistentEntity entity && entity.isUsingDeprecatedInternalId()) { + Functions.id(rootNode).as(Constants.NAME_OF_INTERNAL_ID); + } + expressions.add(Functions.elementId(rootNode).as(Constants.NAME_OF_ELEMENT_ID)); + expressions.add(rootNode.property(nameOfIdProperty).as(Constants.NAME_OF_ID)); + String row = "entity"; return Cypher.unwind(parameter(Constants.NAME_OF_ENTITY_LIST_PARAM)).as(row) .merge(rootNode.withProperties(nameOfIdProperty, Cypher.property(row, Constants.NAME_OF_ID))) .mutate(rootNode, Cypher.property(row, Constants.NAME_OF_PROPERTIES_PARAM)) - .returning( - Functions.id(rootNode).as(Constants.NAME_OF_INTERNAL_ID), - Functions.elementId(rootNode).as(Constants.NAME_OF_ELEMENT_ID), - rootNode.property(nameOfIdProperty).as(Constants.NAME_OF_ID) - ) + .returning(expressions) .build(); } @@ -412,7 +420,6 @@ public Statement prepareSaveOfRelationship(Neo4jPersistentEntity neo4jPersist .named(START_NODE_NAME); final Node endNode = anyNode(END_NODE_NAME); - String idPropertyName = neo4jPersistentEntity.getRequiredIdProperty().getPropertyName(); Parameter idParameter = parameter(Constants.FROM_ID_PARAMETER_NAME); String type = relationship.isDynamic() ? dynamicRelationshipType : relationship.getType(); @@ -420,17 +427,45 @@ public Statement prepareSaveOfRelationship(Neo4jPersistentEntity neo4jPersist startNode.relationshipTo(endNode, type) : startNode.relationshipFrom(endNode, type)).named(RELATIONSHIP_NAME); + var startNodeIdFunction = getNodeIdFunction(neo4jPersistentEntity); return match(startNode) - .where(neo4jPersistentEntity.isUsingInternalIds() ? Functions.id(startNode).isEqualTo(idParameter) - : startNode.property(idPropertyName).isEqualTo(idParameter)) - .match(endNode).where(Functions.id(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME))) + .where(startNodeIdFunction.apply(startNode).isEqualTo(idParameter)) + .match(endNode).where(Functions.elementId(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME))) .merge(relationshipFragment) - .returning( - Functions.id(relationshipFragment).as(Constants.NAME_OF_INTERNAL_ID), - Functions.elementId(relationshipFragment).as(Constants.NAME_OF_ELEMENT_ID) - ) + .returning(getReturnedIdExpressionsForRelationship(relationship, relationshipFragment)) .build(); } + + private static Function getNodeIdFunction(Neo4jPersistentEntity entity) { + + Function startNodeIdFunction; + var idProperty = entity.getRequiredIdProperty(); + if (entity.isUsingInternalIds()) { + if (entity.isUsingDeprecatedInternalId()) { + startNodeIdFunction = Functions::id; + } else { + startNodeIdFunction = Functions::elementId; + } + } else { + startNodeIdFunction = node -> node.property(idProperty.getPropertyName()); + } + return startNodeIdFunction; + } + + private static Function getRelationshipIdFunction(RelationshipDescription relationshipDescription) { + + Function result = Functions::elementId; + if (relationshipDescription.hasRelationshipProperties()) { + Neo4jPersistentEntity entity = (Neo4jPersistentEntity) relationshipDescription.getRelationshipPropertiesEntity(); + if (entity != null && entity.isUsingDeprecatedInternalId()) { + result = Functions::id; + } else { + result = Functions::elementId; + } + } + return result; + } + @NonNull public Statement prepareSaveOfRelationships(Neo4jPersistentEntity neo4jPersistentEntity, RelationshipDescription relationship, @Nullable String dynamicRelationshipType) { @@ -441,7 +476,6 @@ public Statement prepareSaveOfRelationships(Neo4jPersistentEntity neo4jPersis .named(START_NODE_NAME); final Node endNode = anyNode(END_NODE_NAME); - String idPropertyName = neo4jPersistentEntity.getRequiredIdProperty().getPropertyName(); String type = relationship.isDynamic() ? dynamicRelationshipType : relationship.getType(); Relationship relationshipFragment = (relationship.isOutgoing() ? @@ -453,15 +487,10 @@ public Statement prepareSaveOfRelationships(Neo4jPersistentEntity neo4jPersis return Cypher.unwind(parameter(Constants.NAME_OF_RELATIONSHIP_LIST_PARAM)).as(row) .with(row) .match(startNode) - .where(neo4jPersistentEntity.isUsingInternalIds() - ? internalId(startNode).isEqualTo(idProperty) - : startNode.property(idPropertyName).isEqualTo(idProperty)) - .match(endNode).where(internalId(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME))) + .where(getNodeIdFunction(neo4jPersistentEntity).apply(startNode).isEqualTo(idProperty)) + .match(endNode).where(Functions.elementId(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME))) .merge(relationshipFragment) - .returning( - Functions.id(relationshipFragment).as(Constants.NAME_OF_INTERNAL_ID), - Functions.elementId(relationshipFragment).as(Constants.NAME_OF_ELEMENT_ID) - ) + .returning(getReturnedIdExpressionsForRelationship(relationship, relationshipFragment)) .build(); } @@ -476,7 +505,6 @@ public Statement prepareSaveOfRelationshipWithProperties(Neo4jPersistentEntity idParameter = parameter(Constants.FROM_ID_PARAMETER_NAME); Parameter relationshipProperties = parameter(Constants.NAME_OF_PROPERTIES_PARAM); @@ -488,21 +516,20 @@ public Statement prepareSaveOfRelationshipWithProperties(Neo4jPersistentEntity getReturnedIdExpressionsForRelationship(RelationshipDescription relationship, Relationship relationshipFragment) { + List result = new ArrayList<>(); + if (relationship.hasRelationshipProperties() && relationship.getRelationshipPropertiesEntity() instanceof Neo4jPersistentEntity entity && entity.isUsingDeprecatedInternalId()) { + result.add(Functions.id(relationshipFragment).as(Constants.NAME_OF_INTERNAL_ID)); + } + result.add(Functions.elementId(relationshipFragment).as(Constants.NAME_OF_ELEMENT_ID)); + return result; + } + @NonNull public Statement prepareDeleteOf( Neo4jPersistentEntity neo4jPersistentEntity, @@ -565,7 +601,6 @@ public Statement prepareDeleteOf( NodeDescription target = relationshipDescription.getTarget(); Node endNode = node(target.getPrimaryLabel(), target.getAdditionalLabels()); - String idPropertyName = neo4jPersistentEntity.getRequiredIdProperty().getPropertyName(); boolean outgoing = relationshipDescription.isOutgoing(); String relationshipType = relationshipDescription.isDynamic() ? null : relationshipDescription.getType(); @@ -577,9 +612,8 @@ public Statement prepareDeleteOf( Parameter idParameter = parameter(Constants.FROM_ID_PARAMETER_NAME); return match(relationship) - .where(neo4jPersistentEntity.isUsingInternalIds() ? Functions.id(startNode).isEqualTo(idParameter) - : startNode.property(idPropertyName).isEqualTo(idParameter)) - .and(Functions.id(relationship).in(Cypher.parameter(Constants.NAME_OF_KNOWN_RELATIONSHIPS_PARAM)).not()) + .where(getNodeIdFunction(neo4jPersistentEntity).apply(startNode).isEqualTo(idParameter)) + .and(getRelationshipIdFunction(relationshipDescription).apply(relationship).in(Cypher.parameter(Constants.NAME_OF_KNOWN_RELATIONSHIPS_PARAM)).not()) .delete(relationship.getRequiredSymbolicName()) .build(); } @@ -677,11 +711,6 @@ public Collection createGenericReturnStatement() { return returnExpressions; } - @SuppressWarnings("deprecation") - private FunctionInvocation internalId(Node node) { - return node.internalId(); - } - private MapProjection projectPropertiesAndRelationships(PropertyFilter.RelaxedPropertyPath parentPath, Neo4jPersistentEntity nodeDescription, SymbolicName nodeName, Predicate includedProperties, @Nullable RelationshipDescription relationshipDescription, List processedRelationships) { @@ -749,8 +778,10 @@ private List projectNodeProperties(PropertyFilter.RelaxedPropertyPath pa nodePropertiesProjection.add(Constants.NAME_OF_LABELS); nodePropertiesProjection.add(Functions.labels(node)); - nodePropertiesProjection.add(Constants.NAME_OF_INTERNAL_ID); - nodePropertiesProjection.add(Functions.id(node)); + if (nodeDescription instanceof Neo4jPersistentEntity entity && entity.isUsingDeprecatedInternalId()) { + nodePropertiesProjection.add(Constants.NAME_OF_INTERNAL_ID); + nodePropertiesProjection.add(Functions.id(node)); + } nodePropertiesProjection.add(Constants.NAME_OF_ELEMENT_ID); nodePropertiesProjection.add(Functions.elementId(node)); return nodePropertiesProjection; 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 9b3e4ba97..960a87e8c 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 @@ -54,7 +54,6 @@ import org.springframework.data.mapping.model.ParameterValueProvider; import org.springframework.data.neo4j.core.convert.Neo4jConversionService; import org.springframework.data.neo4j.core.mapping.callback.EventSupport; -import org.springframework.data.neo4j.core.schema.ElementId; import org.springframework.data.neo4j.core.schema.TargetNode; import org.springframework.data.util.ReflectionUtils; import org.springframework.data.util.TypeInformation; @@ -148,7 +147,7 @@ private MapAccessor determineQueryRoot(MapAccessor mapAccessor, @Nullable Ne Node node = value.asNode(); if (primaryLabels.stream().anyMatch(node::hasLabel)) { // it has a matching label // We haven't seen this node yet, so we take it - if (knownObjects.getObject("N" + IdentitySupport.getInternalId(node)) == null) { + if (knownObjects.getObject("N" + IdentitySupport.getElementId(node)) == null) { matchingNodes.add(node); } else { seenMatchingNodes.add(node); @@ -281,7 +280,7 @@ private static MapAccessor mergeRootNodeWithRecord(Node node, MapAccessor record Map mergedAttributes = new HashMap<>(node.size() + record.size() + 1); mergedAttributes.put(Constants.NAME_OF_INTERNAL_ID, IdentitySupport.getInternalId(node)); - mergedAttributes.put(Constants.NAME_OF_ELEMENT_ID, ElementId.of(node).value()); + mergedAttributes.put(Constants.NAME_OF_ELEMENT_ID, node.elementId()); mergedAttributes.put(Constants.NAME_OF_LABELS, node.labels()); mergedAttributes.putAll(node.asMap(Function.identity())); mergedAttributes.putAll(record.asMap(Function.identity())); @@ -310,7 +309,7 @@ private ET map(MapAccessor queryResult, Neo4jPersistentEntity nodeDescr // losing the central access. The behaviour of knowObjects should take different sources of ids into account, // as relationships and nodes might have overlapping values String direction = relationshipDescription != null ? relationshipDescription.getDirection().name() : null; - String internalId = IdentitySupport.getInternalId(queryResult, direction); + String internalId = IdentitySupport.getPrefixedElementId(queryResult, direction); Supplier mappedObjectSupplier = () -> { knownObjects.setInCreation(internalId); @@ -656,11 +655,18 @@ private Optional createInstanceOfRelationships(Neo4jPersistentProperty p List relationshipsAndProperties = new ArrayList<>(); if (Values.NULL.equals(list)) { - Long sourceNodeId = IdentitySupport.getInternalId(values); - //Long sourceNodeId = getInternalIdAsLong(values); - - Function sourceIdSelector = relationshipDescription.isIncoming() ? IdentitySupport::getEndId : IdentitySupport::getStartId; - Function targetIdSelector = relationshipDescription.isIncoming() ? IdentitySupport::getStartId : IdentitySupport::getEndId; + String sourceNodeId; + Function sourceIdSelector; + Function targetIdSelector = relationshipDescription.isIncoming() ? Relationship::startNodeElementId : Relationship::endNodeElementId; + if (IdentitySupport.getElementId(values) == null) { + // this can happen when someone used dto mapping and added the "classical" approach + sourceNodeId = Optional.ofNullable(IdentitySupport.getInternalId(values)).map(l -> Long.toString(l)).orElseThrow(); + Function hlp = relationshipDescription.isIncoming() ? Relationship::endNodeId : Relationship::startNodeId; + sourceIdSelector = hlp.andThen(l -> Long.toString(l)); + } else { + sourceNodeId = IdentitySupport.getElementId(values); + sourceIdSelector = relationshipDescription.isIncoming() ? Relationship::endNodeElementId : Relationship::startNodeElementId; + } // Retrieve all matching relationships from the result's list(s) Collection allMatchingTypeRelationshipsInResult = @@ -671,21 +677,21 @@ private Optional createInstanceOfRelationships(Neo4jPersistentProperty p Collection allNodesWithMatchingLabelInResult = extractMatchingNodes(nodesFromResult, targetLabel); for (Node possibleValueNode : allNodesWithMatchingLabelInResult) { - long targetNodeId = IdentitySupport.getInternalId(possibleValueNode); + String targetNodeId = IdentitySupport.getElementId(possibleValueNode); Neo4jPersistentEntity concreteTargetNodeDescription = getMostConcreteTargetNodeDescription(genericTargetNodeDescription, possibleValueNode); Set relationshipsProcessed = new HashSet<>(); for (Relationship possibleRelationship : allMatchingTypeRelationshipsInResult) { - if (targetIdSelector.apply(possibleRelationship) == targetNodeId) { + if (targetIdSelector.apply(possibleRelationship).equals(targetNodeId)) { // Reduce the amount of relationships in the candidate list. // If this relationship got processed twice (OUTGOING, INCOMING), it is never needed again // and therefor should not be in the list. // Otherwise, for highly linked data it could potentially cause a StackOverflowError. String direction = relationshipDescription.getDirection().name(); - if (knownObjects.hasProcessedRelationshipCompletely("R" + direction + IdentitySupport.getInternalId(possibleRelationship))) { + if (knownObjects.hasProcessedRelationshipCompletely("R" + direction + IdentitySupport.getElementId(possibleRelationship))) { relationshipsFromResult.remove(possibleRelationship); } // If the target is the same(equal) node, get the related object from the cache. @@ -711,7 +717,7 @@ private Optional createInstanceOfRelationships(Neo4jPersistentProperty p if (fetchMore) { relationshipProperties = map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult); } else { - Object objectFromStore = knownObjects.getObject(IdentitySupport.getInternalId(possibleRelationship, relationshipDescription.getDirection().name())); + Object objectFromStore = knownObjects.getObject(IdentitySupport.getPrefixedElementId(possibleRelationship, relationshipDescription.getDirection().name())); relationshipProperties = objectFromStore != null ? objectFromStore : map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult); @@ -736,7 +742,7 @@ private Optional createInstanceOfRelationships(Neo4jPersistentProperty p if (fetchMore) { valueEntry = map(relatedEntity, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult); } else { - Object objectFromStore = knownObjects.getObject(IdentitySupport.getInternalId(relatedEntity, null)); + Object objectFromStore = knownObjects.getObject(IdentitySupport.getPrefixedElementId(relatedEntity, null)); valueEntry = objectFromStore != null ? objectFromStore : map(relatedEntity, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult); @@ -754,7 +760,7 @@ private Optional createInstanceOfRelationships(Neo4jPersistentProperty p if (fetchMore) { relationshipProperties = map(relatedEntityRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult); } else { - Object objectFromStore = knownObjects.getObject(IdentitySupport.getInternalId(relatedEntityRelationship, relationshipDescription.getDirection().name())); + Object objectFromStore = knownObjects.getObject(IdentitySupport.getPrefixedElementId(relatedEntityRelationship, relationshipDescription.getDirection().name())); relationshipProperties = objectFromStore != null ? objectFromStore : map(relatedEntityRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult); @@ -832,13 +838,15 @@ private Collection extractRelationships(MapAccessor allValues) { .filter(this.relationshipType::isTypeOf) .map(Value::asRelationship) .forEach(allRelationshipsInResult::add); - return allRelationshipsInResult; } private static Value extractValueOf(Neo4jPersistentProperty property, MapAccessor propertyContainer) { if (property.isInternalIdProperty()) { - return Values.value(IdentitySupport.getInternalId(propertyContainer)); + if (Neo4jPersistentEntity.DEPRECATED_GENERATED_ID_TYPES.contains(property.getType())) { + return Values.value(IdentitySupport.getInternalId(propertyContainer)); + } + return Values.value(IdentitySupport.getElementId(propertyContainer)); } else if (property.isComposite()) { String prefix = property.computePrefixWithDelimiter(); @@ -924,7 +932,7 @@ private boolean containsNode(Node node) { try { read.lock(); - return internalIdStore.containsKey(IdentitySupport.getInternalId(node)); + return internalIdStore.containsKey(IdentitySupport.getElementId(node)); } finally { read.unlock(); } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java index 4f66ec0e0..d7964d3fa 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java @@ -38,7 +38,6 @@ import org.springframework.data.mapping.Association; import org.springframework.data.mapping.model.BasicPersistentEntity; import org.springframework.data.neo4j.core.schema.DynamicLabels; -import org.springframework.data.neo4j.core.schema.ElementId; import org.springframework.data.neo4j.core.schema.GeneratedValue; import org.springframework.data.neo4j.core.schema.IdGenerator; import org.springframework.data.neo4j.core.schema.Node; @@ -62,8 +61,7 @@ final class DefaultNeo4jPersistentEntity extends BasicPersistentEntity implements Neo4jPersistentEntity { - private static final Set> DEPRECATED_GENERATED_ID_TYPES = Set.of(Long.class, long.class); - private static final Set> VALID_GENERATED_ID_TYPES = Stream.concat(Stream.of(ElementId.class), DEPRECATED_GENERATED_ID_TYPES.stream()).collect(Collectors.toUnmodifiableSet()); + private static final Set> VALID_GENERATED_ID_TYPES = Stream.concat(Stream.of(String.class), DEPRECATED_GENERATED_ID_TYPES.stream()).collect(Collectors.toUnmodifiableSet()); private static final LogAccessor log = new LogAccessor(LogFactory.getLog(Neo4jPersistentEntity.class)); /** @@ -444,7 +442,8 @@ private IdDescription computeIdDescription() { "Internally generated ids can only be assigned to one of " + VALID_GENERATED_ID_TYPES); } - if (DEPRECATED_GENERATED_ID_TYPES.contains(idProperty.getActualType())) { + var isDeprecated = DEPRECATED_GENERATED_ID_TYPES.contains(idProperty.getActualType()); + if (isDeprecated) { Supplier messageSupplier = () -> String.format("" + "The entity %s is using a Long value for storing internally generated Neo4j ids. " + "The Neo4j internal Long Ids are deprecated, please consider using an external ID generator.", @@ -452,7 +451,7 @@ private IdDescription computeIdDescription() { log.warn(messageSupplier); } - return IdDescription.forInternallyGeneratedIds(Constants.NAME_OF_TYPED_ROOT_NODE.apply(this)); + return IdDescription.forInternallyGeneratedIds(Constants.NAME_OF_TYPED_ROOT_NODE.apply(this), isDeprecated); } // Externally generated ids. diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/IdDescription.java b/src/main/java/org/springframework/data/neo4j/core/mapping/IdDescription.java index 39fc0527e..bc2c96041 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/IdDescription.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/IdDescription.java @@ -58,11 +58,15 @@ public final class IdDescription { public static IdDescription forAssignedIds(SymbolicName symbolicName, String graphPropertyName) { Assert.notNull(graphPropertyName, "Graph property name is required"); - return new IdDescription(symbolicName, null, null, graphPropertyName); + return new IdDescription(symbolicName, null, null, graphPropertyName, false); } public static IdDescription forInternallyGeneratedIds(SymbolicName symbolicName) { - return new IdDescription(symbolicName, GeneratedValue.InternalIdGenerator.class, null, null); + return forInternallyGeneratedIds(symbolicName, false); + } + + public static IdDescription forInternallyGeneratedIds(SymbolicName symbolicName, boolean isDeprecated) { + return new IdDescription(symbolicName, GeneratedValue.InternalIdGenerator.class, null, null, isDeprecated); } public static IdDescription forExternallyGeneratedIds(SymbolicName symbolicName, @@ -73,18 +77,18 @@ public static IdDescription forExternallyGeneratedIds(SymbolicName symbolicName, try { Assert.hasText(idGeneratorRef, "Reference to an ID generator has precedence"); - return new IdDescription(symbolicName, null, idGeneratorRef, graphPropertyName); + return new IdDescription(symbolicName, null, idGeneratorRef, graphPropertyName, false); } catch (IllegalArgumentException e) { Assert.notNull(idGeneratorClass, "Class of id generator is required"); Assert.isTrue(idGeneratorClass != GeneratedValue.InternalIdGenerator.class, "Cannot use InternalIdGenerator for externally generated ids"); - return new IdDescription(symbolicName, idGeneratorClass, null, graphPropertyName); + return new IdDescription(symbolicName, idGeneratorClass, null, graphPropertyName, false); } } private IdDescription(SymbolicName symbolicName, @Nullable Class> idGeneratorClass, - @Nullable String idGeneratorRef, @Nullable String graphPropertyName) { + @Nullable String idGeneratorRef, @Nullable String graphPropertyName, boolean isDeprecated) { this.idGeneratorClass = idGeneratorClass; this.idGeneratorRef = idGeneratorRef != null && idGeneratorRef.isEmpty() ? null : idGeneratorRef; @@ -93,7 +97,7 @@ private IdDescription(SymbolicName symbolicName, @Nullable Class { final Node rootNode = Cypher.anyNode(symbolicName); if (this.isInternallyGeneratedId()) { - return Functions.id(rootNode); + return isDeprecated ? Functions.id(rootNode) : Functions.elementId(rootNode); } else { return this.getOptionalGraphPropertyName() .map(propertyName -> Cypher.property(symbolicName, propertyName)).get(); diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/IdentitySupport.java b/src/main/java/org/springframework/data/neo4j/core/mapping/IdentitySupport.java index 31cac5e47..ee8650a4b 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/IdentitySupport.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/IdentitySupport.java @@ -17,14 +17,13 @@ import static org.apiguardian.api.API.Status.INTERNAL; +import java.util.function.Function; + import org.apiguardian.api.API; -import org.neo4j.driver.Record; import org.neo4j.driver.types.Entity; import org.neo4j.driver.types.MapAccessor; import org.neo4j.driver.types.Node; import org.neo4j.driver.types.Relationship; -import org.neo4j.driver.types.TypeSystem; -import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; @@ -32,7 +31,7 @@ * This class is not part of any public API and will be changed without further notice as needed. It's * primary goal is to mitigate the changes in Neo4j5, which introduces the notion of an {@literal element id} for both nodes * and relationships while deprecating {@literal id} at the same time. The identity support allows to isolate our calls - * deprecated API in one central place and will exists for SDN 7 only to make SDN 7 work with both Neo4j 4.4 and Neo4j 5.x. + * deprecated API in one central place and will exist for SDN 7 only to make SDN 7 work with both Neo4j 4.4 and Neo4j 5.x. * * @author Michael J. Simons * @soundtrack Buckethead - SIGIL Soundtrack @@ -44,31 +43,12 @@ public final class IdentitySupport { private IdentitySupport() { } - /** - * Updates the internal id of a given client side entity from a server side entity using a property accessor. - * Does nothing if the local entity does not use internally generated ids. - * - * @param entityMetaData The entity's meta data - * @param propertyAccessor An accessor to the entity - * @param entity As received via the driver - */ - public static void updateInternalId(Neo4jPersistentEntity entityMetaData, - PersistentPropertyAccessor propertyAccessor, Entity entity) { - - if (!entityMetaData.isUsingInternalIds()) { - return; - } - - propertyAccessor.setProperty(entityMetaData.getRequiredIdProperty(), getInternalId(entity)); - } - /** * @param entity The entity container as received from the server. * @return The internal id */ - @SuppressWarnings("deprecation") - public static Long getInternalId(Entity entity) { - return entity.id(); + public static String getElementId(Entity entity) { + return entity.elementId(); } /** @@ -78,9 +58,24 @@ public static Long getInternalId(Entity entity) { * @return An internal id */ @Nullable + public static String getElementId(@NonNull MapAccessor row) { + if (row instanceof Entity entity) { + return getElementId(entity); + } + + var columnToUse = Constants.NAME_OF_ELEMENT_ID; + if (row.get(columnToUse) == null || row.get(columnToUse).isNull()) { + return null; + } + + return row.get(columnToUse).asString(); + } + + @Nullable + @Deprecated public static Long getInternalId(@NonNull MapAccessor row) { if (row instanceof Entity entity) { - return getInternalId(entity); + return entity.id(); } var columnToUse = Constants.NAME_OF_INTERNAL_ID; @@ -92,41 +87,20 @@ public static Long getInternalId(@NonNull MapAccessor row) { } @Nullable - public static String getInternalId(@NonNull MapAccessor queryResult, @Nullable String seed) { + public static String getPrefixedElementId(@NonNull MapAccessor queryResult, @Nullable String seed) { if (queryResult instanceof Node) { - return "N" + getInternalId(queryResult); + return "N" + getElementId(queryResult); } else if (queryResult instanceof Relationship) { - return "R" + seed + getInternalId(queryResult); - } else if (!(queryResult.get(Constants.NAME_OF_INTERNAL_ID) == null || queryResult.get(Constants.NAME_OF_INTERNAL_ID).isNull())) { - return "N" + queryResult.get(Constants.NAME_OF_INTERNAL_ID).asLong(); + return "R" + seed + getElementId(queryResult); + } else if (!(queryResult.get(Constants.NAME_OF_ELEMENT_ID) == null || queryResult.get(Constants.NAME_OF_ELEMENT_ID).isNull())) { + return "N" + queryResult.get(Constants.NAME_OF_ELEMENT_ID).asString(); } return null; } - /** - * Returns the start id of the relationship - * - * @param relationship A relationship to retrieve the start identity - * @return An internal id - */ - @SuppressWarnings("deprecation") - public static Long getStartId(Relationship relationship) { - return relationship.startNodeId(); - } - - /** - * Returns the end id of the relationship - * - * @param relationship A relationship to retrieve the end identity - * @return An internal id - */ - @SuppressWarnings("deprecation") - public static Long getEndId(Relationship relationship) { - return relationship.endNodeId(); - } - - public static Long getInternalId(TypeSystem typeSystem, Record row) { - return getInternalId(row); + public static Function mapperForRelatedIdValues(@Nullable Neo4jPersistentProperty idProperty) { + boolean deprecatedHolder = idProperty != null && Neo4jPersistentEntity.DEPRECATED_GENERATED_ID_TYPES.contains(idProperty.getType()); + return deprecatedHolder ? IdentitySupport::getInternalId : IdentitySupport::getElementId; } } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentEntity.java b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentEntity.java index 7f20d2ac9..548e8997a 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentEntity.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentEntity.java @@ -16,6 +16,7 @@ package org.springframework.data.neo4j.core.mapping; import java.util.Optional; +import java.util.Set; import org.apiguardian.api.API; import org.springframework.data.mapping.AssociationHandler; @@ -40,6 +41,8 @@ public interface Neo4jPersistentEntity extends MutablePersistentEntity, NodeDescription { + Set> DEPRECATED_GENERATED_ID_TYPES = Set.of(Long.class, long.class); + /** * @return An optional property pointing to a {@link java.util.Collection Collection<String>} containing dynamic * "runtime managed" labels. @@ -53,4 +56,10 @@ public interface Neo4jPersistentEntity */ boolean isRelationshipPropertiesEntity(); + /** + * @return True if the underlying domain classes uses {@code id()} to compute internally generated ids. + */ + default boolean isUsingDeprecatedInternalId() { + return isUsingInternalIds() && Neo4jPersistentEntity.DEPRECATED_GENERATED_ID_TYPES.contains(getRequiredIdProperty().getType()); + } } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java b/src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java index a062b5c65..9cefed810 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java @@ -19,11 +19,9 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.locks.StampedLock; -import java.util.stream.Collectors; import org.apiguardian.api.API; import org.springframework.lang.NonNull; @@ -66,7 +64,7 @@ public enum ProcessState { * A map pointing from a processed object to the internal id. * This will be useful during the persistence to avoid another DB network round-trip. */ - private final Map processedObjectsIds = new HashMap<>(); + private final Map processedObjectsIds = new HashMap<>(); public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappingContext) { @@ -75,22 +73,24 @@ public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappin this.mappingContext = mappingContext; } - public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappingContext, Object initialObject, Long internalId) { + public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappingContext, Object initialObject, String elementId) { this(mappingContext); Assert.notNull(initialObject, "Initial object must not be null"); - Assert.notNull(internalId, "The initial objects internal ID must not be null"); + Assert.notNull(elementId, "The initial objects element ID must not be null"); - storeHashedVersionInProcessedObjectsIds(initialObject, internalId); + storeHashedVersionInProcessedObjectsIds(initialObject, elementId); } /** * @param relationshipDescription Check whether this relationship description has been processed - * @param valuesToStore Check whether all the values in the collection have been processed + * @param valuesToStore Check whether all the values in the collection have been processed * @return The state of things processed */ - public ProcessState getStateOf(Object fromId, RelationshipDescription relationshipDescription, @Nullable Collection valuesToStore) { - + public ProcessState getStateOf(@Nullable Object fromId, RelationshipDescription relationshipDescription, @Nullable Collection valuesToStore) { + if (fromId == null) { + return ProcessState.PROCESSED_BOTH; + } final long stamp = lock.readLock(); try { boolean hasProcessedRelationship = hasProcessedRelationship(fromId, relationshipDescription); @@ -116,31 +116,7 @@ public ProcessState getStateOf(Object fromId, RelationshipDescription relationsh * can get processed for different objects of the same entity. * One could say that this is a Tuple but it has a nicer name. */ - private static class RelationshipDescriptionWithSourceId { - private final Object id; - private final RelationshipDescription relationshipDescription; - - RelationshipDescriptionWithSourceId(Object id, RelationshipDescription relationshipDescription) { - this.id = id; - this.relationshipDescription = relationshipDescription; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - RelationshipDescriptionWithSourceId that = (RelationshipDescriptionWithSourceId) o; - return id.equals(that.id) && relationshipDescription.equals(that.relationshipDescription); - } - - @Override - public int hashCode() { - return Objects.hash(id, relationshipDescription); - } + private record RelationshipDescriptionWithSourceId(Object id, RelationshipDescription relationshipDescription) { } /** @@ -165,24 +141,24 @@ public void markRelationshipAsProcessed(Object fromId, @Nullable RelationshipDes * Marks the passed objects as processed * * @param valueToStore If not {@literal null}, all non-null values will be marked as processed - * @param internalId The internal id of the value processed + * @param elementId The internal id of the value processed */ - public void markValueAsProcessed(Object valueToStore, Long internalId) { + public void markEntityAsProcessed(Object valueToStore, String elementId) { final long stamp = lock.writeLock(); try { - doMarkValueAsProcessed(valueToStore, internalId); + doMarkValueAsProcessed(valueToStore, elementId); storeProcessedInAlias(valueToStore, valueToStore); } finally { lock.unlock(stamp); } } - private void doMarkValueAsProcessed(Object valueToStore, Long internalId) { + private void doMarkValueAsProcessed(Object valueToStore, String elementId) { Object value = extractRelatedValueFromRelationshipProperties(valueToStore); - storeHashedVersionInProcessedObjectsIds(valueToStore, internalId); - storeHashedVersionInProcessedObjectsIds(value, internalId); + storeHashedVersionInProcessedObjectsIds(valueToStore, elementId); + storeHashedVersionInProcessedObjectsIds(value, elementId); } /** @@ -204,6 +180,7 @@ public boolean hasProcessedValue(Object value) { Neo4jPersistentEntity entity = mappingContext.getRequiredPersistentEntity(typeOfValue); Neo4jPersistentProperty idProperty = entity.getIdProperty(); Object id = idProperty == null ? null : entity.getPropertyAccessor(valueToCheck).getProperty(idProperty); + // After the lookup by system.identityHashCode failed for a processed object alias, // we must traverse or iterate over all value with the matching type and compare the domain ids // to figure out if the logical object has already been processed through a different object instance. @@ -214,7 +191,7 @@ public boolean hasProcessedValue(Object value) { .findAny(); if (alreadyProcessedObject.isPresent()) { // Skip the show the next time around. processed = true; - Long internalId = getInternalId(alreadyProcessedObject.get()); + String internalId = getObjectId(alreadyProcessedObject.get()); if (internalId != null) { stamp = lock.tryConvertToWriteLock(stamp); doMarkValueAsProcessed(valueToCheck, internalId); @@ -245,26 +222,32 @@ public boolean hasProcessedRelationship(Object fromId, @Nullable RelationshipDes return false; } - public void markValueAsProcessedAs(Object valueToStore, Object bean) { + public void markAsAliased(Object aliasEntity, Object entityOrId) { final long stamp = lock.writeLock(); try { - storeProcessedInAlias(valueToStore, bean); + storeProcessedInAlias(aliasEntity, entityOrId); } finally { lock.unlock(stamp); } } + /** + * This returns an id for the given object. We deliberate use the wording of a generic object id as that might either be + * the Neo4j 5+ {@literal elementId()} or on older Neo4j versions or with older data modules {@code toString(id())}. + * + * @param object The object for which an id is requested + * @return The objects id + */ @Nullable - public Long getInternalId(Object object) { + public String getObjectId(Object object) { final long stamp = lock.readLock(); try { Object valueToCheck = extractRelatedValueFromRelationshipProperties(object); - Long possibleId = getProcessedObjectIds(valueToCheck); + String possibleId = getProcessedObjectIds(valueToCheck); return possibleId != null ? possibleId : getProcessedObjectIds(getProcessedAs(valueToCheck)); } finally { lock.unlock(stamp); } - } public Object getProcessedAs(Object entity) { @@ -278,7 +261,7 @@ public Object getProcessedAs(Object entity) { } @Nullable - private Long getProcessedObjectIds(@Nullable Object entity) { + private String getProcessedObjectIds(@Nullable Object entity) { if (entity == null) { return null; } @@ -299,27 +282,27 @@ private Object extractRelatedValueFromRelationshipProperties(Object valueToStore /* * Convenience wrapper functions to avoid exposing the System.identityHashCode "everywhere" in this class. */ - private void storeHashedVersionInProcessedObjectsIds(Object initialObject, Long internalId) { - processedObjectsIds.put(System.identityHashCode(initialObject), internalId); + private void storeHashedVersionInProcessedObjectsIds(Object initialObject, String elementId) { + processedObjectsIds.put(System.identityHashCode(initialObject), elementId); } - private void storeProcessedInAlias(Object valueToStore, Object bean) { - processedObjectsAlias.put(System.identityHashCode(valueToStore), bean); + private void storeProcessedInAlias(Object aliasEntity, Object targetEntity) { + processedObjectsAlias.put(System.identityHashCode(aliasEntity), targetEntity); } private Object getProcessedAsWithDefaults(Object entity) { return processedObjectsAlias.getOrDefault(System.identityHashCode(entity), entity); } - private boolean hasProcessed(Object valueToCheck) { - return processedObjectsAlias.containsKey(System.identityHashCode(valueToCheck)); + private boolean hasProcessed(Object entity) { + return processedObjectsAlias.containsKey(System.identityHashCode(entity)); } - private boolean hasProcessedAllOf(@Nullable Collection valuesToStore) { + private boolean hasProcessedAllOf(@Nullable Collection entities) { // there can be null elements in the unified collection of values to store. - if (valuesToStore == null) { + if (entities == null) { return false; } - return processedObjectsIds.keySet().containsAll(valuesToStore.stream().map(System::identityHashCode).collect(Collectors.toList())); + return processedObjectsIds.keySet().containsAll(entities.stream().map(System::identityHashCode).toList()); } } diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/NodeDescription.java b/src/main/java/org/springframework/data/neo4j/core/mapping/NodeDescription.java index 64bafab32..b03690b19 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/NodeDescription.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/NodeDescription.java @@ -134,6 +134,7 @@ default boolean isUsingInternalIds() { @Nullable NodeDescription getParentNodeDescription(); + /** * @return An expression that represents the right identifier type. */ diff --git a/src/test/java/org/springframework/data/neo4j/core/mapping/CypherGeneratorTest.java b/src/test/java/org/springframework/data/neo4j/core/mapping/CypherGeneratorTest.java index 41019781b..766f29ff1 100644 --- a/src/test/java/org/springframework/data/neo4j/core/mapping/CypherGeneratorTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/mapping/CypherGeneratorTest.java @@ -26,7 +26,7 @@ import java.util.regex.Pattern; import java.util.stream.Stream; -import org.junit.Assert; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -35,6 +35,8 @@ import org.mockito.Mockito; import org.neo4j.cypherdsl.core.Cypher; import org.neo4j.cypherdsl.core.Statement; +import org.neo4j.cypherdsl.core.renderer.Configuration; +import org.neo4j.cypherdsl.core.renderer.Dialect; import org.neo4j.cypherdsl.core.renderer.Renderer; import org.springframework.data.domain.Sort; import org.springframework.data.neo4j.core.schema.Id; @@ -57,8 +59,8 @@ void shouldCreateRelationshipCreationQueryWithLabelIfPresent() { relationshipDescription, "REL"); String expectedQuery = "MATCH (startNode:`Entity1`) WHERE startNode.id = $fromId MATCH (endNode)" - + " WHERE id(endNode) = $toId MERGE (startNode)<-[relProps:`REL`]-(endNode) RETURN id(relProps) AS __internalNeo4jId__, toString(id(relProps)) AS __elementId__"; - Assert.assertEquals(expectedQuery, Renderer.getDefaultRenderer().render(statement)); + + " WHERE elementId(endNode) = $toId MERGE (startNode)<-[relProps:`REL`]-(endNode) RETURN elementId(relProps) AS __elementId__"; + Assertions.assertEquals(expectedQuery, Renderer.getRenderer(Configuration.newConfig().withDialect(Dialect.NEO4J_5).build()).render(statement)); } @Test @@ -73,8 +75,8 @@ void shouldCreateRelationshipCreationQueryWithMultipleLabels() { String expectedQuery = "MATCH (startNode:`Entity1`:`MultipleLabel`) WHERE startNode.id = $fromId MATCH (endNode)" - + " WHERE id(endNode) = $toId MERGE (startNode)<-[relProps:`REL`]-(endNode) RETURN id(relProps) AS __internalNeo4jId__, toString(id(relProps)) AS __elementId__"; - Assert.assertEquals(expectedQuery, Renderer.getDefaultRenderer().render(statement)); + + " WHERE elementId(endNode) = $toId MERGE (startNode)<-[relProps:`REL`]-(endNode) RETURN elementId(relProps) AS __elementId__"; + Assertions.assertEquals(expectedQuery, Renderer.getRenderer(Configuration.newConfig().withDialect(Dialect.NEO4J_5).build()).render(statement)); } @Test @@ -83,16 +85,18 @@ void shouldCreateRelationshipCreationQueryWithoutUsingInternalIds() { Neo4jPersistentEntity persistentEntity = Mockito.mock(Neo4jPersistentEntity.class); Neo4jPersistentProperty persistentProperty = Mockito.mock(Neo4jPersistentProperty.class); + doReturn(Long.class).when(persistentProperty).getType(); when(relationshipDescription.isDynamic()).thenReturn(true); when(persistentEntity.isUsingInternalIds()).thenReturn(true); when(persistentEntity.getRequiredIdProperty()).thenReturn(persistentProperty); + when(persistentEntity.isUsingDeprecatedInternalId()).thenReturn(true); Statement statement = CypherGenerator.INSTANCE.prepareSaveOfRelationship(persistentEntity, relationshipDescription, "REL"); String expectedQuery = "MATCH (startNode) WHERE id(startNode) = $fromId MATCH (endNode)" - + " WHERE id(endNode) = $toId MERGE (startNode)<-[relProps:`REL`]-(endNode) RETURN id(relProps) AS __internalNeo4jId__, toString(id(relProps)) AS __elementId__"; - Assert.assertEquals(expectedQuery, Renderer.getDefaultRenderer().render(statement)); + + " WHERE elementId(endNode) = $toId MERGE (startNode)<-[relProps:`REL`]-(endNode) RETURN elementId(relProps) AS __elementId__"; + Assertions.assertEquals(expectedQuery, Renderer.getRenderer(Configuration.newConfig().withDialect(Dialect.NEO4J_5).build()).render(statement)); } @Test @@ -104,8 +108,8 @@ void shouldCreateRelationshipRemoveQueryWithLabelIfPresent() { Statement statement = CypherGenerator.INSTANCE.prepareDeleteOf(persistentEntity, relationshipDescription); - String expectedQuery = "MATCH (startNode:`Entity1`)<-[rel]-(:`Entity2`) WHERE (startNode.id = $fromId AND NOT (id(rel) IN $__knownRelationShipIds__)) DELETE rel"; - Assert.assertEquals(expectedQuery, Renderer.getDefaultRenderer().render(statement)); + String expectedQuery = "MATCH (startNode:`Entity1`)<-[rel]-(:`Entity2`) WHERE (startNode.id = $fromId AND NOT (elementId(rel) IN $__knownRelationShipIds__)) DELETE rel"; + Assertions.assertEquals(expectedQuery, Renderer.getRenderer(Configuration.newConfig().withDialect(Dialect.NEO4J_5).build()).render(statement)); } @Test @@ -119,8 +123,8 @@ void shouldCreateRelationshipRemoveQueryWithMultipleLabels() { Statement statement = CypherGenerator.INSTANCE.prepareDeleteOf(persistentEntity, relationshipDescription); - String expectedQuery = "MATCH (startNode:`Entity1`:`MultipleLabel`)<-[rel]-(:`Entity2`:`MultipleLabel`) WHERE (startNode.id = $fromId AND NOT (id(rel) IN $__knownRelationShipIds__)) DELETE rel"; - Assert.assertEquals(expectedQuery, Renderer.getDefaultRenderer().render(statement)); + String expectedQuery = "MATCH (startNode:`Entity1`:`MultipleLabel`)<-[rel]-(:`Entity2`:`MultipleLabel`) WHERE (startNode.id = $fromId AND NOT (elementId(rel) IN $__knownRelationShipIds__)) DELETE rel"; + Assertions.assertEquals(expectedQuery, Renderer.getRenderer(Configuration.newConfig().withDialect(Dialect.NEO4J_5).build()).render(statement)); } @Test @@ -131,16 +135,18 @@ void shouldCreateRelationshipRemoveQueryWithoutUsingInternalIds() { RelationshipDescription relationshipDescription = Mockito.mock(RelationshipDescription.class); Neo4jPersistentEntity persistentEntity = Mockito.mock(Neo4jPersistentEntity.class); Neo4jPersistentProperty persistentProperty = Mockito.mock(Neo4jPersistentProperty.class); + doReturn(Long.class).when(persistentProperty).getType(); doReturn(relatedEntity).when(relationshipDescription).getTarget(); when(relationshipDescription.isDynamic()).thenReturn(true); when(persistentEntity.isUsingInternalIds()).thenReturn(true); when(persistentEntity.getRequiredIdProperty()).thenReturn(persistentProperty); + when(persistentEntity.isUsingDeprecatedInternalId()).thenReturn(true); Statement statement = CypherGenerator.INSTANCE.prepareDeleteOf(persistentEntity, relationshipDescription); - String expectedQuery = "MATCH (startNode)<-[rel]-(:`Entity2`) WHERE (id(startNode) = $fromId AND NOT (id(rel) IN $__knownRelationShipIds__)) DELETE rel"; - Assert.assertEquals(expectedQuery, Renderer.getDefaultRenderer().render(statement)); + String expectedQuery = "MATCH (startNode)<-[rel]-(:`Entity2`) WHERE (id(startNode) = $fromId AND NOT (elementId(rel) IN $__knownRelationShipIds__)) DELETE rel"; + Assertions.assertEquals(expectedQuery, Renderer.getRenderer(Configuration.newConfig().withDialect(Dialect.NEO4J_5).build()).render(statement)); } private static Stream pageables() { diff --git a/src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java b/src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java index 4f979abbc..b5a673ec2 100644 --- a/src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java @@ -904,7 +904,7 @@ static class InvalidId { static class InvalidIdType { @Id @GeneratedValue @SuppressWarnings("unused") - private String id; + private Double id; } @Node diff --git a/src/test/java/org/springframework/data/neo4j/core/mapping/callback/IdPopulatorTest.java b/src/test/java/org/springframework/data/neo4j/core/mapping/callback/IdPopulatorTest.java index a9ee221e6..b3cae78a5 100644 --- a/src/test/java/org/springframework/data/neo4j/core/mapping/callback/IdPopulatorTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/mapping/callback/IdPopulatorTest.java @@ -58,7 +58,7 @@ void shouldRejectNullEntity() { @Test void shouldIgnoreInternalIdGenerator() { - IdDescription toBeReturned = IdDescription.forInternallyGeneratedIds(Constants.NAME_OF_ROOT_NODE); + IdDescription toBeReturned = IdDescription.forInternallyGeneratedIds(Constants.NAME_OF_ROOT_NODE, true); doReturn(toBeReturned).when(nodeDescription).getIdDescription(); doReturn(nodeDescription).when(neo4jMappingContext).getRequiredPersistentEntity(Sample.class); diff --git a/src/test/java/org/springframework/data/neo4j/integration/imperative/InheritanceMappingIT.java b/src/test/java/org/springframework/data/neo4j/integration/imperative/InheritanceMappingIT.java index b97cafe7c..a207d6162 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/imperative/InheritanceMappingIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/imperative/InheritanceMappingIT.java @@ -35,8 +35,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.data.neo4j.core.mapping.IdentitySupport; -import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration; import org.springframework.data.neo4j.core.DatabaseSelectionProvider; import org.springframework.data.neo4j.core.Neo4jTemplate; import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; @@ -52,6 +50,7 @@ import org.springframework.data.neo4j.repository.query.Query; import org.springframework.data.neo4j.test.BookmarkCapture; import org.springframework.data.neo4j.test.Neo4jExtension; +import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration; import org.springframework.data.neo4j.test.Neo4jIntegrationTest; import org.springframework.data.repository.query.Param; import org.springframework.transaction.PlatformTransactionManager; @@ -371,8 +370,8 @@ void shouldMatchPolymorphicInterfacesWhenFetchedById(@Autowired ParentModelRepos Record record = createRelationsToDifferentImplementations(); - Optional optionalDivision = repository.findById( - IdentitySupport.getInternalId(record.get(0).asNode())); + @SuppressWarnings("deprecation") + Optional optionalDivision = repository.findById(record.get(0).asNode().id()); assertThat(optionalDivision).isPresent(); assertThat(optionalDivision).hasValueSatisfying(twoDifferentInterfacesHaveBeenLoaded()); } diff --git a/src/test/java/org/springframework/data/neo4j/integration/imperative/Neo4jClientIT.java b/src/test/java/org/springframework/data/neo4j/integration/imperative/Neo4jClientIT.java index a6976b991..4d51b3658 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/imperative/Neo4jClientIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/imperative/Neo4jClientIT.java @@ -34,8 +34,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.data.neo4j.core.mapping.IdentitySupport; -import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration; import org.springframework.data.neo4j.core.DatabaseSelectionProvider; import org.springframework.data.neo4j.core.Neo4jClient; import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; @@ -43,7 +41,9 @@ import org.springframework.data.neo4j.integration.shared.common.PersonWithAllConstructor; import org.springframework.data.neo4j.test.BookmarkCapture; import org.springframework.data.neo4j.test.Neo4jExtension.Neo4jConnectionSupport; +import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration; import org.springframework.data.neo4j.test.Neo4jIntegrationTest; +import org.springframework.data.neo4j.test.TestIdentitySupport; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.EnableTransactionManagement; import org.springframework.transaction.support.TransactionTemplate; @@ -85,7 +85,7 @@ void clientShouldIntegrateWithCypherDSL(@Autowired TransactionTemplate transacti .first().extracting(r -> r.get("n").get("value").asLong()).isEqualTo(42L); transactionStatus.setRollbackOnly(); - return IdentitySupport.getInternalId(records.get(0).get("n").asNode()); + return TestIdentitySupport.getInternalId(records.get(0).get("n").asNode()); }); // Make sure we actually interacted with the managed transaction (that had been rolled back) diff --git a/src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java b/src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java index a63a6d0ac..45acf5d6a 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java @@ -98,7 +98,6 @@ import org.springframework.data.neo4j.core.UserSelection; import org.springframework.data.neo4j.core.UserSelectionProvider; import org.springframework.data.neo4j.core.convert.Neo4jConversions; -import org.springframework.data.neo4j.core.mapping.IdentitySupport; import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext; import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager; @@ -162,6 +161,7 @@ import org.springframework.data.neo4j.test.Neo4jExtension; import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration; import org.springframework.data.neo4j.test.ServerVersion; +import org.springframework.data.neo4j.test.TestIdentitySupport; import org.springframework.data.neo4j.types.CartesianPoint2d; import org.springframework.data.neo4j.types.GeographicPoint2d; import org.springframework.data.repository.query.FluentQuery; @@ -736,9 +736,9 @@ void customFindMapsDeepRelationships(@Autowired PetRepository repository) { Record record = doWithSession(session -> session.run( "CREATE (p1:Pet{name: 'Pet1'})-[:Has]->(p2:Pet{name: 'Pet2'}), (p2)-[:Has]->(p3:Pet{name: 'Pet3'}) RETURN p1, p2, p3").single()); - long petNode1Id = IdentitySupport.getInternalId(record.get("p1").asNode()); - long petNode2Id = IdentitySupport.getInternalId(record.get("p2").asNode()); - long petNode3Id = IdentitySupport.getInternalId(record.get("p3").asNode()); + long petNode1Id = TestIdentitySupport.getInternalId(record.get("p1").asNode()); + long petNode2Id = TestIdentitySupport.getInternalId(record.get("p2").asNode()); + long petNode3Id = TestIdentitySupport.getInternalId(record.get("p3").asNode()); Pet loadedPet = repository.customQueryWithDeepRelationshipMapping(petNode1Id); @@ -910,12 +910,12 @@ void findEntityWithRelationship(@Autowired RelationshipRepository repository) { Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long clubId = IdentitySupport.getInternalId(clubNode); - long hobbyNode1Id = IdentitySupport.getInternalId(hobbyNode1); - long hobbyNode2Id = IdentitySupport.getInternalId(hobbyNode2); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long clubId = TestIdentitySupport.getInternalId(clubNode); + long hobbyNode1Id = TestIdentitySupport.getInternalId(hobbyNode1); + long hobbyNode2Id = TestIdentitySupport.getInternalId(hobbyNode2); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship loadedPerson = repository.findById(personId).get(); assertThat(loadedPerson.getName()).isEqualTo("Freddie"); @@ -950,9 +950,9 @@ void findDeepSameLabelsAndTypeRelationships(@Autowired PetRepository repository) Record record = doWithSession(session -> session.run( "CREATE (p1:Pet{name: 'Pet1'})-[:Has]->(p2:Pet{name: 'Pet2'}), (p2)-[:Has]->(p3:Pet{name: 'Pet3'}) RETURN p1, p2, p3").single()); - long petNode1Id = IdentitySupport.getInternalId(record.get("p1").asNode()); - long petNode2Id = IdentitySupport.getInternalId(record.get("p2").asNode()); - long petNode3Id = IdentitySupport.getInternalId(record.get("p3").asNode()); + long petNode1Id = TestIdentitySupport.getInternalId(record.get("p1").asNode()); + long petNode2Id = TestIdentitySupport.getInternalId(record.get("p2").asNode()); + long petNode3Id = TestIdentitySupport.getInternalId(record.get("p3").asNode()); Pet loadedPet = repository.findById(petNode1Id).get(); Pet comparisonPet2 = new Pet(petNode2Id, "Pet2"); @@ -998,7 +998,7 @@ void findBySameLabelRelationshipPropertyMultipleLevels(@Autowired PetRepository @Test void findLoopingDeepRelationships(@Autowired LoopingRelationshipRepository loopingRelationshipRepository) { - long type1Id = IdentitySupport.getInternalId(doWithSession(session -> session.run(""" + long type1Id = TestIdentitySupport.getInternalId(doWithSession(session -> session.run(""" CREATE (t1:LoopingType1)-[:NEXT_TYPE]->(:LoopingType2)-[:NEXT_TYPE]->(:LoopingType3)-[:NEXT_TYPE]-> (:LoopingType1)-[:NEXT_TYPE]->(:LoopingType2)-[:NEXT_TYPE]->(:LoopingType3)-[:NEXT_TYPE]-> (:LoopingType1)-[:NEXT_TYPE]->(:LoopingType2)-[:NEXT_TYPE]->(:LoopingType3)-[:NEXT_TYPE]-> @@ -1049,9 +1049,9 @@ void findEntityWithRelationshipToTheSameNode(@Autowired RelationshipRepository r Node hobbyNode1 = record.get("h1").asNode(); Node petNode1 = record.get("p1").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNode1Id = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNode1Id = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); PersonWithRelationship loadedPerson = repository.findById(personId).get(); assertThat(loadedPerson.getName()).isEqualTo("Freddie"); @@ -1075,17 +1075,17 @@ void findEntityWithRelationshipToTheSameNode(@Autowired RelationshipRepository r @Test void findEntityWithBidirectionalRelationshipInConstructorThrowsException(@Autowired BidirectionalStartRepository repository) { - long startId = IdentitySupport.getInternalId(doWithSession(session -> session + var node = doWithSession(session -> session .run(""" CREATE (n:BidirectionalStart{name:'Ernie'})-[:CONNECTED]->(e:BidirectionalEnd{name:'Bert'}), (e)<-[:ANOTHER_CONNECTION]-(anotherStart:BidirectionalStart{name:'Elmo'}) RETURN n""" ) - .single().get("n").asNode())); + .single().get("n").asNode()); - assertThatThrownBy(() -> repository.findById(startId)) - .hasRootCauseMessage("The node with id " + startId + " has a logical cyclic mapping dependency; " + + assertThatThrownBy(() -> repository.findById(TestIdentitySupport.getInternalId(node))) + .hasRootCauseMessage("The node with id " + node.elementId() + " has a logical cyclic mapping dependency; " + "its creation caused the creation of another node that has a reference to this") .hasRootCauseInstanceOf(MappingException.class); @@ -1133,7 +1133,7 @@ private long createFriendlyPets() { @Test void findEntityWithBidirectionalRelationshipFromIncomingSide(@Autowired BidirectionalEndRepository repository) { - long endId = IdentitySupport.getInternalId(doWithSession(session -> session.run( + long endId = TestIdentitySupport.getInternalId(doWithSession(session -> session.run( "CREATE (n:BidirectionalStart{name:'Ernie'})-[:CONNECTED]->(e:BidirectionalEnd{name:'Bert'}) RETURN e") .single().get("e").asNode())); @@ -1151,14 +1151,14 @@ void findMultipleEntitiesWithRelationship(@Autowired RelationshipRepository repo .run("CREATE (n:PersonWithRelationship{name:'Freddie'})-[:Has]->(h:Hobby{name:'Music'}), (n)-[:Has]->(p:Pet{name: 'Jerry'}) RETURN n, h, p") .single()); - long hobbyNode1Id = IdentitySupport.getInternalId(record.get("h").asNode()); - long petNode1Id = IdentitySupport.getInternalId(record.get("p").asNode()); + long hobbyNode1Id = TestIdentitySupport.getInternalId(record.get("h").asNode()); + long petNode1Id = TestIdentitySupport.getInternalId(record.get("p").asNode()); record = doWithSession(session -> session.run( "CREATE (n:PersonWithRelationship{name:'SomeoneElse'})-[:Has]->(h:Hobby{name:'Music2'}), (n)-[:Has]->(p:Pet{name: 'Jerry2'}) RETURN n, h, p").single()); - long hobbyNode2Id = IdentitySupport.getInternalId(record.get("h").asNode()); - long petNode2Id = IdentitySupport.getInternalId(record.get("p").asNode()); + long hobbyNode2Id = TestIdentitySupport.getInternalId(record.get("h").asNode()); + long petNode2Id = TestIdentitySupport.getInternalId(record.get("p").asNode()); List loadedPersons = repository.findAll(); @@ -1190,10 +1190,10 @@ void findEntityWithRelationshipViaQuery(@Autowired RelationshipRepository reposi Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship loadedPerson = repository.getPersonWithRelationshipsViaQuery(); assertThat(loadedPerson.getName()).isEqualTo("Freddie"); @@ -1228,10 +1228,10 @@ void findEntityWithRelationshipViaPathQuery(@Autowired RelationshipRepository re Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship loadedPerson = repository.getPersonWithRelationshipsViaPathQuery(); assertThat(loadedPerson.getName()).isEqualTo("Freddie"); @@ -1251,7 +1251,7 @@ void findEntityWithRelationshipViaPathQuery(@Autowired RelationshipRepository re @Test void findEntityWithRelationshipWithAssignedId(@Autowired PetRepository repository) { - long petNodeId = IdentitySupport.getInternalId(doWithSession(session -> session + long petNodeId = TestIdentitySupport.getInternalId(doWithSession(session -> session .run("CREATE (p:Pet{name:'Jerry'})-[:Has]->(t:Thing{theId:'t1', name:'Thing1'}) RETURN p, t").single() .get("p").asNode())); @@ -1438,7 +1438,7 @@ void shouldBeStorableOnSets( + "{since: 2000, active: false, localDate: date('2000-06-28'), myEnum: 'SOMETHING_DIFFERENT', point: point({x: 2, y: 3})}" + "]->(h2:Hobby{name:'Something else'})" + "RETURN n, h1, h2").single().get("n").asNode()); - long personId = IdentitySupport.getInternalId(hlp); + long personId = TestIdentitySupport.getInternalId(hlp); Optional optionalPerson = template.findById(personId, PersonWithRelationshipWithProperties2.class); assertThat(optionalPerson).hasValueSatisfying(person -> { assertThat(person.getName()).isEqualTo("Freddie"); @@ -1466,9 +1466,9 @@ void findEntityWithRelationshipWithProperties( Node hobbyNode1 = record.get("h1").asNode(); Node hobbyNode2 = record.get("h2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNode1Id = IdentitySupport.getInternalId(hobbyNode1); - long hobbyNode2Id = IdentitySupport.getInternalId(hobbyNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNode1Id = TestIdentitySupport.getInternalId(hobbyNode1); + long hobbyNode2Id = TestIdentitySupport.getInternalId(hobbyNode2); Optional optionalPerson = repository.findById(personId); assertThat(optionalPerson).isPresent(); @@ -1507,7 +1507,7 @@ void findEntityWithRelationshipWithProperties( @Test void findEntityWithRelationshipWithPropertiesScalar(@Autowired PersonWithRelationshipWithPropertiesRepository repository) { - long personId = IdentitySupport.getInternalId(doWithSession(session -> session.run("CREATE (n:PersonWithRelationshipWithProperties{name:'Freddie'})," + long personId = TestIdentitySupport.getInternalId(doWithSession(session -> session.run("CREATE (n:PersonWithRelationshipWithProperties{name:'Freddie'})," + " (n)-[:WORKS_IN{since: 1995}]->(:Club{name:'Blubb'})," + "(n) - [:OWNS {place: 'The place to be'}] -> (c1:Club {name: 'Berlin Mitte'}), " + "(n) - [:OWNS {place: 'Whatever'}] -> (c2:Club {name: 'Schachklub'}) " @@ -1524,7 +1524,7 @@ void findEntityWithRelationshipWithPropertiesScalar(@Autowired PersonWithRelatio void findEntityWithRelationshipWithPropertiesSameLabel( @Autowired FriendRepository repository) { - long friendId = IdentitySupport.getInternalId(doWithSession(session -> session.run("CREATE (n:Friend{name:'Freddie'})," + long friendId = TestIdentitySupport.getInternalId(doWithSession(session -> session.run("CREATE (n:Friend{name:'Freddie'})," + " (n)-[:KNOWS{since: 1995}]->(:Friend{name:'Frank'})" + "RETURN n").single().get("n").asNode())); @@ -1634,9 +1634,9 @@ void findEntityWithRelationshipWithPropertiesFromCustomQuery( Node hobbyNode1 = record.get("h1").asNode(); Node hobbyNode2 = record.get("h2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNode1Id = IdentitySupport.getInternalId(hobbyNode1); - long hobbyNode2Id = IdentitySupport.getInternalId(hobbyNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNode1Id = TestIdentitySupport.getInternalId(hobbyNode1); + long hobbyNode2Id = TestIdentitySupport.getInternalId(hobbyNode2); PersonWithRelationshipWithProperties person = repository.loadFromCustomQuery(personId); assertThat(person.getName()).isEqualTo("Freddie"); @@ -1671,7 +1671,7 @@ void findEntityWithRelationshipWithPropertiesFromCustomQuery( void loadEntityWithRelationshipWithPropertiesFromCustomQueryIncoming( @Autowired HobbyWithRelationshipWithPropertiesRepository repository) { - long personId = IdentitySupport.getInternalId(doWithSession( + long personId = TestIdentitySupport.getInternalId(doWithSession( session -> session.run("CREATE (n:AltPerson{name:'Freddie'}), (n)-[l1:LIKES {rating: 5}]->(h1:AltHobby{name:'Music'}) RETURN n, h1").single().get("n").asNode())); AltHobby hobby = repository.loadFromCustomQuery(personId); @@ -1686,7 +1686,7 @@ void loadEntityWithRelationshipWithPropertiesFromCustomQueryIncoming( @Test void loadSameNodeWithDoubleRelationship(@Autowired HobbyWithRelationshipWithPropertiesRepository repository) { - long personId = IdentitySupport.getInternalId(doWithSession(session -> session.run("CREATE (n:AltPerson{name:'Freddie'})," + + long personId = TestIdentitySupport.getInternalId(doWithSession(session -> session.run("CREATE (n:AltPerson{name:'Freddie'})," + " (n)-[l1:LIKES {rating: 5}]->(h1:AltHobby{name:'Music'})," + " (n)-[l2:LIKES {rating: 1}]->(h1)" + " RETURN n, h1").single().get("n").asNode())); @@ -1736,9 +1736,8 @@ void findAndMapMultipleLevelRelationshipProperties( @Test void updateAndCreateRelationshipProperties(@Autowired HobbyWithRelationshipWithPropertiesRepository repository) { - @SuppressWarnings("deprecation") long hobbyId = doWithSession( - session -> session.run("CREATE (n:AltPerson{name:'Freddie'}), (n)-[l1:LIKES {rating: 5}]->(h1:AltHobby{name:'Music'}) RETURN n, h1").single().get("h1").asNode().id()); + session -> TestIdentitySupport.getInternalId(session.run("CREATE (n:AltPerson{name:'Freddie'}), (n)-[l1:LIKES {rating: 5}]->(h1:AltHobby{name:'Music'}) RETURN n, h1").single().get("h1").asNode())); AltHobby hobby = repository.findById(hobbyId).get(); assertThat(hobby.getName()).isEqualTo("Music"); @@ -1746,13 +1745,13 @@ void updateAndCreateRelationshipProperties(@Autowired HobbyWithRelationshipWithP AltLikedByPersonRelationship liked = new AltLikedByPersonRelationship(); liked.setAltPerson(new AltPerson("SomethingElse")); + liked.setRating(2); hobby.getLikedBy().add(liked); repository.save(hobby); AltHobby savedHobby = repository.findById(hobbyId).get(); assertThat(savedHobby.getLikedBy()).hasSize(2); - } } @@ -1792,7 +1791,7 @@ void saveSingleEntity(@Autowired PersonRepository repository) { assertThat(record.containsKey("n")).isTrue(); Node node = record.get("n").asNode(); - assertThat(savedPerson.getId()).isEqualTo(IdentitySupport.getInternalId(node)); + assertThat(savedPerson.getId()).isEqualTo(TestIdentitySupport.getInternalId(node)); assertThat(node.get("things").asList()).containsExactly("b", "a"); }); } @@ -1939,7 +1938,7 @@ void updateSingleEntity(@Autowired PersonRepository repository) { assertThat(record.containsKey("n")).isTrue(); Node node = record.get("n").asNode(); - assertThat(IdentitySupport.getInternalId(node)).isEqualTo(savedPerson.getId()); + assertThat(TestIdentitySupport.getInternalId(node)).isEqualTo(savedPerson.getId()); assertThat(node.get("first_name").asString()).isEqualTo(savedPerson.getFirstName()); assertThat(node.get("nullable").asString()).isEqualTo(savedPerson.getNullable()); assertThat(node.get("things").asList()).isEmpty(); @@ -2145,7 +2144,7 @@ void saveSingleEntityWithRelationships(@Autowired RelationshipRepository reposit assertThat(record.containsKey("n")).isTrue(); Node rootNode = record.get("n").asNode(); - assertThat(savedPerson.getId()).isEqualTo(IdentitySupport.getInternalId(rootNode)); + assertThat(savedPerson.getId()).isEqualTo(TestIdentitySupport.getInternalId(rootNode)); assertThat(savedPerson.getName()).isEqualTo("Freddie"); List> petsWithHobbies = record.get("petsWithHobbies").asList(Value::asList); @@ -2206,7 +2205,7 @@ void saveSingleEntityWithRelationshipsTwiceDoesNotCreateMoreRelationships( assertThat(record.containsKey("n")).isTrue(); Node rootNode = record.get("n").asNode(); - assertThat(savedPerson.getId()).isEqualTo(IdentitySupport.getInternalId(rootNode)); + assertThat(savedPerson.getId()).isEqualTo(TestIdentitySupport.getInternalId(rootNode)); assertThat(savedPerson.getName()).isEqualTo("Freddie"); List> petsWithHobbies = record.get("petsWithHobbies").asList(Value::asList); @@ -2264,7 +2263,7 @@ void saveEntityWithAlreadyExistingTargetNode(@Autowired RelationshipRepository r assertThat(record.containsKey("n")).isTrue(); Node rootNode = record.get("n").asNode(); - assertThat(savedPerson.getId()).isEqualTo(IdentitySupport.getInternalId(rootNode)); + assertThat(savedPerson.getId()).isEqualTo(TestIdentitySupport.getInternalId(rootNode)); assertThat(savedPerson.getName()).isEqualTo("Freddie"); assertThat(record.get("hobbies").asList(entry -> entry.asNode().get("name").asString())) @@ -2307,7 +2306,7 @@ void saveEntityWithAlreadyExistingSourceAndTargetNode(@Autowired RelationshipRep assertThat(record.containsKey("n")).isTrue(); Node rootNode = record.get("n").asNode(); - assertThat(savedPerson.getId()).isEqualTo(IdentitySupport.getInternalId(rootNode)); + assertThat(savedPerson.getId()).isEqualTo(TestIdentitySupport.getInternalId(rootNode)); assertThat(savedPerson.getName()).isEqualTo("Freddie"); assertThat(record.get("hobbies").asList(entry -> entry.asNode().get("name").asString())) @@ -3052,10 +3051,10 @@ void findEntityWithRelationshipByFindOneByExample(@Autowired RelationshipReposit Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship probe = new PersonWithRelationship(); probe.setName("Freddie"); @@ -3092,10 +3091,10 @@ void findEntityWithRelationshipByFindAllByExample(@Autowired RelationshipReposit Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship probe = new PersonWithRelationship(); probe.setName("Freddie"); @@ -3142,10 +3141,10 @@ void findEntityWithRelationshipByFindAllByExampleWithSort(@Autowired Relationshi Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship probe = new PersonWithRelationship(); probe.setName("Freddie"); @@ -3181,10 +3180,10 @@ void findEntityWithRelationshipByFindAllByExampleWithPageable(@Autowired Relatio Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship probe = new PersonWithRelationship(); probe.setName("Freddie"); @@ -3773,9 +3772,9 @@ void createNodeAndRelationshipWithMultipleLabels(@Autowired MultipleLabelReposit void findNodeWithMultipleLabels(@Autowired MultipleLabelRepository multipleLabelRepository) { Record record = doWithSession(session -> session.run("CREATE (n1:A:B:C), (n2:B:C), (n3:A) return n1, n2, n3").single()); - long n1Id = IdentitySupport.getInternalId(record.get("n1").asNode()); - long n2Id = IdentitySupport.getInternalId(record.get("n2").asNode()); - long n3Id = IdentitySupport.getInternalId(record.get("n3").asNode()); + long n1Id = TestIdentitySupport.getInternalId(record.get("n1").asNode()); + long n2Id = TestIdentitySupport.getInternalId(record.get("n2").asNode()); + long n3Id = TestIdentitySupport.getInternalId(record.get("n3").asNode()); Assertions.assertThat(multipleLabelRepository.findById(n1Id)).isPresent(); Assertions.assertThat(multipleLabelRepository.findById(n2Id)).isNotPresent(); @@ -3786,9 +3785,9 @@ void findNodeWithMultipleLabels(@Autowired MultipleLabelRepository multipleLabel void deleteNodeWithMultipleLabels(@Autowired MultipleLabelRepository multipleLabelRepository) { Record record = doWithSession(session -> session.run("CREATE (n1:A:B:C), (n2:B:C), (n3:A) return n1, n2, n3").single()); - long n1Id = IdentitySupport.getInternalId(record.get("n1").asNode()); - long n2Id = IdentitySupport.getInternalId(record.get("n2").asNode()); - long n3Id = IdentitySupport.getInternalId(record.get("n3").asNode()); + long n1Id = TestIdentitySupport.getInternalId(record.get("n1").asNode()); + long n2Id = TestIdentitySupport.getInternalId(record.get("n2").asNode()); + long n3Id = TestIdentitySupport.getInternalId(record.get("n3").asNode()); multipleLabelRepository.deleteById(n1Id); multipleLabelRepository.deleteById(n2Id); diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/AbstractElementIdTestBase.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/AbstractElementIdTestBase.java new file mode 100644 index 000000000..eca189879 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/AbstractElementIdTestBase.java @@ -0,0 +1,69 @@ +/* + * Copyright 2011-2023 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.pure_element_id; + +import java.util.Objects; +import java.util.function.Predicate; +import java.util.regex.Pattern; + +import org.junit.jupiter.api.BeforeEach; +import org.neo4j.driver.Driver; +import org.neo4j.driver.Session; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.neo4j.test.BookmarkCapture; +import org.springframework.data.neo4j.test.Neo4jExtension; + +abstract class AbstractElementIdTestBase { + + protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport; + + @BeforeEach + void setupData(@Autowired Driver driver, @Autowired BookmarkCapture bookmarkCapture) { + + try (Session session = driver.session()) { + session.run("MATCH (n) DETACH DELETE n").consume(); + bookmarkCapture.seedWith(session.lastBookmarks()); + } + } + + private static final Pattern NEO5J_ELEMENT_ID_PATTERN = Pattern.compile("\\d+:.+:\\d+"); + + Predicate validIdForCurrentNeo4j() { + Predicate nonNull = Objects::nonNull; + + if (neo4jConnectionSupport.isCypher5SyntaxCompatible()) { + return nonNull.and(NEO5J_ELEMENT_ID_PATTERN.asMatchPredicate()); + } + + // Must be a valid long, and yes, I know how to write a pattern for that, too + return nonNull.and(v -> { + try { + Long.parseLong(v); + } catch (NumberFormatException e) { + return false; + } + return true; + }); + } + + static String adaptQueryTo44IfNecessary(String query) { + if (!neo4jConnectionSupport.isCypher5SyntaxCompatible()) { + query = query.replaceAll("elementId\\((.+?)\\)", "toString(id($1))"); + } + return query; + } + +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ImperativeElementIdIT.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ImperativeElementIdIT.java new file mode 100644 index 000000000..9e94eec50 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ImperativeElementIdIT.java @@ -0,0 +1,362 @@ +/* + * Copyright 2011-2023 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.pure_element_id; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.neo4j.driver.Driver; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.neo4j.core.DatabaseSelectionProvider; +import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; +import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager; +import org.springframework.data.neo4j.repository.Neo4jRepository; +import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories; +import org.springframework.data.neo4j.test.BookmarkCapture; +import org.springframework.data.neo4j.test.LogbackCapture; +import org.springframework.data.neo4j.test.LogbackCapturingExtension; +import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration; +import org.springframework.data.neo4j.test.Neo4jIntegrationTest; +import org.springframework.lang.NonNull; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.EnableTransactionManagement; + +/** + * Assertions that no {@code id()} calls are generated when no deprecated id types are present + * + * @author Michael J. Simons + */ +@Neo4jIntegrationTest +@ExtendWith(LogbackCapturingExtension.class) +public class ImperativeElementIdIT extends AbstractElementIdTestBase { + + interface Repo1 extends Neo4jRepository { + } + + interface Repo2 extends Neo4jRepository { + } + + interface Repo3 extends Neo4jRepository { + } + + interface Repo4 extends Neo4jRepository { + } + + @Test + void simpleNodeCreationShouldFillIdAndNotUseIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1) { + + var node = repo1.save(new NodeWithGeneratedId1("from-sdn-repo")); + assertThat(node.getId()) + .matches(validIdForCurrentNeo4j()); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void simpleNodeAllCreationShouldFillIdAndNotUseIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1) { + + var nodes = repo1.saveAll(List.of(new NodeWithGeneratedId1("from-sdn-repo"))); + assertThat(nodes).isNotEmpty() + .extracting(NodeWithGeneratedId1::getId) + .allMatch(validIdForCurrentNeo4j()); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void findByIdMustNotCallIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + String id; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + id = session.run("CREATE (n:NodeWithGeneratedId1 {value: 'whatever'}) RETURN n").single().get("n").asNode().elementId(); + } + + var optionalNode = repo1.findById(id); + assertThat(optionalNode).map(NodeWithGeneratedId1::getValue) + .hasValue("whatever"); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void findAllMustNotCallIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + session.run("CREATE (n:NodeWithGeneratedId1 {value: 'whatever'}) RETURN n").single().get("n").asNode().elementId(); + } + + var nodes = repo1.findAll(); + assertThat(nodes).isNotEmpty() + .extracting(NodeWithGeneratedId1::getId) + .allMatch(validIdForCurrentNeo4j()); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void updateMustNotCallIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + NodeWithGeneratedId1 node; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var dbNode = session.run("CREATE (n:NodeWithGeneratedId1 {value: 'whatever'}) RETURN n").single().get("n").asNode(); + node = new NodeWithGeneratedId1(dbNode.get("value").asString() + "_edited"); + node.setId(dbNode.elementId()); + } + + node = repo1.save(node); + assertThat(node).extracting(NodeWithGeneratedId1::getValue) + .isEqualTo("whatever_edited"); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void updateAllMustNotCallIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + NodeWithGeneratedId1 node; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var dbNode = session.run("CREATE (n:NodeWithGeneratedId1 {value: 'whatever'}) RETURN n").single().get("n").asNode(); + node = new NodeWithGeneratedId1(dbNode.get("value").asString() + "_edited"); + node.setId(dbNode.elementId()); + } + + var nodes = repo1.saveAll(List.of(node)); + assertThat(nodes).isNotEmpty() + .extracting(NodeWithGeneratedId1::getId) + .allMatch(validIdForCurrentNeo4j()); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void nodeAndRelationshipsWithoutPropsAndIdsMustNotUseIdFunctionWhileCreating(LogbackCapture logbackCapture, @Autowired Repo2 repo2, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + var owner = new NodeWithGeneratedId2("owner"); + owner.setRelatedNodes(List.of(new NodeWithGeneratedId1("child1"), new NodeWithGeneratedId1("child2"))); + owner = repo2.save(owner); + + assertThat(owner.getId()).isNotNull(); + assertThat(owner.getRelatedNodes()) + .allSatisfy(owned -> assertThat(owned.getId()).matches(validIdForCurrentNeo4j())); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void nodeAndRelationshipsWithoutPropsAndIdsMustNotUseIdFunctionWhileUpdating(LogbackCapture logbackCapture, @Autowired Repo2 repo2, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + String ownerId; + String ownedId; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var row = session.run("CREATE (n:NodeWithGeneratedId2 {value: 'owner'}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1 {value:'owned'}) RETURN *").single(); + ownerId = row.get("n").asNode().elementId(); + ownedId = row.get("m").asNode().elementId(); + } + + var owner = repo2.findById(ownerId).orElseThrow(); + assertThat(owner.getRelatedNodes()) + .hasSize(1) + .first() + .extracting(NodeWithGeneratedId1::getId) + .isEqualTo(ownedId); + + + owner.getRelatedNodes().get(0).setValue("owned_changed"); + owner.setValue("owner_changed"); + + repo2.save(owner); + + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var count = session.run(adaptQueryTo44IfNecessary(""" + MATCH (n:NodeWithGeneratedId2 {value: $v1}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1 {value: $v2}) + WHERE elementId(n) = $id1 AND elementId(m) = $id2 + RETURN count(*)"""), + Map.of("v1", "owner_changed", "v2", "owned_changed", "id1", ownerId, "id2", ownedId)).single().get(0).asLong(); + assertThat(count).isOne(); + } + } + + @Test + void relsWithPropOnCreation(LogbackCapture logbackCapture, @Autowired Repo3 repo3, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + var owner = new NodeWithGeneratedId3("owner"); + var target1 = new NodeWithGeneratedId1("target1"); + var target2 = new NodeWithGeneratedId1("target2"); + + owner.setRelatedNodes( + List.of( + new RelWithProps(target1, "vr1"), + new RelWithProps(target2, "vr2")) + ); + + owner = repo3.save(owner); + assertThat(owner.getId()).matches(validIdForCurrentNeo4j()); + assertThat(owner.getRelatedNodes()) + .hasSize(2) + .allSatisfy(r -> assertThat(r.getTarget().getId()).isNotNull()) + .extracting(RelWithProps::getRelValue) + .containsExactlyInAnyOrder("vr1", "vr2"); + + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var count = session.run(adaptQueryTo44IfNecessary(""" + MATCH (n:NodeWithGeneratedId3 {value: $v1}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1) + WHERE elementId(n) = $id1 + AND r.relValue IN $rv + RETURN count(*)"""), + Map.of("v1", "owner", "id1", owner.getId(), "rv", List.of("vr1", "vr2"))).single().get(0).asLong(); + assertThat(count).isEqualTo(2L); + } + } + + @Test + void relsWithPropOnUpdate(LogbackCapture logbackCapture, @Autowired Repo3 repo3, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + String ownerId; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + ownerId = session.run(adaptQueryTo44IfNecessary(""" + CREATE (n:NodeWithGeneratedId3 {value: 'owner'}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1 {value: 'owned'}) + RETURN elementId(n)""") + ).single().get(0).asString(); + } + + var owner = repo3.findById(ownerId).orElseThrow(); + owner.setValue("owner_updated"); + var rel = owner.getRelatedNodes().get(0); + rel.setRelValue("whatever"); + rel.getTarget().setValue("owned_updated"); + + repo3.save(owner); + + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var count = session.run(adaptQueryTo44IfNecessary(""" + MATCH (n:NodeWithGeneratedId3 {value: $v1}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1 {value: $v2}) + WHERE elementId(n) = $id1 + AND r.relValue IN $rv + RETURN count(*)"""), + Map.of("v1", "owner_updated", "v2", "owned_updated", "id1", owner.getId(), "rv", List.of("whatever"))).single().get(0).asLong(); + assertThat(count).isEqualTo(1L); + } + } + + @Test + void relsWithsCyclesOnCreation(LogbackCapture logbackCapture, @Autowired Repo4 repo4, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + var owner = new NodeWithGeneratedId4("owner"); + var intermediate = new NodeWithGeneratedId4.Intermediate(); + intermediate.setEnd(new NodeWithGeneratedId4("end")); + owner.setIntermediate(intermediate); + + owner = repo4.save(owner); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + var validIdForCurrentNeo4j = validIdForCurrentNeo4j(); + assertThat(owner.getId()).matches(validIdForCurrentNeo4j); + assertThat(owner.getIntermediate().getId()).matches(validIdForCurrentNeo4j); + assertThat(owner.getIntermediate().getEnd().getId()).matches(validIdForCurrentNeo4j); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var query = """ + MATCH (n:NodeWithGeneratedId4 {value: $v1}) -[r:INTERMEDIATE]-> (i:Intermediate) -[:END]-> (e:NodeWithGeneratedId4 {value: $v2}) + WHERE elementId(n) = $id1 + AND elementId(i) = $id2 + AND elementId(e) = $id3 + RETURN count(*)"""; + var count = session.run(adaptQueryTo44IfNecessary(query), + Map.of("v1", "owner", "v2", "end", "id1", owner.getId(), "id2", owner.getIntermediate().getId(), "id3", owner.getIntermediate().getEnd().getId())) + .single().get(0).asLong(); + assertThat(count).isEqualTo(1L); + } + } + + @Test + void relsWithsCyclesOnUpdate(LogbackCapture logbackCapture, @Autowired Repo4 repo4, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + String ownerId; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + ownerId = session.run(adaptQueryTo44IfNecessary(""" + CREATE (n:NodeWithGeneratedId4 {value: 'a'}) -[r:INTERMEDIATE]-> (i:Intermediate) -[:END]-> (e:NodeWithGeneratedId4 {value: 'b'}) + RETURN elementId(n)""")) + .single().get(0).asString(); + } + + var owner = repo4.findAllById(List.of(ownerId)).get(0); + owner.setValue("owner"); + owner.getIntermediate().getEnd().setValue("end"); + + owner = repo4.save(owner); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + var validIdForCurrentNeo4j = validIdForCurrentNeo4j(); + assertThat(owner.getId()).matches(validIdForCurrentNeo4j); + assertThat(owner.getIntermediate().getId()).matches(validIdForCurrentNeo4j); + assertThat(owner.getIntermediate().getEnd().getId()).matches(validIdForCurrentNeo4j); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var count = session.run(adaptQueryTo44IfNecessary(""" + MATCH (n:NodeWithGeneratedId4 {value: $v1}) -[r:INTERMEDIATE]-> (i:Intermediate) -[:END]-> (e:NodeWithGeneratedId4 {value: $v2}) + WHERE elementId(n) = $id1 + AND elementId(i) = $id2 + AND elementId(e) = $id3 + RETURN count(*)"""), + Map.of("v1", "owner", "v2", "end", "id1", owner.getId(), "id2", owner.getIntermediate().getId(), "id3", owner.getIntermediate().getEnd().getId())) + .single().get(0).asLong(); + assertThat(count).isEqualTo(1L); + } + } + + private static void assertThatLogMessageDoNotIndicateIDUsage(LogbackCapture logbackCapture) { + assertThat(logbackCapture.getFormattedMessages()) + .noneMatch(s -> s.contains("Neo.ClientNotification.Statement.FeatureDeprecationWarning") || + s.contains("The query used a deprecated function. ('id' is no longer supported)")); + } + + @Configuration + @EnableTransactionManagement + @EnableNeo4jRepositories(considerNestedRepositories = true) + static class Config extends Neo4jImperativeTestConfiguration { + + @Bean + public BookmarkCapture bookmarkCapture() { + return new BookmarkCapture(); + } + + @Override + @NonNull + public PlatformTransactionManager transactionManager(@NonNull Driver driver, @NonNull DatabaseSelectionProvider databaseNameProvider) { + + BookmarkCapture bookmarkCapture = bookmarkCapture(); + return new Neo4jTransactionManager(driver, databaseNameProvider, + Neo4jBookmarkManager.create(bookmarkCapture)); + } + + @Bean + @NonNull + public Driver driver() { + + return neo4jConnectionSupport.getDriver(); + } + + @Override + public boolean isCypher5Compatible() { + return neo4jConnectionSupport.isCypher5SyntaxCompatible(); + } + } +} diff --git a/src/main/java/org/springframework/data/neo4j/core/schema/DefaultElementId.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId1.java similarity index 50% rename from src/main/java/org/springframework/data/neo4j/core/schema/DefaultElementId.java rename to src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId1.java index f4b49efde..fc9a61040 100644 --- a/src/main/java/org/springframework/data/neo4j/core/schema/DefaultElementId.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId1.java @@ -13,15 +13,41 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.neo4j.core.schema; +package org.springframework.data.neo4j.integration.issues.pure_element_id; + +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; +import org.springframework.data.neo4j.core.schema.Node; /** - * Default representation of an element id. - * * @author Michael J. Simons - * @param value The ids value - * @soundtrack GötterDÄmmerung - Tribut an die beste Band der Welt - * @since 7.0.0 */ -record DefaultElementId(String value) implements ElementId { +@Node +public class NodeWithGeneratedId1 { + + @Id + @GeneratedValue + private String id; + + private String value; + + public NodeWithGeneratedId1(String value) { + this.value = value; + } + + public void setId(String id) { + this.id = id; + } + + public String getId() { + return id; + } + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } } diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId2.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId2.java new file mode 100644 index 000000000..357e54a21 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId2.java @@ -0,0 +1,63 @@ +/* + * Copyright 2011-2023 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.pure_element_id; + +import java.util.List; + +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; +import org.springframework.data.neo4j.core.schema.Node; +import org.springframework.data.neo4j.core.schema.Relationship; + +/** + * @author Michael J. Simons + */ +@Node +public class NodeWithGeneratedId2 { + + @Id + @GeneratedValue + private String id; + + private String value; + + @Relationship + private List relatedNodes; + + public NodeWithGeneratedId2(String value) { + this.value = value; + } + + public String getId() { + return id; + } + + public String getValue() { + return value; + } + + public List getRelatedNodes() { + return relatedNodes; + } + + public void setRelatedNodes(List relatedNodes) { + this.relatedNodes = relatedNodes; + } + + public void setValue(String value) { + this.value = value; + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId3.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId3.java new file mode 100644 index 000000000..4fbac163b --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId3.java @@ -0,0 +1,63 @@ +/* + * Copyright 2011-2023 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.pure_element_id; + +import java.util.List; + +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; +import org.springframework.data.neo4j.core.schema.Node; +import org.springframework.data.neo4j.core.schema.Relationship; + +/** + * @author Michael J. Simons + */ +@Node +public class NodeWithGeneratedId3 { + + @Id + @GeneratedValue + private String id; + + private String value; + + @Relationship + private List relatedNodes; + + public NodeWithGeneratedId3(String value) { + this.value = value; + } + + public String getId() { + return id; + } + + public String getValue() { + return value; + } + + public List getRelatedNodes() { + return relatedNodes; + } + + public void setRelatedNodes(List relatedNodes) { + this.relatedNodes = relatedNodes; + } + + public void setValue(String value) { + this.value = value; + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId4.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId4.java new file mode 100644 index 000000000..f5bd9150a --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/NodeWithGeneratedId4.java @@ -0,0 +1,88 @@ +/* + * Copyright 2011-2023 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.pure_element_id; + +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; +import org.springframework.data.neo4j.core.schema.Node; +import org.springframework.data.neo4j.core.schema.Relationship; + +/** + * @author Michael J. Simons + */ +@Node +public class NodeWithGeneratedId4 { + + + @Node + static class Intermediate { + @Id + @GeneratedValue + private String id; + + @Relationship + NodeWithGeneratedId4 end; + + String getId() { + return id; + } + + void setId(String id) { + this.id = id; + } + + NodeWithGeneratedId4 getEnd() { + return end; + } + + void setEnd(NodeWithGeneratedId4 end) { + this.end = end; + } + } + + @Id + @GeneratedValue + private String id; + + private String value; + + @Relationship + private Intermediate intermediate; + + public NodeWithGeneratedId4(String value) { + this.value = value; + } + + public String getId() { + return id; + } + + public String getValue() { + return value; + } + + public Intermediate getIntermediate() { + return intermediate; + } + + public void setIntermediate(Intermediate intermediate) { + this.intermediate = intermediate; + } + + public void setValue(String value) { + this.value = value; + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ReactiveElementIdIT.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ReactiveElementIdIT.java new file mode 100644 index 000000000..6e8a129bc --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ReactiveElementIdIT.java @@ -0,0 +1,376 @@ +/* + * Copyright 2011-2023 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.pure_element_id; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.neo4j.driver.Driver; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.neo4j.core.ReactiveDatabaseSelectionProvider; +import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; +import org.springframework.data.neo4j.core.transaction.ReactiveNeo4jTransactionManager; +import org.springframework.data.neo4j.repository.ReactiveNeo4jRepository; +import org.springframework.data.neo4j.repository.config.EnableReactiveNeo4jRepositories; +import org.springframework.data.neo4j.test.BookmarkCapture; +import org.springframework.data.neo4j.test.LogbackCapture; +import org.springframework.data.neo4j.test.LogbackCapturingExtension; +import org.springframework.data.neo4j.test.Neo4jIntegrationTest; +import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration; +import org.springframework.lang.NonNull; +import org.springframework.transaction.ReactiveTransactionManager; +import org.springframework.transaction.annotation.EnableTransactionManagement; + +/** + * Assertions that no {@code id()} calls are generated when no deprecated id types are present. + * This test deliberately uses blocking calls into reactor because it's really not about the reactive flows but about + * catching all the code paths that might interact with ids on the reactive side of things. Yes, Reactors testing tools + * are known. + * + * @author Michael J. Simons + */ +@Neo4jIntegrationTest +@ExtendWith(LogbackCapturingExtension.class) +public class ReactiveElementIdIT extends AbstractElementIdTestBase { + + + interface Repo1 extends ReactiveNeo4jRepository { + } + + interface Repo2 extends ReactiveNeo4jRepository { + } + + interface Repo3 extends ReactiveNeo4jRepository { + } + + interface Repo4 extends ReactiveNeo4jRepository { + } + + + @Test + void simpleNodeCreationShouldFillIdAndNotUseIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1) { + + var node = repo1.save(new NodeWithGeneratedId1("from-sdn-repo")).block(); + assertThat(node).isNotNull(); + assertThat(node.getId()) + .matches(validIdForCurrentNeo4j()); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void simpleNodeAllCreationShouldFillIdAndNotUseIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1) { + + var nodes = repo1.saveAll(List.of(new NodeWithGeneratedId1("from-sdn-repo"))).collectList().block(); + assertThat(nodes).isNotEmpty() + .extracting(NodeWithGeneratedId1::getId) + .allMatch(validIdForCurrentNeo4j()); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void findByIdMustNotCallIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + String id; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + id = session.run("CREATE (n:NodeWithGeneratedId1 {value: 'whatever'}) RETURN n").single().get("n").asNode().elementId(); + } + + var optionalNode = Optional.ofNullable(repo1.findById(id).block()); + assertThat(optionalNode).map(NodeWithGeneratedId1::getValue) + .hasValue("whatever"); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void findAllMustNotCallIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + session.run("CREATE (n:NodeWithGeneratedId1 {value: 'whatever'}) RETURN n").single().get("n").asNode().elementId(); + } + + var nodes = repo1.findAll().collectList().block(); + assertThat(nodes).isNotEmpty() + .extracting(NodeWithGeneratedId1::getId) + .allMatch(validIdForCurrentNeo4j()); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void updateMustNotCallIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + NodeWithGeneratedId1 node; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var dbNode = session.run("CREATE (n:NodeWithGeneratedId1 {value: 'whatever'}) RETURN n").single().get("n").asNode(); + node = new NodeWithGeneratedId1(dbNode.get("value").asString() + "_edited"); + node.setId(dbNode.elementId()); + } + + node = repo1.save(node).block(); + assertThat(node).extracting(NodeWithGeneratedId1::getValue) + .isEqualTo("whatever_edited"); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void updateAllMustNotCallIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + NodeWithGeneratedId1 node; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var dbNode = session.run("CREATE (n:NodeWithGeneratedId1 {value: 'whatever'}) RETURN n").single().get("n").asNode(); + node = new NodeWithGeneratedId1(dbNode.get("value").asString() + "_edited"); + node.setId(dbNode.elementId()); + } + + var nodes = repo1.saveAll(List.of(node)).collectList().block(); + assertThat(nodes).isNotEmpty() + .extracting(NodeWithGeneratedId1::getId) + .allMatch(validIdForCurrentNeo4j()); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void nodeAndRelationshipsWithoutPropsAndIdsMustNotUseIdFunctionWhileCreating(LogbackCapture logbackCapture, @Autowired Repo2 repo2, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + var owner = new NodeWithGeneratedId2("owner"); + owner.setRelatedNodes(List.of(new NodeWithGeneratedId1("child1"), new NodeWithGeneratedId1("child2"))); + owner = repo2.save(owner).block(); + + assertThat(owner).isNotNull(); + assertThat(owner.getId()).isNotNull(); + assertThat(owner.getRelatedNodes()) + .allSatisfy(owned -> assertThat(owned.getId()).matches(validIdForCurrentNeo4j())); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + } + + @Test + void nodeAndRelationshipsWithoutPropsAndIdsMustNotUseIdFunctionWhileUpdating(LogbackCapture logbackCapture, @Autowired Repo2 repo2, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + String ownerId; + String ownedId; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var row = session.run("CREATE (n:NodeWithGeneratedId2 {value: 'owner'}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1 {value:'owned'}) RETURN *").single(); + ownerId = row.get("n").asNode().elementId(); + ownedId = row.get("m").asNode().elementId(); + } + + var owner = repo2.findById(ownerId).block(); + assertThat(owner).isNotNull(); + assertThat(owner.getRelatedNodes()) + .hasSize(1) + .first() + .extracting(NodeWithGeneratedId1::getId) + .isEqualTo(ownedId); + + + owner.getRelatedNodes().get(0).setValue("owned_changed"); + owner.setValue("owner_changed"); + + repo2.save(owner).block(); + + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var count = session.run(adaptQueryTo44IfNecessary(""" + MATCH (n:NodeWithGeneratedId2 {value: $v1}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1 {value: $v2}) + WHERE elementId(n) = $id1 AND elementId(m) = $id2 + RETURN count(*)"""), + Map.of("v1", "owner_changed", "v2", "owned_changed", "id1", ownerId, "id2", ownedId)).single().get(0).asLong(); + assertThat(count).isOne(); + } + } + + @Test + void relsWithPropOnCreation(LogbackCapture logbackCapture, @Autowired Repo3 repo3, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + var owner = new NodeWithGeneratedId3("owner"); + var target1 = new NodeWithGeneratedId1("target1"); + var target2 = new NodeWithGeneratedId1("target2"); + + owner.setRelatedNodes( + List.of( + new RelWithProps(target1, "vr1"), + new RelWithProps(target2, "vr2")) + ); + + owner = repo3.save(owner).block(); + assertThat(owner).isNotNull(); + assertThat(owner.getId()).matches(validIdForCurrentNeo4j()); + assertThat(owner.getRelatedNodes()) + .hasSize(2) + .allSatisfy(r -> assertThat(r.getTarget().getId()).isNotNull()) + .extracting(RelWithProps::getRelValue) + .containsExactlyInAnyOrder("vr1", "vr2"); + + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var count = session.run(adaptQueryTo44IfNecessary(""" + MATCH (n:NodeWithGeneratedId3 {value: $v1}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1) + WHERE elementId(n) = $id1 + AND r.relValue IN $rv + RETURN count(*)"""), + Map.of("v1", "owner", "id1", owner.getId(), "rv", List.of("vr1", "vr2"))).single().get(0).asLong(); + assertThat(count).isEqualTo(2L); + } + } + + @Test + void relsWithPropOnUpdate(LogbackCapture logbackCapture, @Autowired Repo3 repo3, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + String ownerId; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + ownerId = session.run(adaptQueryTo44IfNecessary(""" + CREATE (n:NodeWithGeneratedId3 {value: 'owner'}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1 {value: 'owned'}) + RETURN elementId(n)""") + ).single().get(0).asString(); + } + + var owner = repo3.findById(ownerId).block(); + assertThat(owner).isNotNull(); + owner.setValue("owner_updated"); + var rel = owner.getRelatedNodes().get(0); + rel.setRelValue("whatever"); + rel.getTarget().setValue("owned_updated"); + + repo3.save(owner).block(); + + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var count = session.run(adaptQueryTo44IfNecessary(""" + MATCH (n:NodeWithGeneratedId3 {value: $v1}) -[r:RELATED_NODES] -> (m:NodeWithGeneratedId1 {value: $v2}) + WHERE elementId(n) = $id1 + AND r.relValue IN $rv + RETURN count(*)"""), + Map.of("v1", "owner_updated", "v2", "owned_updated", "id1", owner.getId(), "rv", List.of("whatever"))).single().get(0).asLong(); + assertThat(count).isEqualTo(1L); + } + } + + @Test + void relsWithsCyclesOnCreation(LogbackCapture logbackCapture, @Autowired Repo4 repo4, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + var owner = new NodeWithGeneratedId4("owner"); + var intermediate = new NodeWithGeneratedId4.Intermediate(); + intermediate.setEnd(new NodeWithGeneratedId4("end")); + owner.setIntermediate(intermediate); + + owner = repo4.save(owner).block(); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + var validIdForCurrentNeo4j = validIdForCurrentNeo4j(); + assertThat(owner).isNotNull(); + assertThat(owner.getId()).matches(validIdForCurrentNeo4j); + assertThat(owner.getIntermediate().getId()).matches(validIdForCurrentNeo4j); + assertThat(owner.getIntermediate().getEnd().getId()).matches(validIdForCurrentNeo4j); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var query = """ + MATCH (n:NodeWithGeneratedId4 {value: $v1}) -[r:INTERMEDIATE]-> (i:Intermediate) -[:END]-> (e:NodeWithGeneratedId4 {value: $v2}) + WHERE elementId(n) = $id1 + AND elementId(i) = $id2 + AND elementId(e) = $id3 + RETURN count(*)"""; + var count = session.run(adaptQueryTo44IfNecessary(query), + Map.of("v1", "owner", "v2", "end", "id1", owner.getId(), "id2", owner.getIntermediate().getId(), "id3", owner.getIntermediate().getEnd().getId())) + .single().get(0).asLong(); + assertThat(count).isEqualTo(1L); + } + } + + @Test + void relsWithsCyclesOnUpdate(LogbackCapture logbackCapture, @Autowired Repo4 repo4, @Autowired BookmarkCapture bookmarkCapture, @Autowired Driver driver) { + + String ownerId; + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + ownerId = session.run(adaptQueryTo44IfNecessary(""" + CREATE (n:NodeWithGeneratedId4 {value: 'a'}) -[r:INTERMEDIATE]-> (i:Intermediate) -[:END]-> (e:NodeWithGeneratedId4 {value: 'b'}) + RETURN elementId(n)""")) + .single().get(0).asString(); + } + + var owner = repo4.findAllById(List.of(ownerId)).blockFirst(); + assertThat(owner).isNotNull(); + owner.setValue("owner"); + owner.getIntermediate().getEnd().setValue("end"); + + owner = repo4.save(owner).block(); + assertThatLogMessageDoNotIndicateIDUsage(logbackCapture); + + var validIdForCurrentNeo4j = validIdForCurrentNeo4j(); + assertThat(owner).isNotNull(); + assertThat(owner.getId()).matches(validIdForCurrentNeo4j); + assertThat(owner.getIntermediate().getId()).matches(validIdForCurrentNeo4j); + assertThat(owner.getIntermediate().getEnd().getId()).matches(validIdForCurrentNeo4j); + + try (var session = driver.session(bookmarkCapture.createSessionConfig())) { + var count = session.run(adaptQueryTo44IfNecessary(""" + MATCH (n:NodeWithGeneratedId4 {value: $v1}) -[r:INTERMEDIATE]-> (i:Intermediate) -[:END]-> (e:NodeWithGeneratedId4 {value: $v2}) + WHERE elementId(n) = $id1 + AND elementId(i) = $id2 + AND elementId(e) = $id3 + RETURN count(*)"""), + Map.of("v1", "owner", "v2", "end", "id1", owner.getId(), "id2", owner.getIntermediate().getId(), "id3", owner.getIntermediate().getEnd().getId())) + .single().get(0).asLong(); + assertThat(count).isEqualTo(1L); + } + } + + private static void assertThatLogMessageDoNotIndicateIDUsage(LogbackCapture logbackCapture) { + assertThat(logbackCapture.getFormattedMessages()) + .noneMatch(s -> s.contains("Neo.ClientNotification.Statement.FeatureDeprecationWarning") || + s.contains("The query used a deprecated function. ('id' is no longer supported)")); + } + + @Configuration + @EnableTransactionManagement + @EnableReactiveNeo4jRepositories(considerNestedRepositories = true) + static class Config extends Neo4jReactiveTestConfiguration { + + @Bean + public BookmarkCapture bookmarkCapture() { + return new BookmarkCapture(); + } + + @Override + @NonNull + public ReactiveTransactionManager reactiveTransactionManager(@NonNull Driver driver, @NonNull ReactiveDatabaseSelectionProvider databaseSelectionProvider) { + + BookmarkCapture bookmarkCapture = bookmarkCapture(); + return new ReactiveNeo4jTransactionManager(driver, databaseSelectionProvider, + Neo4jBookmarkManager.create(bookmarkCapture)); + } + + @Bean + @NonNull + public Driver driver() { + + return neo4jConnectionSupport.getDriver(); + } + + @Override + public boolean isCypher5Compatible() { + return neo4jConnectionSupport.isCypher5SyntaxCompatible(); + } + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/RelWithProps.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/RelWithProps.java new file mode 100644 index 000000000..b77465fca --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/RelWithProps.java @@ -0,0 +1,64 @@ +/* + * Copyright 2011-2023 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.pure_element_id; + +import org.springframework.data.neo4j.core.schema.RelationshipId; +import org.springframework.data.neo4j.core.schema.RelationshipProperties; +import org.springframework.data.neo4j.core.schema.TargetNode; + +/** + * @author Michael J. Simons + */ +@RelationshipProperties +public class RelWithProps { + + @RelationshipId + private String id; + + @TargetNode + private NodeWithGeneratedId1 target; + + private String relValue; + + public RelWithProps(NodeWithGeneratedId1 target, String relValue) { + this.target = target; + this.relValue = relValue; + } + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public String getRelValue() { + return relValue; + } + + public void setRelValue(String relValue) { + this.relValue = relValue; + } + + public NodeWithGeneratedId1 getTarget() { + return target; + } + + public void setTarget(NodeWithGeneratedId1 target) { + this.target = target; + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jClientIT.java b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jClientIT.java index 6ec6b59c3..262ab12d4 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jClientIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jClientIT.java @@ -17,23 +17,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import org.neo4j.cypherdsl.core.Statement; -import org.neo4j.cypherdsl.core.executables.ExecutableResultStatement; -import org.neo4j.driver.Query; -import org.neo4j.driver.Record; -import org.neo4j.driver.SimpleQueryRunner; -import org.neo4j.driver.async.AsyncQueryRunner; -import org.neo4j.driver.reactivestreams.ReactiveQueryRunner; -import org.neo4j.driver.reactivestreams.ReactiveResult; -import org.neo4j.driver.summary.ResultSummary; -import org.reactivestreams.Publisher; -import org.springframework.data.neo4j.core.mapping.IdentitySupport; -import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration; - -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; -import reactor.test.StepVerifier; - import java.util.Collection; import java.util.Collections; import java.util.List; @@ -48,9 +31,19 @@ import org.junit.jupiter.api.Test; import org.neo4j.cypherdsl.core.Cypher; import org.neo4j.cypherdsl.core.Node; +import org.neo4j.cypherdsl.core.Statement; +import org.neo4j.cypherdsl.core.executables.ExecutableResultStatement; import org.neo4j.driver.Driver; +import org.neo4j.driver.Query; +import org.neo4j.driver.Record; import org.neo4j.driver.Session; +import org.neo4j.driver.SimpleQueryRunner; import org.neo4j.driver.Transaction; +import org.neo4j.driver.async.AsyncQueryRunner; +import org.neo4j.driver.reactivestreams.ReactiveQueryRunner; +import org.neo4j.driver.reactivestreams.ReactiveResult; +import org.neo4j.driver.summary.ResultSummary; +import org.reactivestreams.Publisher; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -62,10 +55,16 @@ import org.springframework.data.neo4j.test.BookmarkCapture; import org.springframework.data.neo4j.test.Neo4jExtension.Neo4jConnectionSupport; import org.springframework.data.neo4j.test.Neo4jIntegrationTest; +import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration; +import org.springframework.data.neo4j.test.TestIdentitySupport; import org.springframework.transaction.ReactiveTransactionManager; import org.springframework.transaction.annotation.EnableTransactionManagement; import org.springframework.transaction.reactive.TransactionalOperator; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + /** * @author Michael J. Simons */ @@ -99,7 +98,7 @@ void clientShouldIntegrateWithCypherDSL(@Autowired TransactionalOperator transac transactionalOperator.execute(transaction -> { Flux inner = client.getQueryRunner() .flatMapMany(statement::fetchWith) - .doOnNext(r -> vanishedId.set(IdentitySupport.getInternalId(r.get("n").asNode()))) + .doOnNext(r -> vanishedId.set(TestIdentitySupport.getInternalId(r.get("n").asNode()))) .map(record -> record.get("n").get("value").asLong()); transaction.setRollbackOnly(); diff --git a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java index 86339e361..e2df46d75 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java @@ -19,16 +19,6 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.tuple; -import org.neo4j.driver.TransactionContext; -import org.neo4j.driver.reactivestreams.ReactiveResult; -import org.neo4j.driver.reactivestreams.ReactiveSession; -import org.springframework.data.neo4j.core.mapping.IdentitySupport; -import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration; - -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; -import reactor.test.StepVerifier; - import java.time.LocalDate; import java.util.ArrayList; import java.util.Arrays; @@ -54,8 +44,11 @@ import org.neo4j.driver.Driver; import org.neo4j.driver.Record; import org.neo4j.driver.Session; +import org.neo4j.driver.TransactionContext; import org.neo4j.driver.Value; import org.neo4j.driver.Values; +import org.neo4j.driver.reactivestreams.ReactiveResult; +import org.neo4j.driver.reactivestreams.ReactiveSession; import org.neo4j.driver.types.Node; import org.neo4j.driver.types.Point; import org.neo4j.driver.types.Relationship; @@ -111,6 +104,8 @@ import org.springframework.data.neo4j.repository.query.Query; import org.springframework.data.neo4j.test.BookmarkCapture; import org.springframework.data.neo4j.test.Neo4jExtension; +import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration; +import org.springframework.data.neo4j.test.TestIdentitySupport; import org.springframework.data.neo4j.types.CartesianPoint2d; import org.springframework.data.neo4j.types.GeographicPoint2d; import org.springframework.data.repository.query.FluentQuery; @@ -122,6 +117,10 @@ import org.springframework.transaction.annotation.EnableTransactionManagement; import org.springframework.transaction.reactive.TransactionalOperator; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + /** * @author Gerrit Meier * @author Michael J. Simons @@ -440,10 +439,10 @@ void findEntityWithRelationshipByFindOneByExample(@Autowired ReactiveRelationshi Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship probe = new PersonWithRelationship(); probe.setName("Freddie"); @@ -481,10 +480,10 @@ void findEntityWithRelationshipByFindAllByExample(@Autowired ReactiveRelationshi Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship probe = new PersonWithRelationship(); probe.setName("Freddie"); @@ -522,10 +521,10 @@ void findEntityWithRelationshipByFindAllByExampleWithSort(@Autowired ReactiveRel Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); PersonWithRelationship probe = new PersonWithRelationship(); probe.setName("Freddie"); @@ -844,12 +843,12 @@ void loadEntityWithRelationship(@Autowired ReactiveRelationshipRepository reposi Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long clubId = IdentitySupport.getInternalId(clubNode); - long hobbyNode1Id = IdentitySupport.getInternalId(hobbyNode1); - long hobbyNode2Id = IdentitySupport.getInternalId(hobbyNode2); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long clubId = TestIdentitySupport.getInternalId(clubNode); + long hobbyNode1Id = TestIdentitySupport.getInternalId(hobbyNode1); + long hobbyNode2Id = TestIdentitySupport.getInternalId(hobbyNode2); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); StepVerifier.create(repository.findById(personId)).assertNext(loadedPerson -> { @@ -880,7 +879,7 @@ void loadEntityWithRelationship(@Autowired ReactiveRelationshipRepository reposi } @Test - void loadEntityWithRelationshipToTheSameNode(@Autowired ReactiveRelationshipRepository repository) { + void findEntityWithRelationshipToTheSameNode(@Autowired ReactiveRelationshipRepository repository) { Record record = doWithSession(session -> session .run(""" @@ -895,9 +894,9 @@ void loadEntityWithRelationshipToTheSameNode(@Autowired ReactiveRelationshipRepo Node hobbyNode1 = record.get("h1").asNode(); Node petNode1 = record.get("p1").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNode1Id = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNode1Id = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); StepVerifier.create(repository.findById(personId)).assertNext(loadedPerson -> { @@ -938,7 +937,7 @@ void loadLoopingDeepRelationships(@Autowired ReactiveLoopingRelationshipReposito RETURN t1 """).single(); - return IdentitySupport.getInternalId(record.get("t1").asNode()); + return TestIdentitySupport.getInternalId(record.get("t1").asNode()); }); StepVerifier.create(loopingRelationshipRepository.findById(type1Id)).assertNext(type1 -> { @@ -969,18 +968,17 @@ void loadLoopingDeepRelationships(@Autowired ReactiveLoopingRelationshipReposito @Test void loadEntityWithBidirectionalRelationship(@Autowired BidirectionalStartRepository repository) { - long startId = doWithSession(session -> { + Node startNode = doWithSession(session -> { Record record = session.run("CREATE (n:BidirectionalStart{name:'Ernie'})-[:CONNECTED]->(e:BidirectionalEnd{name:'Bert'}) RETURN n").single(); - Node startNode = record.get("n").asNode(); - return IdentitySupport.getInternalId(startNode); + return record.get("n").asNode(); }); - StepVerifier.create(repository.findById(startId)) + StepVerifier.create(repository.findById(TestIdentitySupport.getInternalId(startNode))) .verifyErrorMatches(error -> { Throwable cause = error.getCause(); return cause instanceof MappingException && cause.getMessage().equals( - "The node with id " + startId + " has a logical cyclic mapping dependency; " + + "The node with id " + startNode.elementId() + " has a logical cyclic mapping dependency; " + "its creation caused the creation of another node that has a reference to this"); }); @@ -993,7 +991,7 @@ void loadEntityWithBidirectionalRelationshipFromIncomingSide(@Autowired Bidirect Record record = session.run("CREATE (n:BidirectionalStart{name:'Ernie'})-[:CONNECTED]->(e:BidirectionalEnd{name:'Bert'}) RETURN e").single(); Node endNode = record.get("e").asNode(); - return IdentitySupport.getInternalId(endNode); + return TestIdentitySupport.getInternalId(endNode); }); StepVerifier.create(repository.findById(endId)).assertNext(entity -> { @@ -1008,8 +1006,8 @@ void loadMultipleEntitiesWithRelationship(@Autowired ReactiveRelationshipReposit .run("CREATE (n:PersonWithRelationship{name:'Freddie'})-[:Has]->(h:Hobby{name:'Music'}), (n)-[:Has]->(p:Pet{name: 'Jerry'}) RETURN n, h, p") .single()); - long hobbyNode1Id = IdentitySupport.getInternalId(record.get("h").asNode()); - long petNode1Id = IdentitySupport.getInternalId(record.get("p").asNode()); + long hobbyNode1Id = TestIdentitySupport.getInternalId(record.get("h").asNode()); + long petNode1Id = TestIdentitySupport.getInternalId(record.get("p").asNode()); record = doWithSession(session -> session.run(""" CREATE @@ -1018,8 +1016,8 @@ record = doWithSession(session -> session.run(""" RETURN n, h, p """).single()); - long hobbyNode2Id = IdentitySupport.getInternalId(record.get("h").asNode()); - long petNode2Id = IdentitySupport.getInternalId(record.get("p").asNode()); + long hobbyNode2Id = TestIdentitySupport.getInternalId(record.get("h").asNode()); + long petNode2Id = TestIdentitySupport.getInternalId(record.get("p").asNode()); StepVerifier.create(repository.findAll()).recordWith(ArrayList::new).expectNextCount(2) .consumeRecordedWith(loadedPersons -> { @@ -1058,10 +1056,10 @@ void loadEntityWithRelationshipViaQuery(@Autowired ReactiveRelationshipRepositor Node petNode1 = record.get("p1").asNode(); Node petNode2 = record.get("p2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNodeId = IdentitySupport.getInternalId(hobbyNode1); - long petNode1Id = IdentitySupport.getInternalId(petNode1); - long petNode2Id = IdentitySupport.getInternalId(petNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNodeId = TestIdentitySupport.getInternalId(hobbyNode1); + long petNode1Id = TestIdentitySupport.getInternalId(petNode1); + long petNode2Id = TestIdentitySupport.getInternalId(petNode2); StepVerifier.create(repository.getPersonWithRelationshipsViaQuery()).assertNext(loadedPerson -> { assertThat(loadedPerson.getName()).isEqualTo("Freddie"); @@ -1085,7 +1083,7 @@ void loadEntityWithRelationshipWithAssignedId(@Autowired ReactivePetRepository r Record record = session.run("CREATE (p:Pet{name:'Jerry'})-[:Has]->(t:Thing{theId:'t1', name:'Thing1'}) RETURN p, t").single(); Node petNode = record.get("p").asNode(); - return IdentitySupport.getInternalId(petNode); + return TestIdentitySupport.getInternalId(petNode); }); StepVerifier.create(repository.findById(petNodeId)).assertNext(pet -> { @@ -1182,9 +1180,9 @@ void loadEntityWithRelationshipWithProperties(@Autowired ReactivePersonWithRelat Node hobbyNode1 = record.get("h1").asNode(); Node hobbyNode2 = record.get("h2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNode1Id = IdentitySupport.getInternalId(hobbyNode1); - long hobbyNode2Id = IdentitySupport.getInternalId(hobbyNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNode1Id = TestIdentitySupport.getInternalId(hobbyNode1); + long hobbyNode2Id = TestIdentitySupport.getInternalId(hobbyNode2); StepVerifier.create(repository.findById(personId)).assertNext(person -> { assertThat(person.getName()).isEqualTo("Freddie"); @@ -1324,9 +1322,9 @@ void loadEntityWithRelationshipWithPropertiesFromCustomQuery( Node hobbyNode1 = record.get("h1").asNode(); Node hobbyNode2 = record.get("h2").asNode(); - long personId = IdentitySupport.getInternalId(personNode); - long hobbyNode1Id = IdentitySupport.getInternalId(hobbyNode1); - long hobbyNode2Id = IdentitySupport.getInternalId(hobbyNode2); + long personId = TestIdentitySupport.getInternalId(personNode); + long hobbyNode1Id = TestIdentitySupport.getInternalId(hobbyNode1); + long hobbyNode2Id = TestIdentitySupport.getInternalId(hobbyNode2); StepVerifier.create(repository.loadFromCustomQuery(personId)).assertNext(person -> { assertThat(person.getName()).isEqualTo("Freddie"); @@ -1365,7 +1363,7 @@ void loadEntityWithRelationshipWithPropertiesFromCustomQueryIncoming( long personId = doWithSession(session -> { Record record = session.run("CREATE (n:AltPerson{name:'Freddie'}), (n)-[l1:LIKES {rating: 5}]->(h1:AltHobby{name:'Music'}) RETURN n, h1").single(); - return IdentitySupport.getInternalId(record.get("n").asNode()); + return TestIdentitySupport.getInternalId(record.get("n").asNode()); }); StepVerifier.create(repository.loadFromCustomQuery(personId)).assertNext(hobby -> { @@ -1386,7 +1384,7 @@ void loadSameNodeWithDoubleRelationship(@Autowired ReactiveHobbyWithRelationship " (n)-[l1:LIKES {rating: 5}]->(h1:AltHobby{name:'Music'})," + " (n)-[l2:LIKES {rating: 1}]->(h1)" + " RETURN n, h1").single(); - return IdentitySupport.getInternalId(record.get("n").asNode()); + return TestIdentitySupport.getInternalId(record.get("n").asNode()); }); StepVerifier.create(repository.loadFromCustomQuery(personId)).assertNext(hobby -> { @@ -1907,7 +1905,7 @@ void saveSingleEntityWithRelationships(@Autowired ReactiveRelationshipRepository assertThat(record.containsKey("n")).isTrue(); Node rootNode = record.get("n").asNode(); - assertThat(ids.get(0)).isEqualTo(IdentitySupport.getInternalId(rootNode)); + assertThat(ids.get(0)).isEqualTo(TestIdentitySupport.getInternalId(rootNode)); assertThat(rootNode.get("name").asString()).isEqualTo("Freddie"); List> petsWithHobbies = record.get("petsWithHobbies").asList(Value::asList); @@ -1977,7 +1975,7 @@ void saveSingleEntityWithRelationshipsTwiceDoesNotCreateMoreRelationships( assertThat(record.containsKey("n")).isTrue(); Node rootNode = record.get("n").asNode(); - assertThat(ids.get(0)).isEqualTo(IdentitySupport.getInternalId(rootNode)); + assertThat(ids.get(0)).isEqualTo(TestIdentitySupport.getInternalId(rootNode)); assertThat(rootNode.get("name").asString()).isEqualTo("Freddie"); List> petsWithHobbies = record.get("petsWithHobbies").asList(Value::asList); @@ -2038,7 +2036,7 @@ void saveEntityWithAlreadyExistingTargetNode(@Autowired ReactiveRelationshipRepo assertThat(record.containsKey("n")).isTrue(); Node rootNode = record.get("n").asNode(); - assertThat(ids.get(0)).isEqualTo(IdentitySupport.getInternalId(rootNode)); + assertThat(ids.get(0)).isEqualTo(TestIdentitySupport.getInternalId(rootNode)); assertThat(rootNode.get("name").asString()).isEqualTo("Freddie"); assertThat(record.get("hobbies").asList(entry -> entry.asNode().get("name").asString())) @@ -2504,9 +2502,9 @@ void findNodeWithMultipleLabels(@Autowired ReactiveMultipleLabelRepository repos long n3Id; Record record = doWithSession(session -> session.run("CREATE (n1:A:B:C), (n2:B:C), (n3:A) return n1, n2, n3").single()); - n1Id = IdentitySupport.getInternalId(record.get("n1").asNode()); - n2Id = IdentitySupport.getInternalId(record.get("n2").asNode()); - n3Id = IdentitySupport.getInternalId(record.get("n3").asNode()); + n1Id = TestIdentitySupport.getInternalId(record.get("n1").asNode()); + n2Id = TestIdentitySupport.getInternalId(record.get("n2").asNode()); + n3Id = TestIdentitySupport.getInternalId(record.get("n3").asNode()); StepVerifier.create(repository.findById(n1Id)).expectNextCount(1).verifyComplete(); StepVerifier.create(repository.findById(n2Id)).verifyComplete(); @@ -2521,9 +2519,9 @@ void deleteNodeWithMultipleLabels(@Autowired ReactiveMultipleLabelRepository rep long n3Id; Record record = doWithSession(session -> session.run("CREATE (n1:A:B:C), (n2:B:C), (n3:A) return n1, n2, n3").single()); - n1Id = IdentitySupport.getInternalId(record.get("n1").asNode()); - n2Id = IdentitySupport.getInternalId(record.get("n2").asNode()); - n3Id = IdentitySupport.getInternalId(record.get("n3").asNode()); + n1Id = TestIdentitySupport.getInternalId(record.get("n1").asNode()); + n2Id = TestIdentitySupport.getInternalId(record.get("n2").asNode()); + n3Id = TestIdentitySupport.getInternalId(record.get("n3").asNode()); repository.deleteById(n1Id).block(); repository.deleteById(n2Id).block(); diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/common/AltLikedByPersonRelationship.java b/src/test/java/org/springframework/data/neo4j/integration/shared/common/AltLikedByPersonRelationship.java index aab27e4f0..891179368 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/shared/common/AltLikedByPersonRelationship.java +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/common/AltLikedByPersonRelationship.java @@ -67,4 +67,12 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(rating, altPerson); } + + @Override + public String toString() { + return "AltLikedByPersonRelationship{" + + "rating=" + rating + + ", altPerson=" + altPerson.getName() + + '}'; + } } diff --git a/src/main/java/org/springframework/data/neo4j/core/schema/ElementId.java b/src/test/java/org/springframework/data/neo4j/test/TestIdentitySupport.java similarity index 64% rename from src/main/java/org/springframework/data/neo4j/core/schema/ElementId.java rename to src/test/java/org/springframework/data/neo4j/test/TestIdentitySupport.java index 6320a7a02..d631e6680 100644 --- a/src/main/java/org/springframework/data/neo4j/core/schema/ElementId.java +++ b/src/test/java/org/springframework/data/neo4j/test/TestIdentitySupport.java @@ -13,33 +13,24 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.neo4j.core.schema; +package org.springframework.data.neo4j.test; import org.neo4j.driver.types.Entity; /** - * A Neo4j element id. - * * @author Michael J. Simons - * @soundtrack GötterDÄmmerung - Tribut an die beste Band der Welt - * @since 7.0.0 */ -public interface ElementId { - - static ElementId of(String value) { - return new DefaultElementId(value); - } +public final class TestIdentitySupport { /** * @param entity The entity container as received from the server. - * @return The element id + * @return The internal id */ - static ElementId of(Entity entity) { - return of(entity.elementId()); + @SuppressWarnings("deprecation") + public static Long getInternalId(Entity entity) { + return entity.id(); } - /** - * @return A unique textual representation. - */ - String value(); + private TestIdentitySupport() { + } }