From 793b2345bd18b511ce2de283bcdf21fb20696f9b Mon Sep 17 00:00:00 2001 From: dhallam Date: Tue, 29 Apr 2014 07:26:55 +0100 Subject: [PATCH] DATAGRAPH-466 - Derived finders issue. Following feedback from @jexp. Changed TreeSet to LinkedHashSet to maintain query insertion order instead of how I'd originally implemented it where the order would be the same, irrespective of insertion order. --- .../neo4j/repository/query/CypherQuery.java | 20 +++-- .../neo4j/repository/DerivedFinderTests.java | 75 ++++++++++++++----- 2 files changed, 72 insertions(+), 23 deletions(-) diff --git a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/CypherQuery.java b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/CypherQuery.java index 78cb022ae..bcca2692b 100644 --- a/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/CypherQuery.java +++ b/spring-data-neo4j/src/main/java/org/springframework/data/neo4j/repository/query/CypherQuery.java @@ -15,6 +15,18 @@ */ package org.springframework.data.neo4j.repository.query; +import static org.springframework.util.StringUtils.collectionToCommaDelimitedString; +import static org.springframework.util.StringUtils.collectionToDelimitedString; +import static org.springframework.util.StringUtils.hasText; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.ListIterator; +import java.util.Map; +import java.util.Set; + import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.mapping.context.PersistentPropertyPath; @@ -24,11 +36,6 @@ import org.springframework.data.neo4j.support.Neo4jTemplate; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.parser.Part; -import java.util.*; - -import static org.springframework.util.StringUtils.*; -import static org.springframework.util.StringUtils.hasText; - public class CypherQuery implements CypherQueryDefinition { private final VariableContext variableContext = new VariableContext(); private final List matchClauses = new ArrayList(); @@ -280,7 +287,8 @@ public class CypherQuery implements CypherQueryDefinition { } private String toQueryString(List matchClauses) { - List result = new ArrayList(matchClauses.size()); + // Use a TreeSet to remove duplicate match clauses and maintain their order + Set result = new LinkedHashSet(); for (MatchClause matchClause : matchClauses) { result.add(matchClause.toString(variableContext)); } diff --git a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/DerivedFinderTests.java b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/DerivedFinderTests.java index bfc8fdb53..fc2d37561 100644 --- a/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/DerivedFinderTests.java +++ b/spring-data-neo4j/src/test/java/org/springframework/data/neo4j/repository/DerivedFinderTests.java @@ -15,41 +15,44 @@ */ package org.springframework.data.neo4j.repository; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.neo4j.graphdb.Direction.INCOMING; +import static org.neo4j.graphdb.Direction.OUTGOING; +import static org.neo4j.helpers.collection.IteratorUtil.asCollection; +import static org.neo4j.helpers.collection.IteratorUtil.single; +import static org.springframework.data.neo4j.SetHelper.asSet; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; + import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.Transaction; -import org.neo4j.helpers.collection.IteratorUtil; import org.neo4j.test.TestGraphDatabaseFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.data.domain.Sort; -import org.springframework.data.neo4j.annotation.*; +import org.springframework.data.neo4j.annotation.Fetch; +import org.springframework.data.neo4j.annotation.GraphId; +import org.springframework.data.neo4j.annotation.Indexed; +import org.springframework.data.neo4j.annotation.NodeEntity; +import org.springframework.data.neo4j.annotation.RelatedTo; import org.springframework.data.neo4j.config.EnableNeo4jRepositories; import org.springframework.data.neo4j.config.Neo4jConfiguration; import org.springframework.data.neo4j.support.Neo4jTemplate; -import org.springframework.data.neo4j.support.index.IndexType; import org.springframework.data.neo4j.support.node.Neo4jHelper; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; -import org.springframework.transaction.annotation.Transactional; - -import java.util.*; - -import static java.util.Arrays.asList; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; -import static org.neo4j.graphdb.Direction.INCOMING; -import static org.neo4j.helpers.collection.IteratorUtil.asCollection; -import static org.neo4j.helpers.collection.IteratorUtil.single; -import static org.springframework.data.neo4j.SetHelper.asSet; @NodeEntity class Ingredient { @@ -93,6 +96,9 @@ class CookBook { CookBook(String title) { this.title = title; } + + @RelatedTo(type="HAS_RECIPE", direction = OUTGOING) + Set recipes; } @NodeEntity @@ -105,6 +111,7 @@ class Recipe { @Fetch Ingredient ingredient; + @RelatedTo(type="HAS_RECIPE", direction = INCOMING) CookBook cookBook; @RelatedTo(direction = INCOMING) @@ -130,6 +137,13 @@ interface DishRepository extends GraphRepository { Dish findByNumber(int number); } +interface CookBookRepository extends GraphRepository { + CookBook findByTitle(String title); + Set findByRecipesAuthor(String recipeAuthor); + Set findByRecipesTitle(String recipeTitle); + Set findByRecipesAuthorAndRecipesTitle(String recipeAuthor, String recipeTitle); +} + interface RecipeRepository extends GraphRepository { Set findById(long id); @@ -198,6 +212,9 @@ public class DerivedFinderTests { @Autowired private DishRepository dishRepository; + + @Autowired + private CookBookRepository cookBookRepository; private Ingredient fish, spice, oliveOil, pear, chocolate; @@ -399,4 +416,28 @@ public class DerivedFinderTests { assertEquals("Nigella" , recs.get(0).author); assertEquals("Heston" , recs.get(1).author); } + + @Test + public void shouldFindByMultipleRelatedProperties() { + Set jamieR = recipeRepository.findByAuthor("Jamie"); + assertThat(jamieR.size(), equalTo(1)); + + CookBook book = cookBookRepository.findByTitle("Naked Chef"); + assertThat(book, notNullValue()); + + // Ensure we can find the book via the recipe author + Set books = cookBookRepository.findByRecipesAuthor("Jamie"); + assertThat(books.size(), equalTo(1)); + assertThat(books.iterator().next().title, equalTo("Naked Chef")); + + // Ensure we can find the book via the recipe title + books = cookBookRepository.findByRecipesTitle("pesto"); + assertThat(books.size(), equalTo(1)); + assertThat(books.iterator().next().title, equalTo("Naked Chef")); + + // Now try to find the book via both the recipe author and the recipe title + books = cookBookRepository.findByRecipesAuthorAndRecipesTitle("Jamie", "pesto"); + assertThat(books.size(), equalTo(1)); + assertThat(books.iterator().next().title, equalTo("Naked Chef")); + } }