Merge pull request #2 from Nosto/bug/namingStrategyWithCustomNames

Handle custom property names when checking naming strategy

.gitignore

@@ -12,3 +12,4 @@ gradle-app.setting
 
 # # Work around https://youtrack.jetbrains.com/issue/IDEA-116898
 # gradle/wrapper/gradle-wrapper.properties
+build/

beanie-core/src/main/java/com/nosto/beanie/AbstractJacksonBeanTest.java

@@ -11,13 +11,13 @@
 package com.nosto.beanie;
 
 import java.io.IOException;
-import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Modifier;
 import java.security.SecureRandom;
 import java.time.Instant;
 import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Random;
 import java.util.function.Supplier;
@@ -61,7 +61,7 @@ public abstract class AbstractJacksonBeanTest<T, U extends T> {
     private final Class<? extends T> deserClass;
     private final Class<? extends U> concreteClass;
 
-    @SuppressWarnings({"JUnitTestCaseWithNonTrivialConstructors"})
+    @SuppressWarnings("JUnitTestCaseWithNonTrivialConstructors")
     public AbstractJacksonBeanTest(Class<? extends U> clazz) {
         this(clazz, clazz);
     }
@@ -115,45 +115,51 @@ public void serde() {
     }
 
     /**
-     * Test that all properties of a bean are named with a consistent naming strategy.
-     * If the bean is configured to use a specific naming strategy, property names should be consistent with that strategy.
+     * Test that all properties of a bean are named with a consistent naming strategy and all property names
+     * comply with the bean's naming strategy.
      */
     @Test
     public void namingStrategy() {
-        BeanDescription description = getDescription();
-        getDescription().findProperties()
-                .forEach(property -> verifyPropertyName(description, property));
-    }
+        BeanDescription beanDescription = getDescription();
+
+        @SuppressWarnings("unchecked")
+        Class<PropertyNamingStrategy> configuredNamingStrategy = Optional.ofNullable(getMapper().getPropertyNamingStrategy())
+                // Required to get around compilation error
+                .map(s -> (Class<PropertyNamingStrategy>) s.getClass())
+                .orElse(PropertyNamingStrategy.class);
 
-    private void verifyPropertyName(BeanDescription bean, BeanPropertyDefinition property) {
-        PropertyNamingStrategy.PropertyNamingStrategyBase namingStrategy = Optional.ofNullable(bean.getClassAnnotations().get(JsonNaming.class))
+        @SuppressWarnings("unchecked")
+        Class<PropertyNamingStrategy> beanPropertyNamingStrategy = Optional.ofNullable(beanDescription.getClassAnnotations().get(JsonNaming.class))
                 .map(JsonNaming::value)
-                .filter(PropertyNamingStrategy.PropertyNamingStrategyBase.class::isAssignableFrom)
-                .map(c -> {
-                    try {
-                        return c.getConstructor().newInstance();
-                    } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
-                        throw new RuntimeException("Cannot construct naming strategy.", e);
-                    }
-                })
-                .map(PropertyNamingStrategy.PropertyNamingStrategyBase.class::cast)
-                .orElseGet(() -> {
-                    // try to detect the naming strategy
-                    String name = property.getName();
-                    if (name.contains("_")) {
-                        return (PropertyNamingStrategy.PropertyNamingStrategyBase) PropertyNamingStrategy.SNAKE_CASE;
+                // Required to get around compilation error
+                .map(s -> (Class<PropertyNamingStrategy>) s)
+                .orElse(configuredNamingStrategy);
+
+        Map<Class<PropertyNamingStrategy>, List<String>> cases = beanDescription.findProperties()
+                .stream()
+                .map(BeanPropertyDefinition::getName)
+                .collect(Collectors.groupingBy(name -> {
+                    if (name.contains("_") && name.toLowerCase().equals(name)) {
+                        // Required to get around compilation error
+                        //noinspection unchecked
+                        return (Class<PropertyNamingStrategy>) PropertyNamingStrategy.SNAKE_CASE.getClass();
+                    } else if (name.toLowerCase().equals(name)) {
+                        // Could be snake case or camel case, so let's assume the class's naming strategy.
+                        return beanPropertyNamingStrategy;
                     } else {
-                        return new PropertyNamingStrategy.PropertyNamingStrategyBase() {
-                            @Override
-                            public String translate(String propertyName) {
-                                return propertyName;
-                            }
-                        };
+                        // Required to get around compilation error
+                        //noinspection unchecked
+                        return (Class<PropertyNamingStrategy>) PropertyNamingStrategy.LOWER_CAMEL_CASE.getClass();
                     }
-                });
+                }));
+        Assert.assertEquals(cases.toString(), 1, cases.size());
+
+        Class<PropertyNamingStrategy> propertyNamingStrategy = cases.keySet()
+                .stream()
+                .findAny()
+                .orElseThrow();
 
-        Assert.assertEquals(String.format("Property %s does not use strategy %s", property.getName(), namingStrategy.getClass()),
-                namingStrategy.translate(property.getName()), property.getName());
+        Assert.assertEquals(beanPropertyNamingStrategy, propertyNamingStrategy);
     }
 
     /**

beanie-core/src/test/java/com/nosto/beanie/ValidCamelCasePropertyNamingStrategyTest.java

@@ -27,11 +27,15 @@ public static class TestBean extends AbstractTestBean {
 
         private final String propertyA;
         private final String propertyB;
+        private final String foo;
 
         @JsonCreator
-        public TestBean(@JsonProperty("propertyA") String propertyA, @JsonProperty("propertyB") String propertyB) {
+        public TestBean(@JsonProperty("propertyA") String propertyA,
+                        @JsonProperty("someProperty") String propertyB,
+                        @JsonProperty("foo") String foo) {
             this.propertyA = propertyA;
             this.propertyB = propertyB;
+            this.foo = foo;
         }
 
         @SuppressWarnings("unused")
@@ -40,8 +44,14 @@ public String getPropertyA() {
         }
 
         @SuppressWarnings("unused")
+        @JsonProperty("someProperty")
         public String getPropertyB() {
             return propertyB;
         }
+
+        @SuppressWarnings("unused")
+        public String getFoo() {
+            return foo;
+        }
     }
 }

beanie-core/src/test/java/com/nosto/beanie/ValidSnakeCasePropertyNamingStrategyTest.java

@@ -30,11 +30,15 @@ public static class TestBean extends AbstractTestBean {
 
         private final String propertyA;
         private final String propertyB;
+        private final String propertyC;
 
         @JsonCreator
-        public TestBean(@JsonProperty("property_a") String propertyA, @JsonProperty("property_b") String propertyB) {
+        public TestBean(@JsonProperty("property_a") String propertyA,
+                        @JsonProperty("some_property") String propertyB,
+                        @JsonProperty("_property_c") String propertyC) {
             this.propertyA = propertyA;
             this.propertyB = propertyB;
+            this.propertyC = propertyC;
         }
 
         @SuppressWarnings("unused")
@@ -43,8 +47,14 @@ public String getPropertyA() {
         }
 
         @SuppressWarnings("unused")
+        @JsonProperty("some_property")
         public String getPropertyB() {
             return propertyB;
         }
+
+        @JsonProperty("_property_c")
+        public String getPropertyC() {
+            return propertyC;
+        }
     }
 }

build.gradle

@@ -34,7 +34,7 @@ allprojects { project ->
     apply plugin: "jacoco"
 
     group 'com.nosto'
-    version '1.0.6'
+    version '1.0.7'
 
     repositories {
         mavenCentral()
@@ -102,6 +102,11 @@ allprojects { project ->
         dependsOn test // tests are required to run before generating the report
     }
 
+    java {
+        sourceCompatibility = JavaVersion.VERSION_1_10
+        targetCompatibility = JavaVersion.VERSION_1_10
+    }
+
     javadoc {
         options.addBooleanOption('html5', true)
     }