Allow extensible type conversion

elide-core/src/main/java/com/yahoo/elide/ElideSettings.java

@@ -13,6 +13,7 @@
 import com.yahoo.elide.core.filter.dialect.SubqueryFilterDialect;
 import com.yahoo.elide.jsonapi.JsonApiMapper;
 import com.yahoo.elide.security.PermissionExecutor;
+import com.yahoo.elide.utils.coerce.converters.ElideConverter;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 
@@ -35,4 +36,5 @@ public class ElideSettings {
     @Getter private final int defaultPageSize;
     @Getter private final boolean useFilterExpressions;
     @Getter private final int updateStatusCode;
+    @Getter private final ElideConverter converter;
 }

elide-core/src/main/java/com/yahoo/elide/ElideSettingsBuilder.java

@@ -18,12 +18,15 @@
 import com.yahoo.elide.jsonapi.JsonApiMapper;
 import com.yahoo.elide.security.PermissionExecutor;
 import com.yahoo.elide.security.executors.ActivePermissionExecutor;
+import com.yahoo.elide.utils.coerce.CoerceUtil;
+import com.yahoo.elide.utils.coerce.converters.ElideConverter;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.function.Function;
 
 /**
@@ -33,15 +36,17 @@ public class ElideSettingsBuilder {
     private final DataStore dataStore;
     private AuditLogger auditLogger;
     private JsonApiMapper jsonApiMapper;
-    private EntityDictionary entityDictionary = new EntityDictionary(new HashMap<>());
-    private Function<RequestScope, PermissionExecutor> permissionExecutorFunction = ActivePermissionExecutor::new;
     private List<JoinFilterDialect> joinFilterDialects;
     private List<SubqueryFilterDialect> subqueryFilterDialects;
-    private int defaultMaxPageSize = Pagination.MAX_PAGE_LIMIT;
-    private int defaultPageSize = Pagination.DEFAULT_PAGE_LIMIT;
     private boolean useFilterExpressions;
     private int updateStatusCode;
 
+    private EntityDictionary entityDictionary = new EntityDictionary(new HashMap<>());
+    private Function<RequestScope, PermissionExecutor> permissionExecutorFunction = ActivePermissionExecutor::new;
+    private int defaultMaxPageSize = Pagination.MAX_PAGE_LIMIT;
+    private int defaultPageSize = Pagination.DEFAULT_PAGE_LIMIT;
+    private ElideConverter converter = new ElideConverter();
+
     /**
      * A new builder used to generate Elide instances. Instantiates an {@link EntityDictionary} without
      * providing a mapping of security checks and uses the provided {@link Slf4jLogger} for audit.
@@ -66,6 +71,8 @@ public ElideSettings build() {
             subqueryFilterDialects.add(new DefaultFilterDialect(entityDictionary));
         }
 
+        CoerceUtil.setup(converter);
+
         return new ElideSettings(
                 auditLogger,
                 dataStore,
@@ -77,7 +84,8 @@ public ElideSettings build() {
                 defaultMaxPageSize,
                 defaultPageSize,
                 useFilterExpressions,
-                updateStatusCode);
+                updateStatusCode,
+                converter);
     }
 
     public ElideSettingsBuilder withAuditLogger(AuditLogger auditLogger) {
@@ -156,4 +164,16 @@ public ElideSettingsBuilder withUseFilterExpressions(boolean useFilterExpression
         this.useFilterExpressions = useFilterExpressions;
         return this;
     }
+
+    /**
+     * You should strongly consider extending the map build by {@code ElideConverter.defaultConverters}. If you
+     * do not then you will lose Elide's default behavior for Maps and Enums
+     *
+     * @param converters
+     * @return
+     */
+    public ElideSettingsBuilder withConverters(Map<Integer, ElideConverter.TypeCoercer> converters) {
+        this.converter = new ElideConverter(converters);
+        return this;
+    }
 }

elide-core/src/main/java/com/yahoo/elide/utils/coerce/CoerceUtil.java

@@ -7,31 +7,19 @@
 
 import com.yahoo.elide.core.exceptions.InvalidAttributeException;
 import com.yahoo.elide.core.exceptions.InvalidValueException;
-import com.yahoo.elide.utils.coerce.converters.EpochToDateConverter;
-import com.yahoo.elide.utils.coerce.converters.FromMapConverter;
-import com.yahoo.elide.utils.coerce.converters.ToEnumConverter;
+import com.yahoo.elide.utils.coerce.converters.ElideConverter;
 import org.apache.commons.beanutils.BeanUtilsBean;
 import org.apache.commons.beanutils.ConversionException;
 import org.apache.commons.beanutils.ConvertUtils;
