From 5114ddb8678f407835ef3d6b498648ef5e2274e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Mon, 20 Jul 2015 17:12:05 +0200 Subject: [PATCH] DATACOUCH-145 - Use Id/Field annotations from Couchbase SDK The @Field annotation has been removed in favor of the one from the Couchbase SDK. Similarly, CouchbasePersistentEntity now recognize the SDK's @Id annotation. Note that if both Spring Data and Couchbase @Id are present (on two distinct fields), the one from Spring Data will be taken into account. --- .../core/CouchbaseTemplateTests.java | 2 +- .../data/couchbase/repository/Party.java | 2 +- .../data/couchbase/repository/cdi/Person.java | 2 +- src/main/asciidoc/entity.adoc | 14 +++- .../BasicCouchbasePersistentEntity.java | 34 +++++++++ .../BasicCouchbasePersistentProperty.java | 13 +++- .../data/couchbase/core/mapping/Field.java | 37 ---------- .../data/couchbase/core/Beer.java | 2 +- ...BasicCouchbasePersistentPropertyTests.java | 73 ++++++++++++++++++- .../core/mapping/CustomConvertersTests.java | 5 +- .../MappingCouchbaseConverterTests.java | 36 +++++++++ 11 files changed, 166 insertions(+), 54 deletions(-) delete mode 100644 src/main/java/org/springframework/data/couchbase/core/mapping/Field.java diff --git a/src/integration/java/org/springframework/data/couchbase/core/CouchbaseTemplateTests.java b/src/integration/java/org/springframework/data/couchbase/core/CouchbaseTemplateTests.java index 75b5d632..394adefa 100644 --- a/src/integration/java/org/springframework/data/couchbase/core/CouchbaseTemplateTests.java +++ b/src/integration/java/org/springframework/data/couchbase/core/CouchbaseTemplateTests.java @@ -52,7 +52,7 @@ import org.springframework.data.annotation.Version; import org.springframework.data.couchbase.IntegrationTestApplicationConfig; import org.springframework.data.couchbase.core.convert.MappingCouchbaseConverter; import org.springframework.data.couchbase.core.mapping.Document; -import org.springframework.data.couchbase.core.mapping.Field; +import com.couchbase.client.java.repository.annotation.Field; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.TestExecutionListeners; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; diff --git a/src/integration/java/org/springframework/data/couchbase/repository/Party.java b/src/integration/java/org/springframework/data/couchbase/repository/Party.java index 53ba990f..4ce3dd47 100644 --- a/src/integration/java/org/springframework/data/couchbase/repository/Party.java +++ b/src/integration/java/org/springframework/data/couchbase/repository/Party.java @@ -3,7 +3,7 @@ package org.springframework.data.couchbase.repository; import java.util.Date; import org.springframework.data.annotation.Id; -import org.springframework.data.couchbase.core.mapping.Field; +import com.couchbase.client.java.repository.annotation.Field; /** * An entity used to test conversion of parameters in query derivations. diff --git a/src/integration/java/org/springframework/data/couchbase/repository/cdi/Person.java b/src/integration/java/org/springframework/data/couchbase/repository/cdi/Person.java index a726d414..cedff7d1 100644 --- a/src/integration/java/org/springframework/data/couchbase/repository/cdi/Person.java +++ b/src/integration/java/org/springframework/data/couchbase/repository/cdi/Person.java @@ -17,7 +17,7 @@ package org.springframework.data.couchbase.repository.cdi; import org.springframework.data.annotation.Id; -import org.springframework.data.couchbase.core.mapping.Field; +import com.couchbase.client.java.repository.annotation.Field; /** * @author Mark Paluch diff --git a/src/main/asciidoc/entity.adoc b/src/main/asciidoc/entity.adoc index b28ca7a7..cdca22b3 100644 --- a/src/main/asciidoc/entity.adoc +++ b/src/main/asciidoc/entity.adoc @@ -6,17 +6,23 @@ This chapter describes how to model Entities and explains their counterpart repr [[basics]] == Documents and Fields -All entities should be annotated with the `@Document` annotation. Also, every field in the entity should be annotated with the `@Field` annotation. While this is - strictly speaking - optional, it helps to reduce edge cases and clearly shows the intent and design of the entity. +All entities should be annotated with the `@Document` annotation. -There is also a special `@Id` annotation which needs to be always in place. Best practice is to also name the property `id`. Here is a very simple `User` entity: +Also, every field in the entity should be annotated with the `@Field` annotation from the Couchbase SDK. While this is - strictly speaking - optional, it helps to reduce edge cases and clearly shows the intent and design of the entity. It can also be used to store the field under a different name. + +There is also a special `@Id` annotation which needs to be always in place. Best practice is to also name the property `id`. + +TIP: Both the Couchbase SDK and Spring Data define their own `@Id` annotation. Either can be used (the Spring Data one will get priority if both are found on different fields). + +Here is a very simple `User` entity: .A simple Document with Fields ==== [source,java] ---- -import org.springframework.data.annotation.Id; +import com.couchbase.client.java.repository.annotation.Id; +import com.couchbase.client.java.repository.annotation.Field; import org.springframework.data.couchbase.core.mapping.Document; -import org.springframework.data.couchbase.core.mapping.Field; @Document public class User { diff --git a/src/main/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentEntity.java b/src/main/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentEntity.java index 16f84223..df697fac 100644 --- a/src/main/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentEntity.java +++ b/src/main/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentEntity.java @@ -16,6 +16,8 @@ package org.springframework.data.couchbase.core.mapping; +import com.couchbase.client.java.repository.annotation.Id; + import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -61,6 +63,38 @@ public class BasicCouchbasePersistentEntity extends BasicPersistentEntity *

