From e3733ab9b5b0d80f6f6293bb766167979f69f6b4 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 19 May 2015 17:34:11 +0200 Subject: [PATCH] DATAKV-105 - Polishing. Moved KeySpaceResolver interfaces into mapping package. Let defaulting happen in BasicKeyValuePersistentEntity so that ClassNameKeySpaceResolver can be made package protected. FowardingCloseableIterator is now generically typed and not bound to Map.Entry. Original pull request: #11. --- .../core/ForwardingCloseableIterator.java | 25 +++++++----- .../data/keyvalue/core/KeyValueTemplate.java | 4 +- .../AnnotationBasedKeySpaceResolver.java | 5 +-- .../BasicKeyValuePersistentEntity.java | 20 ++++++---- .../ClassNameKeySpaceResolver.java | 6 +-- .../core/{ => mapping}/KeySpaceResolver.java | 2 +- .../context/KeyValueMappingContext.java | 23 +++-------- .../data/map/MapKeyValueAdapter.java | 8 ++-- .../ForwardingCloseableIteratorUnitTests.java | 38 ++++++++++++++----- .../KeyValueMappingContextUnitTests.java | 31 --------------- 10 files changed, 75 insertions(+), 87 deletions(-) rename src/main/java/org/springframework/data/keyvalue/core/{ => mapping}/ClassNameKeySpaceResolver.java (88%) rename src/main/java/org/springframework/data/keyvalue/core/{ => mapping}/KeySpaceResolver.java (95%) delete mode 100644 src/test/java/ort/springframework/data/keyvalue/core/mapping/context/KeyValueMappingContextUnitTests.java diff --git a/src/main/java/org/springframework/data/keyvalue/core/ForwardingCloseableIterator.java b/src/main/java/org/springframework/data/keyvalue/core/ForwardingCloseableIterator.java index efd8393..0ecc15d 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/ForwardingCloseableIterator.java +++ b/src/main/java/org/springframework/data/keyvalue/core/ForwardingCloseableIterator.java @@ -16,39 +16,44 @@ package org.springframework.data.keyvalue.core; import java.util.Iterator; -import java.util.Map; import org.springframework.data.util.CloseableIterator; +import org.springframework.util.Assert; /** * Forwards {@link CloseableIterator} invocations to the configured {@link Iterator} delegate. * * @author Christoph Strobl * @author Thomas Darimont + * @author Oliver Gierke * @param * @param */ -public class ForwardingCloseableIterator implements CloseableIterator> { +public class ForwardingCloseableIterator implements CloseableIterator { - private final Iterator> delegate; + private final Iterator delegate; private final Runnable closeHandler; /** * Creates a new {@link ForwardingCloseableIterator}. * - * @param delegate must not be {@literal null} + * @param delegate must not be {@literal null}. */ - public ForwardingCloseableIterator(Iterator> delegate) { + public ForwardingCloseableIterator(Iterator delegate) { this(delegate, null); } /** - * Creates a new {@link ForwardingCloseableIterator} that invokes the configured {@code closeHanlder} on {@link #close()}. + * Creates a new {@link ForwardingCloseableIterator} that invokes the configured {@code closeHandler} on + * {@link #close()}. * - * @param delegate must not be {@literal null} - * @param closeHandler may be {@literal null} + * @param delegate must not be {@literal null}. + * @param closeHandler may be {@literal null}. */ - public ForwardingCloseableIterator(Iterator> delegate, Runnable closeHandler) { + public ForwardingCloseableIterator(Iterator delegate, Runnable closeHandler) { + + Assert.notNull(delegate, "Delegate iterator must not be null!"); + this.delegate = delegate; this.closeHandler = closeHandler; } @@ -67,7 +72,7 @@ public class ForwardingCloseableIterator implements CloseableIterator next() { + public T next() { return delegate.next(); } diff --git a/src/main/java/org/springframework/data/keyvalue/core/KeyValueTemplate.java b/src/main/java/org/springframework/data/keyvalue/core/KeyValueTemplate.java index f56d44a..b7fe642 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/KeyValueTemplate.java +++ b/src/main/java/org/springframework/data/keyvalue/core/KeyValueTemplate.java @@ -55,12 +55,12 @@ public class KeyValueTemplate implements KeyValueOperations, ApplicationContextA private static final PersistenceExceptionTranslator DEFAULT_PERSISTENCE_EXCEPTION_TRANSLATOR = new KeyValuePersistenceExceptionTranslator(); private final KeyValueAdapter adapter; - private final MappingContext, ? extends KeyValuePersistentProperty> mappingContext; private final IdentifierGenerator identifierGenerator; - private ApplicationEventPublisher eventPublisher; private final Set eventTypesToPublish = new HashSet(4); + private PersistenceExceptionTranslator exceptionTranslator = DEFAULT_PERSISTENCE_EXCEPTION_TRANSLATOR; + private ApplicationEventPublisher eventPublisher; /** * Create new {@link KeyValueTemplate} using the given {@link KeyValueAdapter} with a default diff --git a/src/main/java/org/springframework/data/keyvalue/core/mapping/AnnotationBasedKeySpaceResolver.java b/src/main/java/org/springframework/data/keyvalue/core/mapping/AnnotationBasedKeySpaceResolver.java index 4a8004b..5103a27 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/mapping/AnnotationBasedKeySpaceResolver.java +++ b/src/main/java/org/springframework/data/keyvalue/core/mapping/AnnotationBasedKeySpaceResolver.java @@ -26,7 +26,6 @@ import org.springframework.core.annotation.AnnotationUtils; import org.springframework.core.style.ToStringCreator; import org.springframework.data.annotation.Persistent; import org.springframework.data.keyvalue.annotation.KeySpace; -import org.springframework.data.keyvalue.core.KeySpaceResolver; import org.springframework.data.keyvalue.core.mapping.AnnotationBasedKeySpaceResolver.MetaAnnotationUtils.AnnotationDescriptor; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -58,7 +57,7 @@ enum AnnotationBasedKeySpaceResolver implements KeySpaceResolver { return keySpace != null ? keySpace.toString() : null; } - private Object getKeySpace(Class type) { + private static Object getKeySpace(Class type) { KeySpace keyspace = AnnotationUtils.findAnnotation(type, KeySpace.class); @@ -81,6 +80,7 @@ enum AnnotationBasedKeySpaceResolver implements KeySpaceResolver { } } } + return null; } @@ -407,5 +407,4 @@ enum AnnotationBasedKeySpaceResolver implements KeySpaceResolver { } } } - } diff --git a/src/main/java/org/springframework/data/keyvalue/core/mapping/BasicKeyValuePersistentEntity.java b/src/main/java/org/springframework/data/keyvalue/core/mapping/BasicKeyValuePersistentEntity.java index ddeaf27..ec12840 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/mapping/BasicKeyValuePersistentEntity.java +++ b/src/main/java/org/springframework/data/keyvalue/core/mapping/BasicKeyValuePersistentEntity.java @@ -15,37 +15,43 @@ */ package org.springframework.data.keyvalue.core.mapping; -import org.springframework.data.keyvalue.core.KeySpaceResolver; import org.springframework.data.mapping.model.BasicPersistentEntity; import org.springframework.data.util.TypeInformation; -import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** * {@link KeyValuePersistentEntity} implementation that adds specific meta-data such as the {@literal keySpace}.. * * @author Christoph Strobl + * @author Oliver Gierke * @param */ public class BasicKeyValuePersistentEntity extends BasicPersistentEntity implements KeyValuePersistentEntity { + private static final KeySpaceResolver DEFAULT_FALLBACK_RESOLVER = ClassNameKeySpaceResolver.INSTANCE; + private final String keyspace; /** * @param information must not be {@literal null}. - * @param keySpaceResolver must not be {@literal null}. + * @param keySpaceResolver can be {@literal null}. */ public BasicKeyValuePersistentEntity(TypeInformation information, KeySpaceResolver fallbackKeySpaceResolver) { super(information); - Assert.notNull(fallbackKeySpaceResolver, "FallbackKeySpaceResolver must not be null!"); + this.keyspace = detectKeySpace(information.getType(), fallbackKeySpaceResolver); + } - String resolvedKeySpace = AnnotationBasedKeySpaceResolver.INSTANCE.resolveKeySpace(information.getType()); - this.keyspace = StringUtils.hasText(resolvedKeySpace) ? resolvedKeySpace : fallbackKeySpaceResolver - .resolveKeySpace(information.getType()); + private static String detectKeySpace(Class type, KeySpaceResolver fallback) { + String keySpace = AnnotationBasedKeySpaceResolver.INSTANCE.resolveKeySpace(type); + + if (StringUtils.hasText(keySpace)) + return keySpace; + + return (fallback == null ? DEFAULT_FALLBACK_RESOLVER : fallback).resolveKeySpace(type); } /* diff --git a/src/main/java/org/springframework/data/keyvalue/core/ClassNameKeySpaceResolver.java b/src/main/java/org/springframework/data/keyvalue/core/mapping/ClassNameKeySpaceResolver.java similarity index 88% rename from src/main/java/org/springframework/data/keyvalue/core/ClassNameKeySpaceResolver.java rename to src/main/java/org/springframework/data/keyvalue/core/mapping/ClassNameKeySpaceResolver.java index 631e555..1dc4664 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/ClassNameKeySpaceResolver.java +++ b/src/main/java/org/springframework/data/keyvalue/core/mapping/ClassNameKeySpaceResolver.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.keyvalue.core; +package org.springframework.data.keyvalue.core.mapping; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -22,8 +22,9 @@ import org.springframework.util.ClassUtils; * Most trivial implementation of {@link KeySpaceResolver} returning the {@link Class#getName()}. * * @author Christoph Strobl + * @author Oliver Gierke */ -public enum ClassNameKeySpaceResolver implements KeySpaceResolver { +enum ClassNameKeySpaceResolver implements KeySpaceResolver { INSTANCE; @@ -37,5 +38,4 @@ public enum ClassNameKeySpaceResolver implements KeySpaceResolver { Assert.notNull(type, "Type must not be null!"); return ClassUtils.getUserClass(type).getName(); } - } diff --git a/src/main/java/org/springframework/data/keyvalue/core/KeySpaceResolver.java b/src/main/java/org/springframework/data/keyvalue/core/mapping/KeySpaceResolver.java similarity index 95% rename from src/main/java/org/springframework/data/keyvalue/core/KeySpaceResolver.java rename to src/main/java/org/springframework/data/keyvalue/core/mapping/KeySpaceResolver.java index 0287071..dae56ec 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/KeySpaceResolver.java +++ b/src/main/java/org/springframework/data/keyvalue/core/mapping/KeySpaceResolver.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.keyvalue.core; +package org.springframework.data.keyvalue.core.mapping; /** * {@link KeySpaceResolver} determines the {@literal keyspace} a given type is assigned to. A keyspace in this context diff --git a/src/main/java/org/springframework/data/keyvalue/core/mapping/context/KeyValueMappingContext.java b/src/main/java/org/springframework/data/keyvalue/core/mapping/context/KeyValueMappingContext.java index 33cd91b..f544171 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/mapping/context/KeyValueMappingContext.java +++ b/src/main/java/org/springframework/data/keyvalue/core/mapping/context/KeyValueMappingContext.java @@ -18,44 +18,33 @@ package org.springframework.data.keyvalue.core.mapping.context; import java.beans.PropertyDescriptor; import java.lang.reflect.Field; -import org.springframework.data.keyvalue.core.ClassNameKeySpaceResolver; -import org.springframework.data.keyvalue.core.KeySpaceResolver; import org.springframework.data.keyvalue.core.mapping.BasicKeyValuePersistentEntity; +import org.springframework.data.keyvalue.core.mapping.KeySpaceResolver; import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentEntity; import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentProperty; import org.springframework.data.mapping.context.AbstractMappingContext; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.util.TypeInformation; -import org.springframework.util.Assert; /** * Default implementation of a {@link MappingContext} using {@link KeyValuePersistentEntity} and * {@link KeyValuePersistentProperty} as primary abstractions. * * @author Christoph Strobl + * @author Oliver Gierke */ public class KeyValueMappingContext extends AbstractMappingContext, KeyValuePersistentProperty> { - private final KeySpaceResolver fallbackKeySpaceResolver; + private KeySpaceResolver fallbackKeySpaceResolver; /** - * Creates new {@link KeyValueMappingContext} using an {@link ClassNameKeySpaceResolver} for {@literal keyspace} - * resolution. - */ - public KeyValueMappingContext() { - this(ClassNameKeySpaceResolver.INSTANCE); - } - - /** - * Creates new {@link KeyValueMappingContext} using given {@link KeySpaceResolver} + * Configures the {@link KeySpaceResolver} to be used if not explicit key space is annotated to the domain type. * - * @param keySpaceResolver must not be {@literal null}. + * @param fallbackKeySpaceResolver can be {@literal null}. */ - public KeyValueMappingContext(KeySpaceResolver fallbackKeySpaceResolver) { - - Assert.notNull(fallbackKeySpaceResolver, "FallbackKeySpaceResolver must not be null!"); + public void setFallbackKeySpaceResolver(KeySpaceResolver fallbackKeySpaceResolver) { this.fallbackKeySpaceResolver = fallbackKeySpaceResolver; } diff --git a/src/main/java/org/springframework/data/map/MapKeyValueAdapter.java b/src/main/java/org/springframework/data/map/MapKeyValueAdapter.java index bf18e0a..f5bb75a 100644 --- a/src/main/java/org/springframework/data/map/MapKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/map/MapKeyValueAdapter.java @@ -18,6 +18,7 @@ package org.springframework.data.map; import java.io.Serializable; import java.util.Collection; import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; import org.springframework.core.CollectionFactory; @@ -148,8 +149,8 @@ public class MapKeyValueAdapter extends AbstractKeyValueAdapter { * @see org.springframework.data.keyvalue.core.KeyValueAdapter#entries(java.io.Serializable) */ @Override - public CloseableIterator> entries(Serializable keyspace) { - return new ForwardingCloseableIterator(getKeySpaceMap(keyspace).entrySet().iterator()); + public CloseableIterator> entries(Serializable keyspace) { + return new ForwardingCloseableIterator>(getKeySpaceMap(keyspace).entrySet().iterator()); } /* @@ -180,7 +181,7 @@ public class MapKeyValueAdapter extends AbstractKeyValueAdapter { } /** - * Get map associated with given keyspace. + * Get map associated with given key space. * * @param keyspace must not be {@literal null}. * @return @@ -202,5 +203,4 @@ public class MapKeyValueAdapter extends AbstractKeyValueAdapter { private void addMapForKeySpace(Serializable keyspace) { store.put(keyspace, CollectionFactory. createMap(keySpaceMapType, 1000)); } - } diff --git a/src/test/java/org/springframework/data/keyvalue/core/ForwardingCloseableIteratorUnitTests.java b/src/test/java/org/springframework/data/keyvalue/core/ForwardingCloseableIteratorUnitTests.java index 4291cc8..c26d8b2 100644 --- a/src/test/java/org/springframework/data/keyvalue/core/ForwardingCloseableIteratorUnitTests.java +++ b/src/test/java/org/springframework/data/keyvalue/core/ForwardingCloseableIteratorUnitTests.java @@ -22,21 +22,24 @@ import static org.mockito.Mockito.*; import java.util.Iterator; import java.util.Map; +import java.util.Map.Entry; import java.util.NoSuchElementException; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.data.util.CloseableIterator; /** * @author Christoph Strobl * @author Thomas Darimont + * @author Oliver Gierke */ @RunWith(MockitoJUnitRunner.class) public class ForwardingCloseableIteratorUnitTests { - @Mock Iterator> iteratorMock; + @Mock Iterator> iteratorMock; @Mock Runnable closeActionMock; /** @@ -47,22 +50,33 @@ public class ForwardingCloseableIteratorUnitTests { when(iteratorMock.hasNext()).thenReturn(true); - assertThat(new ForwardingCloseableIterator(iteratorMock).hasNext(), is(true)); + CloseableIterator> iterator = new ForwardingCloseableIterator>(iteratorMock); - verify(iteratorMock, times(1)).hasNext(); + try { + assertThat(iterator.hasNext(), is(true)); + verify(iteratorMock, times(1)).hasNext(); + } finally { + iterator.close(); + } } /** * @see DATAKV-99 */ @Test + @SuppressWarnings("unchecked") public void nextShoudDelegateToWrappedIterator() { - when(iteratorMock.next()).thenReturn((Map.Entry) mock(Map.Entry.class)); + when(iteratorMock.next()).thenReturn((Entry) mock(Map.Entry.class)); - assertThat(new ForwardingCloseableIterator(iteratorMock).next(), notNullValue()); + CloseableIterator> iterator = new ForwardingCloseableIterator>(iteratorMock); - verify(iteratorMock, times(1)).next(); + try { + assertThat(iterator.next(), notNullValue()); + verify(iteratorMock, times(1)).next(); + } finally { + iterator.close(); + } } /** @@ -73,7 +87,13 @@ public class ForwardingCloseableIteratorUnitTests { when(iteratorMock.next()).thenThrow(new NoSuchElementException()); - new ForwardingCloseableIterator(iteratorMock).next(); + CloseableIterator> iterator = new ForwardingCloseableIterator>(iteratorMock); + + try { + iterator.next(); + } finally { + iterator.close(); + } } /** @@ -82,7 +102,7 @@ public class ForwardingCloseableIteratorUnitTests { @Test public void closeShouldDoNothingByDefault() { - new ForwardingCloseableIterator(iteratorMock).close(); + new ForwardingCloseableIterator>(iteratorMock).close(); verifyZeroInteractions(iteratorMock); } @@ -93,7 +113,7 @@ public class ForwardingCloseableIteratorUnitTests { @Test public void closeShouldInvokeConfiguredCloseAction() { - new ForwardingCloseableIterator(iteratorMock, closeActionMock).close(); + new ForwardingCloseableIterator>(iteratorMock, closeActionMock).close(); verify(closeActionMock, times(1)).run(); } diff --git a/src/test/java/ort/springframework/data/keyvalue/core/mapping/context/KeyValueMappingContextUnitTests.java b/src/test/java/ort/springframework/data/keyvalue/core/mapping/context/KeyValueMappingContextUnitTests.java deleted file mode 100644 index 0267f5f..0000000 --- a/src/test/java/ort/springframework/data/keyvalue/core/mapping/context/KeyValueMappingContextUnitTests.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2015 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 ort.springframework.data.keyvalue.core.mapping.context; - -import org.junit.Test; -import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext; - -public class KeyValueMappingContextUnitTests { - - /** - * @see DATAKV-105 - */ - @Test(expected = IllegalArgumentException.class) - public void shouldThrowExceptionWhenKeySpaceResolverIsNull() { - new KeyValueMappingContext(null); - } - -}