-import org.apache.commons.beanutils.ConvertUtilsBean;
-import org.apache.commons.beanutils.Converter;
-import org.apache.commons.lang3.ClassUtils;
-
-import java.util.Date;
-import java.util.Map;
 
 /**
  * Class for coercing a value to a target class.
  */
 public class CoerceUtil {
 
-    private static final ToEnumConverter TO_ENUM_CONVERTER = new ToEnumConverter();
-    private static final FromMapConverter FROM_MAP_CONVERTER = new FromMapConverter();
-    private static final EpochToDateConverter EPOCH_TO_DATE_CONVERTER = new EpochToDateConverter();
-
     //static block for setup and registering new converters
     static {
-        setup();
+        setup(new ElideConverter());
     }
 
     /**
@@ -55,32 +43,9 @@ public static <T> T coerce(Object value, Class<T> cls) {
 
     /**
      * Perform CoerceUtil setup.
+     * @param elideConverter the converter to register
      */
-    private static void setup() {
-        BeanUtilsBean.setInstance(new BeanUtilsBean(new ConvertUtilsBean() {
-            {
-                // https://github.com/yahoo/elide/issues/260
-                // enable throwing exceptions when conversion fails
-                register(true, false, 0);
-            }
-
-            @Override
-            /*
-             * Overriding lookup to execute enum converter if target is enum
-             * or map convert if source is map
-             */
-            public Converter lookup(Class<?> sourceType, Class<?> targetType) {
-                if (targetType.isEnum()) {
-                    return TO_ENUM_CONVERTER;
-                } else if (Map.class.isAssignableFrom(sourceType)) {
-                    return FROM_MAP_CONVERTER;
-                } else if ((String.class.isAssignableFrom(sourceType) || Number.class.isAssignableFrom(sourceType))
-                        && ClassUtils.isAssignable(targetType, Date.class)) {
-                    return EPOCH_TO_DATE_CONVERTER;
-                } else {
-                    return super.lookup(sourceType, targetType);
-                }
-            }
-        }));
+    public static void setup(ElideConverter elideConverter) {
+        BeanUtilsBean.setInstance(new BeanUtilsBean(elideConverter));
     }
 }

elide-core/src/main/java/com/yahoo/elide/utils/coerce/converters/ElideConverter.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright 2017, Yahoo Inc.
+ * Licensed under the Apache License, Version 2.0
+ * See LICENSE file in project root for terms.
+ */
+
+package com.yahoo.elide.utils.coerce.converters;
+
+import com.google.common.collect.Multimaps;
+import com.google.common.collect.SortedSetMultimap;
+import com.google.common.collect.TreeMultimap;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import org.apache.commons.beanutils.ConvertUtilsBean;
+import org.apache.commons.beanutils.Converter;
+import org.apache.commons.lang3.ClassUtils;
+
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.BiFunction;
+
+/**
+ * A class that knows how to convert thing to other (different) things.
+ */
+public class ElideConverter extends ConvertUtilsBean {
+    private static final ToEnumConverter TO_ENUM_CONVERTER = new ToEnumConverter();
+    private static final FromMapConverter FROM_MAP_CONVERTER = new FromMapConverter();
+    private static final EpochToDateConverter EPOCH_TO_DATE_CONVERTER = new EpochToDateConverter();
+
+    public static final BiFunction<Class<?>, Class<?>, Boolean> SOURCE_IS_MAP = (source, target) ->
+            Map.class.isAssignableFrom(source);
+    public static final BiFunction<Class<?>, Class<?>, Boolean> TARGET_IS_ENUM = (source, target) ->
+            target.isEnum();
+    public static final BiFunction<Class<?>, Class<?>, Boolean> STR_NUM_TO_DATE = (source, target) ->
+            (String.class.isAssignableFrom(source) || Number.class.isAssignableFrom(source))
+                    && ClassUtils.isAssignable(target, Date.class);
+    public static final int HIGH_PRIORITY = 10;
+    public static final int MEDIUM_PRIORITY = 20;
+    public static final int LOW_PRIORITY = 30;
+
+    private static SortedSetMultimap<Integer, TypeCoercer> CONVERTERS = Multimaps.synchronizedSortedSetMultimap(
+            TreeMultimap.<Integer, TypeCoercer>create());
+
+    /**
+     * Create a new ElideConverter with a set of converters.
+     *
+     * @param converters extra converters to consider before the default converters fire
+     */
+    public ElideConverter(Map<Integer, TypeCoercer> converters) {
+        register(true, false, 0); // #260 - throw exceptions when conversion fails
+        converters.forEach((key, value) -> CONVERTERS.put(key, value));
+    }
+
+    /**
+     * Create a new ElideConverter with the default set of converters.
+     */
+    public ElideConverter() {
+        this(defaultConverters());
+    }
+
+    /*
+     * Overriding lookup to execute enum converter if target is enum
+     * or map convert if source is map
+     */
+    @Override
+    public Converter lookup(Class<?> sourceType, Class<?> targetType) {
+        return CONVERTERS.values().stream()
+                .filter(tc ->  tc.discriminator.apply(sourceType, targetType))
+                .map(TypeCoercer::getConverter)
+                .findFirst()
+                .orElse(super.lookup(sourceType, targetType));
+    }
+
+    public static Map<Integer, TypeCoercer> defaultConverters() {
+        Map<Integer, TypeCoercer> converters = new HashMap<>();
+        converters.put(HIGH_PRIORITY, new TypeCoercer(TARGET_IS_ENUM, TO_ENUM_CONVERTER));
+        converters.put(MEDIUM_PRIORITY, new TypeCoercer(SOURCE_IS_MAP, FROM_MAP_CONVERTER));
+        converters.put(LOW_PRIORITY, new TypeCoercer(STR_NUM_TO_DATE, EPOCH_TO_DATE_CONVERTER));
+        return converters;
+    }
+
+    /**
+     * Value type for registering converter with ElideConverter.
+     */
+    @RequiredArgsConstructor
+    public static class TypeCoercer implements Comparable<TypeCoercer> {
+        public final BiFunction<Class<?>, Class<?>, Boolean> discriminator;
+        @Getter public final Converter converter;
+
+        @Override
+        public int compareTo(TypeCoercer o) {
+            if (this == o) {
+                return 0;
+            }
+            return Integer.compare(hashCode(), o.hashCode());
+        }
+    }
+}

elide-integration-tests/src/test/groovy/com/yahoo/elide/tests/FilterIT.groovy

@@ -4,6 +4,7 @@
  * See LICENSE file in project root for terms.
  */
 package com.yahoo.elide.tests
+
 import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.jayway.restassured.RestAssured
@@ -13,6 +14,7 @@ import org.testng.Assert
 import org.testng.annotations.AfterTest
 import org.testng.annotations.BeforeClass
 import org.testng.annotations.Test
+
 /**
  * Tests for Filters
  */
@@ -434,7 +436,7 @@ class FilterIT extends AbstractIntegrationTestInitializer {
                 .then()
                 .statusCode(HttpStatus.SC_BAD_REQUEST)
 
-         /* Test RSQL Global */
+        /* Test RSQL Global */
         RestAssured
                 .get("/book?filter=title=invalid=Ignored")
                 .then()
@@ -1459,15 +1461,15 @@ class FilterIT extends AbstractIntegrationTestInitializer {
         def result = mapper.readTree(RestAssured.get("book?filter[book.author12.name]=Null Ned").asString())
         Assert.assertEquals(result.get("errors").get(0).asText(),
                 "InvalidPredicateException: Unknown field in filter: author12\n" +
-                "Invalid query parameter: filter[book.author12.name]")
+                        "Invalid query parameter: filter[book.author12.name]")
 
         /* Test RSQL Global */
         result = mapper.readTree(RestAssured.get("book?filter=author12.name=='Null Ned'").asString())
         Assert.assertEquals(result.get("errors").get(0).asText(),
                 "InvalidPredicateException: Invalid filter format: filter\n" +
-                "No such association author12 for type book\n" +
-                "Invalid filter format: filter\n" +
-                "Invalid query parameter: filter")
+                        "No such association author12 for type book\n" +
+                        "Invalid filter format: filter\n" +
+                        "Invalid query parameter: filter")
     }
 
     @Test