This object is used to gather information out of properties on objects that need to be persisted. For example, it * supports overriding of the actual property name by providing custom annotations.

@@ -73,8 +75,8 @@ public class BasicCouchbasePersistentProperty */ @Override public String getFieldName() { - org.springframework.data.couchbase.core.mapping.Field annotation = getField(). - getAnnotation(org.springframework.data.couchbase.core.mapping.Field.class); + com.couchbase.client.java.repository.annotation.Field annotation = getField(). + getAnnotation(com.couchbase.client.java.repository.annotation.Field.class); if (annotation != null && StringUtils.hasText(annotation.value())) { return annotation.value(); @@ -90,4 +92,9 @@ public class BasicCouchbasePersistentProperty return fieldName; } + // DATACOUCH-145: allows SDK's @Id annotation to be used + @Override + public boolean isIdProperty() { + return isAnnotationPresent(Id.class) || super.isIdProperty(); + } } diff --git a/src/main/java/org/springframework/data/couchbase/core/mapping/Field.java b/src/main/java/org/springframework/data/couchbase/core/mapping/Field.java deleted file mode 100644 index 5bb09e30..00000000 --- a/src/main/java/org/springframework/data/couchbase/core/mapping/Field.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright 2012-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 org.springframework.data.couchbase.core.mapping; - -import java.lang.annotation.Documented; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; - -/** - * Annotation to define custom metadata for document fields. - * - * @author Michael Nitschinger - */ -@Documented -@Retention(RetentionPolicy.RUNTIME) -public @interface Field { - - /** - * The key to be used to store the field inside the document. - */ - String value() default ""; - -} diff --git a/src/test/java/org/springframework/data/couchbase/core/Beer.java b/src/test/java/org/springframework/data/couchbase/core/Beer.java index e801c296..59064f46 100644 --- a/src/test/java/org/springframework/data/couchbase/core/Beer.java +++ b/src/test/java/org/springframework/data/couchbase/core/Beer.java @@ -17,7 +17,7 @@ package org.springframework.data.couchbase.core; import org.springframework.data.annotation.Id; -import org.springframework.data.couchbase.core.mapping.Field; +import com.couchbase.client.java.repository.annotation.Field; /** diff --git a/src/test/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentPropertyTests.java b/src/test/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentPropertyTests.java index 3c108e96..223b437f 100644 --- a/src/test/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentPropertyTests.java +++ b/src/test/java/org/springframework/data/couchbase/core/mapping/BasicCouchbasePersistentPropertyTests.java @@ -17,6 +17,8 @@ package org.springframework.data.couchbase.core.mapping; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.lang.reflect.Field; @@ -24,6 +26,7 @@ import org.junit.Before; import org.junit.Test; import org.springframework.data.annotation.Id; +import org.springframework.data.mapping.model.MappingException; import org.springframework.data.mapping.model.PropertyNameFieldNamingStrategy; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.util.ClassTypeInformation; @@ -68,6 +71,55 @@ public class BasicCouchbasePersistentPropertyTests { assertEquals("foobar", getPropertyFor(field).getFieldName()); } + @Test + public void testPrefersSpringIdAnnotation() { + BasicCouchbasePersistentEntity test = new BasicCouchbasePersistentEntity( + ClassTypeInformation.from(Beer.class)); + + Field sdkIdField = ReflectionUtils.findField(Beer.class, "sdkId"); + CouchbasePersistentProperty sdkIdProperty = getPropertyFor(sdkIdField); + Field springIdField = ReflectionUtils.findField(Beer.class, "springId"); + CouchbasePersistentProperty springIdProperty = getPropertyFor(springIdField); + test.addPersistentProperty(sdkIdProperty); + test.addPersistentProperty(springIdProperty); + + assertEquals("sdkId", sdkIdProperty.getFieldName()); + assertEquals("springId", springIdProperty.getFieldName()); + + assertTrue(sdkIdProperty.isIdProperty()); + assertTrue(springIdProperty.isIdProperty()); + + assertEquals(springIdProperty, test.getIdProperty()); + } + + @Test + public void testAcceptsSdkIdAnnotation() { + BasicCouchbasePersistentEntity test = new BasicCouchbasePersistentEntity( + ClassTypeInformation.from(SdkIdentified.class)); + Field id = ReflectionUtils.findField(SdkIdentified.class, "id"); + CouchbasePersistentProperty idProperty = getPropertyFor(id); + test.addPersistentProperty(idProperty); + + assertEquals(idProperty, test.getIdProperty()); + } + + @Test + public void testSdkIdAnnotationEvaluatedAfterSpringIdAnnotationIsIgnored() { + BasicCouchbasePersistentEntity test = new BasicCouchbasePersistentEntity( + ClassTypeInformation.from(Beer.class)); + Field sdkIdField = ReflectionUtils.findField(Beer.class, "sdkId"); + CouchbasePersistentProperty sdkIdProperty = getPropertyFor(sdkIdField); + Field springIdField = ReflectionUtils.findField(Beer.class, "springId"); + CouchbasePersistentProperty springIdProperty = getPropertyFor(springIdField); + + //here this simulates the order in which the annotations would be found + // when "overriding" Spring @Id with SDK's @Id... + test.addPersistentProperty(springIdProperty); + assertEquals(springIdProperty, test.getIdProperty()); + test.addPersistentProperty(sdkIdProperty); + assertEquals(springIdProperty, test.getIdProperty()); + } + /** * Helper method to create a property out of the field. * @@ -84,16 +136,29 @@ public class BasicCouchbasePersistentPropertyTests { */ public class Beer { - @Id - private String id; + @com.couchbase.client.java.repository.annotation.Id + private String sdkId; - @org.springframework.data.couchbase.core.mapping.Field("foobar") + @Id + private String springId; + + @com.couchbase.client.java.repository.annotation.Field("foobar") String name; String description; public String getId() { - return id; + return springId; } } + + /** + * Simple POJO to test that a single ID property from the SDK is taken into account. + */ + public class SdkIdentified { + @com.couchbase.client.java.repository.annotation.Id + private String id; + + String value; + } } diff --git a/src/test/java/org/springframework/data/couchbase/core/mapping/CustomConvertersTests.java b/src/test/java/org/springframework/data/couchbase/core/mapping/CustomConvertersTests.java index f0879aaa..1bfbce4c 100644 --- a/src/test/java/org/springframework/data/couchbase/core/mapping/CustomConvertersTests.java +++ b/src/test/java/org/springframework/data/couchbase/core/mapping/CustomConvertersTests.java @@ -25,13 +25,14 @@ import java.util.Collections; import java.util.Date; import java.util.List; +import com.couchbase.client.java.repository.annotation.Field; +import com.couchbase.client.java.repository.annotation.Id; import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.convert.converter.Converter; -import org.springframework.data.annotation.Id; import org.springframework.data.convert.ReadingConverter; import org.springframework.data.convert.WritingConverter; import org.springframework.data.couchbase.UnitTestApplicationConfig; @@ -121,7 +122,7 @@ public class CustomConvertersTests { } public static class BlogPost { - @Id + @Id //also tests DATACOUCH-145 (this is SDK's @Id) public String id = "key"; @Field diff --git a/src/test/java/org/springframework/data/couchbase/core/mapping/MappingCouchbaseConverterTests.java b/src/test/java/org/springframework/data/couchbase/core/mapping/MappingCouchbaseConverterTests.java index ca3f92da..4912c3fd 100644 --- a/src/test/java/org/springframework/data/couchbase/core/mapping/MappingCouchbaseConverterTests.java +++ b/src/test/java/org/springframework/data/couchbase/core/mapping/MappingCouchbaseConverterTests.java @@ -483,6 +483,25 @@ public class MappingCouchbaseConverterTests { assertEquals(deleted.toDate().getTime(), read.deleted.toDate().getTime()); } + @Test + public void shouldPrioritizeSpringIdOverSdkId() { + SpringIdentified entity = new SpringIdentified(); + CouchbaseDocument converted = new CouchbaseDocument(); + converter.write(entity, converted); + + assertEquals("realId", converted.getId()); + } + + @Test + public void shouldIgnoreSdkIdIfSpringIdFound() { + AmbiguousIdentified entity = new AmbiguousIdentified(); + CouchbaseDocument converted = new CouchbaseDocument(); + converter.write(entity, converted); + + assertEquals("springId", converted.getId()); + } + + static class EntityWithoutID { private String attr0; @@ -661,4 +680,21 @@ public class MappingCouchbaseConverterTests { } } + static class SdkIdentified { + @com.couchbase.client.java.repository.annotation.Id + public String id = "id"; + } + + static class SpringIdentified extends SdkIdentified { + @org.springframework.data.annotation.Id + public String realId = "realId"; + } + + static class AmbiguousIdentified { + @org.springframework.data.annotation.Id + public String springId = "springId"; + + @com.couchbase.client.java.repository.annotation.Id + public String sdkId = "sdkId"; + } }