From 36924f0d5985d2581de40356d11cfdee8fcfa4c5 Mon Sep 17 00:00:00 2001 From: Luanne Misquitta Date: Thu, 9 Jun 2016 10:20:20 +0530 Subject: [PATCH] DATAGRAPH-876 - Derived finder filter values are not thread safe. --- .../query/derived/CypherFilter.java | 79 +++++++++++++++++++ .../query/derived/CypherFinderQuery.java | 16 ++-- .../neo4j/examples/movies/domain/Person.java | 9 +++ .../examples/movies/repo/UserRepository.java | 2 + .../data/neo4j/queries/DerivedQueryTest.java | 60 ++++++++++++++ 5 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/derived/CypherFilter.java diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/derived/CypherFilter.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/derived/CypherFilter.java new file mode 100644 index 000000000..7758adb11 --- /dev/null +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/derived/CypherFilter.java @@ -0,0 +1,79 @@ +/* + * Copyright (c) [2011-2016] "Pivotal Software, Inc." / "Neo Technology" / "Graph Aware Ltd." + * + * This product is licensed to you under the Apache License, Version 2.0 (the "License"). + * You may not use this product except in compliance with the License. + * + * This product may include a number of subcomponents with + * separate copyright notices and license terms. Your use of the source + * code for these subcomponents is subject to the terms and + * conditions of the subcomponent's license, as noted in the LICENSE file. + * + */ + +package org.springframework.data.neo4j.repository.query.derived; + +import org.neo4j.ogm.cypher.BooleanOperator; +import org.neo4j.ogm.cypher.ComparisonOperator; +import org.neo4j.ogm.cypher.Filter; + +/** + * A representation of a Neo4j-OGM Filter that contains no parameter/property values and only holds metadata + * @author Luanne Misquitta + */ +public class CypherFilter { + + Integer propertyPosition; + String propertyName; + Class ownerEntityType; + ComparisonOperator comparisonOperator; + boolean negated; + BooleanOperator booleanOperator; + Class nestedPropertyType; + String nestedPropertyName; + + public void setPropertyPosition(Integer propertyPosition) { + this.propertyPosition = propertyPosition; + } + + public void setPropertyName(String propertyName) { + this.propertyName = propertyName; + } + + public void setOwnerEntityType(Class ownerEntityType) { + this.ownerEntityType = ownerEntityType; + } + + public void setComparisonOperator(ComparisonOperator comparisonOperator) { + this.comparisonOperator = comparisonOperator; + } + + public void setNegated(boolean negated) { + this.negated = negated; + } + + public void setBooleanOperator(BooleanOperator booleanOperator) { + this.booleanOperator = booleanOperator; + } + + public void setNestedPropertyType(Class nestedPropertyType) { + this.nestedPropertyType = nestedPropertyType; + } + + public void setNestedPropertyName(String nestedPropertyName) { + this.nestedPropertyName = nestedPropertyName; + } + + Filter toFilter() { + Filter filter = new Filter(); + filter.setPropertyPosition(propertyPosition); + filter.setPropertyName(propertyName); + filter.setOwnerEntityType(ownerEntityType); + filter.setComparisonOperator(comparisonOperator); + filter.setNegated(negated); + filter.setBooleanOperator(booleanOperator); + filter.setNestedPropertyType(nestedPropertyType); + filter.setNestedPropertyName(nestedPropertyName); + return filter; + } +} diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/derived/CypherFinderQuery.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/derived/CypherFinderQuery.java index 73cc276fb..2f5b2d9d8 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/derived/CypherFinderQuery.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/derived/CypherFinderQuery.java @@ -12,9 +12,11 @@ */ package org.springframework.data.neo4j.repository.query.derived; +import java.util.ArrayList; +import java.util.List; + import org.neo4j.ogm.cypher.BooleanOperator; import org.neo4j.ogm.cypher.ComparisonOperator; -import org.neo4j.ogm.cypher.Filter; import org.neo4j.ogm.cypher.Filters; import org.springframework.data.repository.query.parser.Part; @@ -27,7 +29,7 @@ public class CypherFinderQuery implements DerivedQueryDefinition { private Class entityType; private Part basePart; - private Filters parameters = new Filters(); + private List cypherFilters = new ArrayList<>(); private int paramPosition = 0; public CypherFinderQuery(Class entityType, Part basePart) { @@ -42,13 +44,17 @@ public class CypherFinderQuery implements DerivedQueryDefinition { @Override public Filters getFilters() { - return parameters; + Filters filters = new Filters(); + for (CypherFilter cypherFilter : cypherFilters) { + filters.add(cypherFilter.toFilter()); + } + return filters; } @Override public void addPart(Part part, BooleanOperator booleanOperator) { String property = part.getProperty().getSegment(); - Filter parameter = new Filter(); + CypherFilter parameter = new CypherFilter(); parameter.setPropertyPosition(paramPosition++); parameter.setPropertyName(property); parameter.setOwnerEntityType(entityType); @@ -62,7 +68,7 @@ public class CypherFinderQuery implements DerivedQueryDefinition { parameter.setPropertyName(part.getProperty().getLeafProperty().getSegment()); parameter.setNestedPropertyName(part.getProperty().getSegment()); } - parameters.add(parameter); + cypherFilters.add(parameter); } diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/domain/Person.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/domain/Person.java index e17d39cfc..2b99c392c 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/domain/Person.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/domain/Person.java @@ -20,6 +20,7 @@ public class Person { private Long id; private String name; + private String surname; public String getName() { @@ -30,6 +31,14 @@ public class Person { this.name = name; } + public String getSurname() { + return surname; + } + + public void setSurname(String surname) { + this.surname = surname; + } + public Long getId() { return id; } diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/repo/UserRepository.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/repo/UserRepository.java index 9352248b9..de40f0549 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/repo/UserRepository.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/examples/movies/repo/UserRepository.java @@ -110,4 +110,6 @@ public interface UserRepository extends PersonRepository { @Query("match (u:User)-[r:RATED]->(m:Movie) return u as user, collect({username: u.name, movietitle: m.title, stars:r.stars}) as literalMap") List findRatingsWithLiteralMap(); + User findBySurname(String surname); + } diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/queries/DerivedQueryTest.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/queries/DerivedQueryTest.java index aa9dc3112..16a47318a 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/queries/DerivedQueryTest.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/queries/DerivedQueryTest.java @@ -39,6 +39,9 @@ import java.io.IOException; import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import static org.junit.Assert.*; @@ -469,4 +472,61 @@ public class DerivedQueryTest extends MultiDriverTestClass { assertEquals(2, row.getLiteralMap().size()); } } + + + /** + * @see DATAGRAPH-876 + * @throws InterruptedException + */ + @Test + public void shouldAllowMultiThreadedDerivedFinderExecution() throws InterruptedException { + int numThreads = 3; + executeUpdate("CREATE (m:User {name:'Michal', surname:'Bachman'}), (a:User {name:'Adam', surname:'George'}), (l:User {name:'Luanne', surname:'Misquitta'})"); + + session.clear(); + + String[] firstNames = new String[] {"Michal", "Adam", "Luanne"}; + String[] lastNames = new String[] {"Bachman", "George", "Misquitta"}; + + + ExecutorService executor = Executors.newFixedThreadPool(numThreads); + CountDownLatch latch = new CountDownLatch(numThreads); + + for (int i = 0; i < numThreads; i++) { + executor.submit(new DerivedQueryRunner(latch, firstNames[i], lastNames[i])); + } + latch.await(); // pause until the count reaches 0 + + // force termination of all threads + executor.shutdownNow(); + + + } + + + class DerivedQueryRunner implements Runnable { + + private final CountDownLatch latch; + private final String firstName; + private final String lastName; + + public DerivedQueryRunner(CountDownLatch latch, String firstName, String lastName ) { + this.latch = latch; + this.firstName = firstName; + this.lastName = lastName; + } + + @Override + public void run() { + try + { + User user = userRepository.findBySurname(lastName); + assertNotNull(user); + assertEquals(firstName, user.getName()); + assertEquals(lastName, user.getSurname()); + } finally { + latch.countDown(); + } + } + } }