From 23ff75ee6e1014428d15a9037de70338c6b492b5 Mon Sep 17 00:00:00 2001 From: Thomas Risberg Date: Mon, 28 Mar 2011 13:54:44 -0400 Subject: [PATCH 1/7] needed access to entity when use with partial JPA/cross-store persistence --- .../data/persistence/ChangeSetPersister.java | 96 ++++++++++--------- ...geSetBackedTransactionSynchronization.java | 2 +- 2 files changed, 50 insertions(+), 48 deletions(-) diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/persistence/ChangeSetPersister.java b/spring-data-commons-core/src/main/java/org/springframework/data/persistence/ChangeSetPersister.java index 0aaa02841..742c2e846 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/persistence/ChangeSetPersister.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/persistence/ChangeSetPersister.java @@ -1,47 +1,49 @@ -package org.springframework.data.persistence; - -import org.springframework.dao.DataAccessException; - -/** - * Interface to be implemented by classes that can synchronize - * between data stores and ChangeSets. - * @author Rod Johnson - * - * @param entity key - */ -public interface ChangeSetPersister { - - String ID_KEY = "_id"; - - String CLASS_KEY = "_class"; - - /** - * TODO how to tell when not found? throw exception? - */ - void getPersistentState(Class entityClass, K key, ChangeSet changeSet) throws DataAccessException, NotFoundException; - - /** - * Return id - * @param cs - * @return - * @throws DataAccessException - */ - K getPersistentId(Class entityClass, ChangeSet cs) throws DataAccessException; - - /** - * Return key - * @param cs Key may be null if not persistent - * @return - * @throws DataAccessException - */ - K persistState(Class entityClass, ChangeSet cs) throws DataAccessException; - - /** - * Exception thrown in alternate control flow if getPersistentState - * finds no entity data. - */ - class NotFoundException extends Exception { - - } - -} +package org.springframework.data.persistence; + +import org.springframework.dao.DataAccessException; + +/** + * Interface to be implemented by classes that can synchronize + * between data stores and ChangeSets. + * @author Rod Johnson + * + * @param entity key + */ +public interface ChangeSetPersister { + + String ID_KEY = "_id"; + + String CLASS_KEY = "_class"; + + /** + * TODO how to tell when not found? throw exception? + */ + void getPersistentState(Class entityClass, K key, ChangeSet changeSet) throws DataAccessException, NotFoundException; + + /** + * Return id + * @param entity + * @param cs + * @return + * @throws DataAccessException + */ + K getPersistentId(ChangeSetBacked entity, ChangeSet cs) throws DataAccessException; + + /** + * Return key + * @param entity + * @param cs Key may be null if not persistent + * @return + * @throws DataAccessException + */ + K persistState(ChangeSetBacked entity, ChangeSet cs) throws DataAccessException; + + /** + * Exception thrown in alternate control flow if getPersistentState + * finds no entity data. + */ + class NotFoundException extends Exception { + + } + +} diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/transaction/ChangeSetBackedTransactionSynchronization.java b/spring-data-commons-core/src/main/java/org/springframework/data/transaction/ChangeSetBackedTransactionSynchronization.java index 69c2e595f..5ea9353d6 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/transaction/ChangeSetBackedTransactionSynchronization.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/transaction/ChangeSetBackedTransactionSynchronization.java @@ -23,7 +23,7 @@ public class ChangeSetBackedTransactionSynchronization implements TransactionSyn public void afterCommit() { log.debug("After Commit called for " + entity); - changeSetPersister.persistState(entity.getClass(), entity.getChangeSet()); + changeSetPersister.persistState(entity, entity.getChangeSet()); changeSetTxStatus = 0; } From c05bedce178fd03a023694b1b9040644fe666acc Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 28 Mar 2011 20:31:55 +0200 Subject: [PATCH 2/7] Fixed bug in Property. --- .../data/repository/query/parser/Property.java | 4 ++-- .../data/repository/query/parser/PropertyUnitTests.java | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/repository/query/parser/Property.java b/spring-data-commons-core/src/main/java/org/springframework/data/repository/query/parser/Property.java index df71ad597..afe1fafae 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/repository/query/parser/Property.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/repository/query/parser/Property.java @@ -285,7 +285,7 @@ public class Property { IllegalArgumentException exception = null; try { - return new Property(source, type); + return new Property(source, type, addTail); } catch (IllegalArgumentException e) { exception = e; } @@ -299,7 +299,7 @@ public class Property { String head = source.substring(0, position); String tail = source.substring(position); - return new Property(head, type, tail + addTail); + return create(head, type, tail + addTail); } throw exception; diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/query/parser/PropertyUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/query/parser/PropertyUnitTests.java index f1810ba52..f5426528a 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/repository/query/parser/PropertyUnitTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/query/parser/PropertyUnitTests.java @@ -123,6 +123,12 @@ public class PropertyUnitTests { Property.from("userMapMame", Bar.class); } + + @Test + public void findsNested() { + + Property from = Property.from("barUserName", Sample.class); + } private class Foo { @@ -146,6 +152,7 @@ public class PropertyUnitTests { private String userName; private FooBar user; + private Bar bar; } private class Sample2 { From 95ff87e959c4379c28254df1b1d9ad7366bb8524 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 29 Mar 2011 08:48:58 +0200 Subject: [PATCH 3/7] Configured Maven JAR plugin to package the Bundlor generated MANIFEST.MF. --- spring-data-commons-parent/pom.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spring-data-commons-parent/pom.xml b/spring-data-commons-parent/pom.xml index 4e243f88f..9aa9c19a0 100644 --- a/spring-data-commons-parent/pom.xml +++ b/spring-data-commons-parent/pom.xml @@ -286,6 +286,14 @@ false + + org.apache.maven.plugins + maven-jar-plugin + 2.3.1 + + true + + org.apache.maven.plugins maven-surefire-plugin From 6bb0923c141d45157e595b5719e0bd60be6b3848 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 29 Mar 2011 19:19:23 +0200 Subject: [PATCH 4/7] DATACMNS-26 - Persistent constructor now detects non-public constructor. --- .../org/springframework/data/mapping/BasicMappingContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/BasicMappingContext.java b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/BasicMappingContext.java index b7bca0421..13e3a5d01 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/BasicMappingContext.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/BasicMappingContext.java @@ -295,7 +295,7 @@ public class BasicMappingContext implements MappingContext, InitializingBean, Ap // Find the right constructor PreferredConstructor preferredConstructor = null; - for (Constructor constructor : type.getConstructors()) { + for (Constructor constructor : type.getDeclaredConstructors()) { if (constructor.getParameterTypes().length != 0) { // Non-no-arg constructor if (null == preferredConstructor || constructor.isAnnotationPresent(PersistenceConstructor.class)) { From 9931e885c0a336e367c313d5d3466b945eb7211a Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 31 Mar 2011 18:24:03 +0200 Subject: [PATCH 5/7] Expose type of a query parser Property. --- .../data/repository/query/parser/Property.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/repository/query/parser/Property.java b/spring-data-commons-core/src/main/java/org/springframework/data/repository/query/parser/Property.java index afe1fafae..c845bc54e 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/repository/query/parser/Property.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/repository/query/parser/Property.java @@ -115,6 +115,18 @@ public class Property { return name; } + /** + * Returns the type of the property will return the plain resolved type for + * simple properties, the component type for any {@link Iterable} or the + * value type of a {@link java.util.Map} if the property is one. + * + * @return + */ + public Class getType() { + + return this.type.getType(); + } + /** * Returns the next nested {@link Property}. From c4d81a28f1f4cd259bb587c98c0a8246c1341d4a Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 31 Mar 2011 21:46:33 +0200 Subject: [PATCH 6/7] =?UTF-8?q?DATACMNS-25=20-=20Refactored=20RepositoryFa?= =?UTF-8?q?ctorySupport.getQueryLookupStrategy(=E2=80=A6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Method is not abstract anymore but rather returns null by default. Adapted QueryExecutionMethodInterceptor to handle the null case appropriately by simply skipping the query lookup. It also throws an exception if the repository has query methods defined nonetheless to prevent exceptions at runtime. --- .../support/RepositoryFactorySupport.java | 29 ++++++--- ...eryExecuterMethodInterceptorUnitTests.java | 59 +++++++++++++++++++ 2 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 spring-data-commons-core/src/test/java/org/springframework/data/repository/support/QueryExecuterMethodInterceptorUnitTests.java diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryFactorySupport.java b/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryFactorySupport.java index c6b8477be..5aefd18ae 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryFactorySupport.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/repository/support/RepositoryFactorySupport.java @@ -190,14 +190,15 @@ public abstract class RepositoryFactorySupport { protected abstract Class getRepositoryBaseClass( Class repositoryInterface); - - /** - * Returns the {@link QueryLookupStrategy} for the given {@link Key}. - * - * @param key can be {@literal null} - * @return - */ - protected abstract QueryLookupStrategy getQueryLookupStrategy(Key key); + /** + * Returns the {@link QueryLookupStrategy} for the given {@link Key}. + * + * @param key can be {@literal null} + * @return the {@link QueryLookupStrategy} to use or {@literal null} if no queries should be looked up. + */ + protected QueryLookupStrategy getQueryLookupStrategy(Key key) { + return null; + } /** @@ -254,6 +255,18 @@ public abstract class RepositoryFactorySupport { QueryLookupStrategy lookupStrategy = getQueryLookupStrategy(queryLookupStrategyKey); + + if (lookupStrategy == null) { + + if (repositoryMetadata.hasCustomMethod()) { + throw new IllegalStateException( + "You have defined query method in the repository but " + + "you don't have no query lookup strategy defined. The " + + "infrastructure apparently does not support query methods!"); + } + + return; + } for (Method method : metadata.getQueryMethods()) { RepositoryQuery query = diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/QueryExecuterMethodInterceptorUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/QueryExecuterMethodInterceptorUnitTests.java new file mode 100644 index 000000000..a9e41da5d --- /dev/null +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/support/QueryExecuterMethodInterceptorUnitTests.java @@ -0,0 +1,59 @@ +/* + * Copyright 2011 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 + * + * http://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.repository.support; + +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.data.repository.query.QueryLookupStrategy.Key; +import org.springframework.data.repository.support.RepositoryFactorySupport.QueryExecuterMethodInterceptor; + +/** + * Unit test for {@link QueryExecuterMethodInterceptor}. + * + * @author Oliver Gierke + */ +@RunWith(MockitoJUnitRunner.class) +public class QueryExecuterMethodInterceptorUnitTests { + + @Mock + RepositoryFactorySupport factory; + @Mock + RepositoryMetadata metadata; + + @Test(expected=IllegalStateException.class) + public void rejectsRepositoryInterfaceWithQueryMethodsIfNoQueryLookupStrategyIsDefined() { + + when(metadata.hasCustomMethod()).thenReturn(true); + when(factory.getQueryLookupStrategy(any(Key.class))).thenReturn(null); + + factory.new QueryExecuterMethodInterceptor(metadata, null, new Object()); + } + + @Test + public void skipsQueryLookupsIfQueryLookupStrategyIsNull() { + + when(metadata.hasCustomMethod()).thenReturn(false); + when(factory.getQueryLookupStrategy(any(Key.class))).thenReturn(null); + + factory.new QueryExecuterMethodInterceptor(metadata, null, new Object()); + verify(metadata, times(0)).getQueryMethods(); + } +} From 4b51c5fd1f44f2849022f06d95602ce671f22e4d Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 1 Apr 2011 08:34:39 +0200 Subject: [PATCH 7/7] DATACMNS-23 - Made PageImpl safe to take an empty collection. Added hasContent() method to Page interface. --- .../org/springframework/data/domain/Page.java | 8 ++++++ .../springframework/data/domain/PageImpl.java | 28 ++++++++++++------- .../data/domain/PageImplUnitTests.java | 21 ++++++++++++++ 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/domain/Page.java b/spring-data-commons-core/src/main/java/org/springframework/data/domain/Page.java index fe5c1b4ff..aec3ac952 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/domain/Page.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/domain/Page.java @@ -115,6 +115,14 @@ public interface Page extends Iterable { * @return */ List getContent(); + + + /** + * Returns whether the {@link Page} has content at all. + * + * @return + */ + boolean hasContent(); /** diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/domain/PageImpl.java b/spring-data-commons-core/src/main/java/org/springframework/data/domain/PageImpl.java index f46de7d84..bd0ebb71e 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/domain/PageImpl.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/domain/PageImpl.java @@ -49,10 +49,7 @@ public class PageImpl implements Page { this.content.addAll(content); this.total = total; - - this.pageable = - null == pageable ? new PageRequest(0, content.size()) - : pageable; + this.pageable = pageable; } @@ -75,7 +72,7 @@ public class PageImpl implements Page { */ public int getNumber() { - return pageable.getPageNumber(); + return pageable == null ? 0 : pageable.getPageNumber(); } @@ -86,7 +83,7 @@ public class PageImpl implements Page { */ public int getSize() { - return pageable.getPageSize(); + return pageable == null ? 0 : pageable.getPageSize(); } @@ -97,7 +94,7 @@ public class PageImpl implements Page { */ public int getTotalPages() { - return (int) Math.ceil((double) total / (double) getSize()); + return getSize() == 0 ? 0 : (int) Math.ceil((double) total / (double) getSize()); } @@ -187,6 +184,17 @@ public class PageImpl implements Page { return Collections.unmodifiableList(content); } + + /* + * (non-Javadoc) + * + * @see org.springframework.data.domain.Page#hasContent() + */ + @Override + public boolean hasContent() { + + return !content.isEmpty(); + } /* @@ -196,7 +204,7 @@ public class PageImpl implements Page { */ public Sort getSort() { - return pageable.getSort(); + return pageable == null ? null : pageable.getSort(); } @@ -239,7 +247,7 @@ public class PageImpl implements Page { boolean totalEqual = this.total == that.total; boolean contentEqual = this.content.equals(that.content); - boolean pageableEqual = this.pageable.equals(that.pageable); + boolean pageableEqual = this.pageable == null ? that.pageable == null : this.pageable.equals(that.pageable); return totalEqual && contentEqual && pageableEqual; } @@ -256,7 +264,7 @@ public class PageImpl implements Page { int result = 17; result = 31 * result + (int) (total ^ total >>> 32); - result = 31 * result + pageable.hashCode(); + result = 31 * result + (pageable == null ? 0 : pageable.hashCode()); result = 31 * result + content.hashCode(); return result; diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/domain/PageImplUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/domain/PageImplUnitTests.java index ce51a8bf8..bf22c90c5 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/domain/PageImplUnitTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/domain/PageImplUnitTests.java @@ -16,9 +16,12 @@ package org.springframework.data.domain; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; import static org.springframework.data.domain.UnitTestUtils.*; import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.junit.Test; @@ -78,4 +81,22 @@ public class PageImplUnitTests { new PageImpl(null, null, 0); } + + @Test + public void createsPageForEmptyContentCorrectly() { + List list = Collections.emptyList(); + Page page = new PageImpl(list); + assertThat(page.getContent(), is(list)); + assertThat(page.getNumber(), is(0)); + assertThat(page.getNumberOfElements(), is(0)); + assertThat(page.getSize(), is(0)); + assertThat(page.getSort(), is((Sort) null)); + assertThat(page.getTotalElements(), is(0L)); + assertThat(page.getTotalPages(), is(0)); + assertThat(page.hasNextPage(), is(false)); + assertThat(page.hasPreviousPage(), is(false)); + assertThat(page.isFirstPage(), is(true)); + assertThat(page.isLastPage(), is(true)); + assertThat(page.hasContent(), is(false)); + } }