@@ -1587,9 +1589,9 @@ class FilterIT extends AbstractIntegrationTestInitializer {
         result = mapper.readTree(RestAssured.get("/author?filter=idontexist.books.title=in=('Viva la Roma!','Mamma mia I wantz some pizza!')").asString())
         Assert.assertEquals(result.get("errors").get(0).asText(),
                 "InvalidPredicateException: Invalid filter format: filter\n" +
-                "No such association idontexist for type author\n" +
-                "Invalid filter format: filter\n" +
-                "Invalid query parameter: filter")
+                        "No such association idontexist for type author\n" +
+                        "Invalid filter format: filter\n" +
+                        "Invalid query parameter: filter")
     }
 
     @Test
@@ -1604,9 +1606,9 @@ class FilterIT extends AbstractIntegrationTestInitializer {
         result = mapper.readTree(RestAssured.get("/author?filter=idontexist.title=in=('Viva la Roma!','Mamma mia I wantz some pizza!')").asString())
         Assert.assertEquals(result.get("errors").get(0).asText(),
                 "InvalidPredicateException: Invalid filter format: filter\n" +
-                "No such association idontexist for type author\n" +
-                "Invalid filter format: filter\n" +
-                "Invalid query parameter: filter")
+                        "No such association idontexist for type author\n" +
+                        "Invalid filter format: filter\n" +
+                        "Invalid query parameter: filter")
     }
 
     @Test
@@ -1626,20 +1628,29 @@ class FilterIT extends AbstractIntegrationTestInitializer {
                         "Invalid query parameter: filter")
     }
 
+    /*
+     * Verify that a combination of filters and order by generate working SQL.
+     */
+    @Test
+    void testFilterWithSort() {
+        def result = mapper.readTree(RestAssured.get("/author/${asimovId}/books?filter[book.title][notnull]=true&sort=title").asString())
+        JsonNode data = result.get("data")
+        Assert.assertEquals(data.size(), 2)
+    }
 
     @AfterTest
     void cleanUp() {
         for (int id : authorIds) {
             RestAssured
                     .given()
                     .accept("application/vnd.api+json ext=jsonpatch")
-                    .delete("/author/"+id)
+                    .delete("/author/" + id)
         }
         for (int id : bookIds) {
             RestAssured
                     .given()
                     .accept("application/vnd.api+json ext=jsonpatch")
-                    .delete("/book/"+id)
+                    .delete("/book/" + id)
         }
     }
 }