From e0972d57b90f46a8414ca40f8e9e19144c1d545a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Thu, 16 Jul 2015 16:09:50 +0200 Subject: [PATCH] DATACOUCH-140 - Use Converter to match query params and stored values. When deriving a query, use the CouchbaseConverter to transform the parameters for fields that would be converted upon storage. Reworked the DateConverters so that reading conversion is done from any Number (since N1QL may return longs in Double scientific notation for example). --- .../data/couchbase/repository/Party.java | 78 ++++++++++++++++ .../couchbase/repository/PartyRepository.java | 20 +++++ .../QueryDerivationConversionListener.java | 61 +++++++++++++ .../QueryDerivationConversionTests.java | 89 +++++++++++++++++++ .../convert/AbstractCouchbaseConverter.java | 18 ++++ .../core/convert/CouchbaseConverter.java | 19 ++++ .../core/convert/DateConverters.java | 48 +++++----- .../core/mapping/CouchbaseSimpleTypes.java | 1 + .../repository/query/ConvertingIterator.java | 51 +++++++++++ .../repository/query/N1qlQueryCreator.java | 23 +++-- .../query/ViewBasedCouchbaseQuery.java | 2 +- .../repository/query/ViewQueryCreator.java | 9 +- 12 files changed, 380 insertions(+), 39 deletions(-) create mode 100644 src/integration/java/org/springframework/data/couchbase/repository/Party.java create mode 100644 src/integration/java/org/springframework/data/couchbase/repository/PartyRepository.java create mode 100644 src/integration/java/org/springframework/data/couchbase/repository/QueryDerivationConversionListener.java create mode 100644 src/integration/java/org/springframework/data/couchbase/repository/QueryDerivationConversionTests.java create mode 100644 src/main/java/org/springframework/data/couchbase/repository/query/ConvertingIterator.java diff --git a/src/integration/java/org/springframework/data/couchbase/repository/Party.java b/src/integration/java/org/springframework/data/couchbase/repository/Party.java new file mode 100644 index 00000000..53ba990f --- /dev/null +++ b/src/integration/java/org/springframework/data/couchbase/repository/Party.java @@ -0,0 +1,78 @@ +package org.springframework.data.couchbase.repository; + +import java.util.Date; + +import org.springframework.data.annotation.Id; +import org.springframework.data.couchbase.core.mapping.Field; + +/** + * An entity used to test conversion of parameters in query derivations. + * + * @author Simon Baslé + */ +public class Party { + + @Id + private final String key; + + private final String name; + + @Field("desc") + private final String description; + + private final Date eventDate; + + private final long attendees; + + public Party(String key, String name, String description, Date eventDate, long attendees) { + this.key = key; + this.name = name; + this.description = description; + this.eventDate = eventDate; + this.attendees = attendees; + } + + public String getKey() { + return key; + } + + public String getName() { + return name; + } + + public String getDescription() { + return description; + } + + public Date getEventDate() { + return eventDate; + } + + public long getAttendees() { + return attendees; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + Party party = (Party) o; + + return key.equals(party.key); + + } + + @Override + public int hashCode() { + return key.hashCode(); + } + + @Override + public String toString() { + return "Party{" + + "name='" + name + '\'' + + ", eventDate=" + eventDate + + '}'; + } +} diff --git a/src/integration/java/org/springframework/data/couchbase/repository/PartyRepository.java b/src/integration/java/org/springframework/data/couchbase/repository/PartyRepository.java new file mode 100644 index 00000000..0eff1e5f --- /dev/null +++ b/src/integration/java/org/springframework/data/couchbase/repository/PartyRepository.java @@ -0,0 +1,20 @@ +package org.springframework.data.couchbase.repository; + +import java.util.Date; +import java.util.List; + +import org.springframework.data.couchbase.core.view.View; + +/** + * @author Simon Baslé + */ +public interface PartyRepository extends CouchbaseRepository { + + List findByAttendeesGreaterThanEqual(int minAttendees); + + List findByEventDateIs(Date targetDate); + + @View(designDocument = "party", viewName = "byDate") + List findFirst3ByEventDateGreaterThanEqual(Date targetDate); + +} diff --git a/src/integration/java/org/springframework/data/couchbase/repository/QueryDerivationConversionListener.java b/src/integration/java/org/springframework/data/couchbase/repository/QueryDerivationConversionListener.java new file mode 100644 index 00000000..b9bc209c --- /dev/null +++ b/src/integration/java/org/springframework/data/couchbase/repository/QueryDerivationConversionListener.java @@ -0,0 +1,61 @@ +package org.springframework.data.couchbase.repository; + +import java.util.Calendar; +import java.util.Collections; +import java.util.Date; +import java.util.List; + +import com.couchbase.client.java.Bucket; +import com.couchbase.client.java.PersistTo; +import com.couchbase.client.java.ReplicateTo; +import com.couchbase.client.java.view.DefaultView; +import com.couchbase.client.java.view.DesignDocument; +import com.couchbase.client.java.view.View; + +import org.springframework.data.couchbase.core.CouchbaseTemplate; +import org.springframework.test.context.TestContext; +import org.springframework.test.context.support.DependencyInjectionTestExecutionListener; + +/** + * @author Simon Baslé + */ +public class QueryDerivationConversionListener extends DependencyInjectionTestExecutionListener { + + @Override + public void beforeTestClass(final TestContext testContext) throws Exception { + Bucket client = (Bucket) testContext.getApplicationContext().getBean("couchbaseBucket"); + populateTestData(client); + createAndWaitForDesignDocs(client); + } + + private void populateTestData(Bucket client) { + CouchbaseTemplate template = new CouchbaseTemplate(client); + + Calendar cal = Calendar.getInstance(); + cal.clear(); + cal.set(Calendar.YEAR, 2015); + cal.set(Calendar.DAY_OF_MONTH, 10); + cal.set(Calendar.MONTH, Calendar.JANUARY); + for (int i = 0; i < 12; i++) { + Party p = new Party("testparty-" + i, "party like it's 199" + i, + "An awesome party, 90's themed, every 10 of the month", + cal.getTime(), 100 + i * 10); + template.save(p, PersistTo.MASTER, ReplicateTo.NONE); + cal.roll(Calendar.MONTH, true); + } + + cal.clear(); + cal.set(Calendar.YEAR, 1990); + cal.set(Calendar.MONTH, Calendar.JANUARY); + cal.set(Calendar.DAY_OF_MONTH, 01); + template.save(new Party("aTestParty", "New Year's Eve 90", "Happy New Year", cal.getTime(), 1230000)); + } + + private void createAndWaitForDesignDocs(Bucket client) { + String mapFunction = "function (doc, meta) { if(doc._class == \"" + Party.class.getName() + "\") { emit(doc.eventDate, null); } }"; + View view = DefaultView.create("byDate", mapFunction, "_count"); + List views = Collections.singletonList(view); + DesignDocument designDoc = DesignDocument.create("party", views); + client.bucketManager().upsertDesignDocument(designDoc); + } +} diff --git a/src/integration/java/org/springframework/data/couchbase/repository/QueryDerivationConversionTests.java b/src/integration/java/org/springframework/data/couchbase/repository/QueryDerivationConversionTests.java new file mode 100644 index 00000000..3cb14592 --- /dev/null +++ b/src/integration/java/org/springframework/data/couchbase/repository/QueryDerivationConversionTests.java @@ -0,0 +1,89 @@ +package org.springframework.data.couchbase.repository; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import java.util.Calendar; +import java.util.Date; +import java.util.List; + +import com.couchbase.client.java.Bucket; +import com.couchbase.client.java.document.JsonDocument; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.couchbase.IntegrationTestApplicationConfig; +import org.springframework.data.couchbase.core.CouchbaseTemplate; +import org.springframework.data.couchbase.repository.support.CouchbaseRepositoryFactory; +import org.springframework.data.repository.core.support.RepositoryFactorySupport; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestExecutionListeners; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * @author Simon Baslé + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration(classes = IntegrationTestApplicationConfig.class) +@TestExecutionListeners(QueryDerivationConversionListener.class) +public class QueryDerivationConversionTests { + + @Autowired + private Bucket client; + + @Autowired + private CouchbaseTemplate template; + + private PartyRepository repository; + + @Before + public void setup() throws Exception { + RepositoryFactorySupport factory = new CouchbaseRepositoryFactory(template); + repository = factory.getRepository(PartyRepository.class); + } + + @Test + public void testConvertsDateParameterInN1qlQuery() { + Party partyApril = repository.findOne("testparty-3"); + assertNotNull(partyApril); + System.out.println(partyApril); + + Calendar cal = Calendar.getInstance(); + cal.clear(); + cal.set(2015, Calendar.APRIL, 10); + Date find = cal.getTime(); + + List parties = repository.findByEventDateIs(find); + assertNotNull(parties); + assertEquals(1, parties.size()); + assertEquals(find, parties.get(0).getEventDate()); + + JsonDocument doc = client.get(parties.get(0).getKey()); + assertEquals(find.getTime(), doc.content().get("eventDate")); + } + + @Test + public void testAcceptLongParameterInN1qlQuery() { + List newYear90 = repository.findByAttendeesGreaterThanEqual(1200000); + assertNotNull(newYear90); + assertEquals(1, newYear90.size()); + assertEquals("aTestParty", newYear90.get(0).getKey()); + } + + @Test + public void testConvertDateParameterInViewQuery() { + Calendar cal = Calendar.getInstance(); + cal.clear(); + cal.set(2015, Calendar.AUGUST, 12); + Date find = cal.getTime(); + + List afterSummerParties = repository.findFirst3ByEventDateGreaterThanEqual(find); + assertNotNull(afterSummerParties); + assertEquals(3, afterSummerParties.size()); + for (Party afterSummerParty : afterSummerParties) { + assert(afterSummerParty.getEventDate().after(find)); + } + } +} diff --git a/src/main/java/org/springframework/data/couchbase/core/convert/AbstractCouchbaseConverter.java b/src/main/java/org/springframework/data/couchbase/core/convert/AbstractCouchbaseConverter.java index 58d8d9b4..38ae47df 100644 --- a/src/main/java/org/springframework/data/couchbase/core/convert/AbstractCouchbaseConverter.java +++ b/src/main/java/org/springframework/data/couchbase/core/convert/AbstractCouchbaseConverter.java @@ -88,4 +88,22 @@ public abstract class AbstractCouchbaseConverter implements CouchbaseConverter, conversions.registerConvertersIn(conversionService); } + @Override + public Object convertForWriteIfNeeded(Object value) { + if (value == null) { + return null; + } + + Class targetType = this.conversions.getCustomWriteTarget(value.getClass()); + if (targetType != null) { + return this.conversionService.convert(value, targetType); + } + return value; + } + + @Override + public Class getWriteClassFor(Class clazz) { + Class targetType = this.conversions.getCustomWriteTarget(clazz); + return targetType != null ? targetType : clazz; + } } diff --git a/src/main/java/org/springframework/data/couchbase/core/convert/CouchbaseConverter.java b/src/main/java/org/springframework/data/couchbase/core/convert/CouchbaseConverter.java index 64ffa4fc..d2cc1c96 100644 --- a/src/main/java/org/springframework/data/couchbase/core/convert/CouchbaseConverter.java +++ b/src/main/java/org/springframework/data/couchbase/core/convert/CouchbaseConverter.java @@ -26,10 +26,29 @@ import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProper * Marker interface for the converter, identifying the types to and from that can be converted. * * @author Michael Nitschinger + * @author Simon Baslé */ public interface CouchbaseConverter extends EntityConverter, CouchbasePersistentProperty, Object, CouchbaseDocument>, CouchbaseWriter, EntityReader { + + /** + * Convert the value if necessary to the class that would actually be stored, + * or leave it as is if no conversion needed. + * + * @param value the value to be converted to the class that would actually be stored. + * @return the converted value (or the same value if no conversion necessary). + */ + Object convertForWriteIfNeeded(Object value); + + /** + * Return the Class that would actually be stored for a given Class. + * + * @param clazz the source class. + * @return the target class that would actually be stored. + * @see #convertForWriteIfNeeded(Object) + */ + Class getWriteClassFor(Class clazz); } diff --git a/src/main/java/org/springframework/data/couchbase/core/convert/DateConverters.java b/src/main/java/org/springframework/data/couchbase/core/convert/DateConverters.java index f1e6a5eb..78ff3cde 100644 --- a/src/main/java/org/springframework/data/couchbase/core/convert/DateConverters.java +++ b/src/main/java/org/springframework/data/couchbase/core/convert/DateConverters.java @@ -54,18 +54,18 @@ public final class DateConverters { converters.add(DateToLongConverter.INSTANCE); converters.add(CalendarToLongConverter.INSTANCE); - converters.add(LongToDateConverter.INSTANCE); - converters.add(LongToCalendarConverter.INSTANCE); + converters.add(NumberToDateConverter.INSTANCE); + converters.add(NumberToCalendarConverter.INSTANCE); if (JODA_TIME_IS_PRESENT) { converters.add(LocalDateToLongConverter.INSTANCE); converters.add(LocalDateTimeToLongConverter.INSTANCE); converters.add(DateTimeToLongConverter.INSTANCE); converters.add(DateMidnightToLongConverter.INSTANCE); - converters.add(LongToLocalDateConverter.INSTANCE); - converters.add(LongToLocalDateTimeConverter.INSTANCE); - converters.add(LongToDateTimeConverter.INSTANCE); - converters.add(LongToDateMidnightConverter.INSTANCE); + converters.add(NumberToLocalDateConverter.INSTANCE); + converters.add(NumberToLocalDateTimeConverter.INSTANCE); + converters.add(NumberToDateTimeConverter.INSTANCE); + converters.add(NumberToDateMidnightConverter.INSTANCE); } return converters; @@ -92,33 +92,33 @@ public final class DateConverters { } @ReadingConverter - public enum LongToDateConverter implements Converter { + public enum NumberToDateConverter implements Converter { INSTANCE; @Override - public Date convert(Long source) { + public Date convert(Number source) { if (source == null) { return null; } Date date = new Date(); - date.setTime(source); + date.setTime(source.longValue()); return date; } } @ReadingConverter - public enum LongToCalendarConverter implements Converter { + public enum NumberToCalendarConverter implements Converter { INSTANCE; @Override - public Calendar convert(Long source) { + public Calendar convert(Number source) { if (source == null) { return null; } Calendar calendar = Calendar.getInstance(); - calendar.setTimeInMillis(source * 1000); + calendar.setTimeInMillis(source.longValue() * 1000); return calendar; } } @@ -164,42 +164,42 @@ public final class DateConverters { } @ReadingConverter - public enum LongToLocalDateConverter implements Converter { + public enum NumberToLocalDateConverter implements Converter { INSTANCE; @Override - public LocalDate convert(Long source) { - return source == null ? null : new LocalDate(source); + public LocalDate convert(Number source) { + return source == null ? null : new LocalDate(source.longValue()); } } @ReadingConverter - public enum LongToLocalDateTimeConverter implements Converter { + public enum NumberToLocalDateTimeConverter implements Converter { INSTANCE; @Override - public LocalDateTime convert(Long source) { - return source == null ? null : new LocalDateTime(source); + public LocalDateTime convert(Number source) { + return source == null ? null : new LocalDateTime(source.longValue()); } } @ReadingConverter - public enum LongToDateTimeConverter implements Converter { + public enum NumberToDateTimeConverter implements Converter { INSTANCE; @Override - public DateTime convert(Long source) { - return source == null ? null : new DateTime(source); + public DateTime convert(Number source) { + return source == null ? null : new DateTime(source.longValue()); } } @ReadingConverter - public enum LongToDateMidnightConverter implements Converter { + public enum NumberToDateMidnightConverter implements Converter { INSTANCE; @Override - public DateMidnight convert(Long source) { - return source == null ? null : new DateMidnight(source); + public DateMidnight convert(Number source) { + return source == null ? null : new DateMidnight(source.longValue()); } } diff --git a/src/main/java/org/springframework/data/couchbase/core/mapping/CouchbaseSimpleTypes.java b/src/main/java/org/springframework/data/couchbase/core/mapping/CouchbaseSimpleTypes.java index 6d450894..bb403585 100644 --- a/src/main/java/org/springframework/data/couchbase/core/mapping/CouchbaseSimpleTypes.java +++ b/src/main/java/org/springframework/data/couchbase/core/mapping/CouchbaseSimpleTypes.java @@ -32,6 +32,7 @@ public abstract class CouchbaseSimpleTypes { Set> simpleTypes = new HashSet>(); simpleTypes.add(RawJsonDocument.class); simpleTypes.add(JsonArray.class); + simpleTypes.add(Number.class); COUCHBASE_SIMPLE_TYPES = Collections.unmodifiableSet(simpleTypes); } diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/ConvertingIterator.java b/src/main/java/org/springframework/data/couchbase/repository/query/ConvertingIterator.java new file mode 100644 index 00000000..af00dae3 --- /dev/null +++ b/src/main/java/org/springframework/data/couchbase/repository/query/ConvertingIterator.java @@ -0,0 +1,51 @@ +/* + * 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.repository.query; + +import java.util.Iterator; + +import org.springframework.data.couchbase.core.convert.CouchbaseConverter; + +/** + * An {@link Iterator Iterator<Object>} that {@link CouchbaseConverter#convertForWriteIfNeeded(Object) converts} + * values to their stored Class if warranted. + */ +class ConvertingIterator implements Iterator { + private final Iterator delegate; + private final CouchbaseConverter converter; + + public ConvertingIterator(Iterator delegate, CouchbaseConverter converter) { + this.delegate = delegate; + this.converter = converter; + } + + @Override + public boolean hasNext() { + return delegate.hasNext(); + } + + @Override + public void remove() { + delegate.remove(); + } + + @Override + public Object next() { + Object next = delegate.next(); + return converter.convertForWriteIfNeeded(next); + } +} diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java b/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java index ff14d683..9ff7c9f0 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java +++ b/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java @@ -32,10 +32,8 @@ import com.couchbase.client.java.query.dsl.path.OrderByPath; import com.couchbase.client.java.query.dsl.path.WherePath; import org.springframework.data.couchbase.core.convert.CouchbaseConverter; -import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity; import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty; import org.springframework.data.domain.Sort; -import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.context.PersistentPropertyPath; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.parser.AbstractQueryCreator; @@ -91,22 +89,19 @@ import org.springframework.data.repository.query.parser.PartTree; */ public class N1qlQueryCreator extends AbstractQueryCreator { - private final ParameterAccessor accessor; private final WherePath selectFrom; - private final MappingContext, CouchbasePersistentProperty> context; + private final CouchbaseConverter converter; public N1qlQueryCreator(PartTree tree, ParameterAccessor parameters, WherePath selectFrom, CouchbaseConverter converter) { super(tree, parameters); - this.accessor = parameters; this.selectFrom = selectFrom; - this.context = converter.getMappingContext(); + this.converter = converter; } @Override protected Expression create(Part part, Iterator iterator) { - PersistentPropertyPath path = context.getPersistentPropertyPath(part.getProperty()); - return prepareExpression(part, path, iterator); + return prepareExpression(part, iterator); } @Override @@ -144,15 +139,17 @@ public class N1qlQueryCreator extends AbstractQueryCreator path, - Iterator parameterValues) { - //FIXME use conversions for the parameters and types + protected Expression prepareExpression(Part part, Iterator iterator) { + PersistentPropertyPath path = converter.getMappingContext() + .getPersistentPropertyPath(part.getProperty()); + ConvertingIterator parameterValues = new ConvertingIterator(iterator, converter); + //get the whole doted path with fieldNames instead of potentially wrong propNames String fieldNamePath = path.toDotPath(CouchbasePersistentProperty.FIELD_NAME); //deal with ignore case boolean ignoreCase = false; - Class leafType = path.getLeafProperty().getType(); + Class leafType = converter.getWriteClassFor(path.getLeafProperty().getType()); boolean isString = leafType == String.class; if (part.shouldIgnoreCase() == Part.IgnoreCaseType.WHEN_POSSIBLE) { ignoreCase = isString; @@ -225,6 +222,7 @@ public class N1qlQueryCreator extends AbstractQueryCreator parameterValues) { //TODO migrate to using the Functions util class when 2.0-dp2 / 2.0 GA Object next = parameterValues.next(); + String pattern; if (next == null) { pattern = ""; @@ -298,4 +296,5 @@ public class N1qlQueryCreator extends AbstractQueryCreator private ViewQuery query; private final PartTree tree; private final int treeCount; + private final CouchbaseConverter converter; - public ViewQueryCreator(PartTree tree, ParameterAccessor parameters, ViewQuery query) { + public ViewQueryCreator(PartTree tree, ParameterAccessor parameters, ViewQuery query, CouchbaseConverter converter) { super(tree, parameters); this.query = query; this.tree = tree; + this.converter = converter; //sanity check the partTree since we have strong restrictions on what's supported: int i = 0; @@ -86,7 +89,9 @@ public class ViewQueryCreator extends AbstractQueryCreator } @Override - protected ViewQuery create(Part part, Iterator iterator) { + protected ViewQuery create(Part part, Iterator objectIterator) { + ConvertingIterator iterator = new ConvertingIterator(objectIterator, converter); + switch (part.getType()) { case GREATER_THAN_EQUAL: startKey(iterator);