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() { + } }