Merge pull request #77 from osmlab/ljdelight/fixRuleList

Support (de)serialization of the RuleList

Author: Michael Cuthbert

gradle/quality.gradle

@@ -24,9 +24,10 @@ sourceSets
 
 test
 {
+    useJUnitPlatform()
     testLogging
     {
-        events "failed"
+        events "passed", "skipped", "failed"
         exceptionFormat = 'full'
     }
 }

src/integrationTest/java/org/maproulette/client/api/ChallengeAPIIntegrationTest.java

@@ -85,6 +85,7 @@ public void updateTest() throws MapRouletteException
                 .customBasemap("customBasemap").build();
 
         final var updatedChallenge = this.getChallengeAPI().update(toUpdateChallenge);
+
         // make sure that it has actually updated.
         this.compareChallenges(toUpdateChallenge, updatedChallenge);
         // now make sure that it is different from the original challenge
@@ -196,7 +197,7 @@ private void compareChallenges(final Challenge challenge1, final Challenge chall
 
     private Challenge getBasicChallenge()
     {
-        return Challenge.builder().parent(1234).name("challengeTest").instruction("TestInstruction")
+        return Challenge.builder().name("challengeTest").instruction("TestInstruction")
                 .description("Testing challenge creation").blurb("Testing challenge creation blurb")
                 .difficulty(ChallengeDifficulty.EXPERT).parent(this.getDefaultProjectIdentifier())
                 .build();

src/main/java/org/maproulette/client/connection/MapRouletteConnection.java

@@ -10,22 +10,21 @@
 import org.maproulette.client.exception.MapRouletteException;
 import org.maproulette.client.exception.MapRouletteRuntimeException;
 import org.maproulette.client.http.ResourceFactory;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import lombok.Getter;
+import lombok.extern.slf4j.Slf4j;
 
 /**
  * The connection class that actually makes the Rest request to the MapRoulette server.
  *
  * @author cuthbertm
  */
+@Slf4j
 public class MapRouletteConnection implements IMapRouletteConnection
 {
     private static final int DEFAULT_CONNECTION_RETRIES = 3;
     private static final int DEFAULT_CONNECTION_WAIT = 5000;
     private static final String KEY_API_KEY = "apiKey";
-    private static final Logger logger = LoggerFactory.getLogger(MapRouletteConnection.class);
     @Getter
     private final MapRouletteConfiguration configuration;
     private final URIBuilder uriBuilder;
@@ -58,17 +57,23 @@ public MapRouletteConnection(final MapRouletteConfiguration configuration)
     @Override
     public Optional<String> execute(final Query query) throws MapRouletteException
     {
+        log.trace("Request: {} {} data={}", query.getMethodName(), query.getUri(), query.getData());
+
         // add authentication to the query
         query.addHeader(KEY_API_KEY, this.configuration.getApiKey());
         return query.execute(this.resourceFactory, this.uriBuilder,
                 throwingFunctionWrapper(resource ->
                 {
                     final var statusCode = resource.getStatusCode();
+                    log.trace("Response code: {} ", statusCode);
+
                     switch (statusCode)
                     {
                         case HttpStatus.SC_OK:
                         case HttpStatus.SC_CREATED:
-                            return resource.getRequestBodyAsString();
+                            final String ret = resource.getRequestBodyAsString();
+                            log.trace("Response body: {}", ret);
+                            return ret;
                         case HttpStatus.SC_NO_CONTENT:
                         case HttpStatus.SC_NOT_FOUND:
                             return "";
@@ -104,7 +109,7 @@ public boolean isAbleToConnectToMapRoulette()
             }
             catch (final Exception e)
             {
-                logger.error(
+                log.error(
                         String.format("Failed to connect to MapRoulette [%s]", this.configuration),
                         e);
                 retries++;

src/main/java/org/maproulette/client/model/Challenge.java

@@ -52,11 +52,16 @@ public class Challenge implements IMapRouletteObject, Serializable
     private String checkinSource = "";
     @NonNull
     private String name;
+
     @Builder.Default
     private ChallengePriority defaultPriority = ChallengePriority.MEDIUM;
-    private RuleList highPriorityRule;
-    private RuleList mediumPriorityRule;
-    private RuleList lowPriorityRule;
+    @Builder.Default
+    private RuleList highPriorityRule = RuleList.builder().build();
+    @Builder.Default
+    private RuleList mediumPriorityRule = RuleList.builder().build();
+    @Builder.Default
+    private RuleList lowPriorityRule = RuleList.builder().build();
+
     @Builder.Default
     private int defaultZoom = DEFAULT_ZOOM;
     @Builder.Default

src/main/java/org/maproulette/client/model/RuleList.java

@@ -2,18 +2,16 @@
 
 import java.io.IOException;
 import java.io.Serializable;
+import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.List;
 
-import org.maproulette.client.exception.MapRouletteRuntimeException;
-import org.maproulette.client.utilities.ObjectMapperSingleton;
-
-import com.fasterxml.jackson.annotation.JsonValue;
+import com.fasterxml.jackson.core.JsonFactory;
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonParser;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
@@ -24,6 +22,8 @@
 import lombok.Builder;
 import lombok.Data;
 import lombok.NoArgsConstructor;
+import lombok.NonNull;
+import lombok.extern.slf4j.Slf4j;
 
 /**
  * @author mcuthbert
@@ -34,32 +34,40 @@
 @AllArgsConstructor
 @JsonDeserialize(using = RuleList.RuleListDeserializer.class)
 @JsonSerialize(using = RuleList.RuleListSerializer.class)
+@Slf4j
 public class RuleList implements Serializable
 {
     private static final String KEY_CONDITION = "condition";
     private static final String KEY_RULES = "rules";
     private static final long serialVersionUID = -1085774480815117637L;
 
-    private String condition;
-    private List<RuleList> ruleList;
-    private List<PriorityRule> rules;
+    @Builder.Default
+    @NonNull
+    private String condition = "";
 
-    public boolean isSet()
-    {
-        return this.condition != null && this.rules != null && !this.rules.isEmpty();
-    }
+    @Builder.Default
+    @NonNull
+    private List<RuleList> ruleList = new ArrayList<>();
 
-    @JsonValue
-    public String toJson()
+    @Builder.Default
+    @NonNull
+    private List<PriorityRule> rules = new ArrayList<>();
+
+    public boolean isSet()
     {
-        try
+        // A condition is needed
+        if (this.condition == null || this.condition.isEmpty())
         {
-            return ObjectMapperSingleton.getMapper().writeValueAsString(this);
+            return false;
         }
-        catch (final JsonProcessingException e)
+        // It is possible to have an empty 'rules' and a non-empty nested rule list.
+        // It is invalid for both to be empty at the same time.
+        if (this.rules.isEmpty() && this.ruleList.isEmpty())
         {
-            throw new MapRouletteRuntimeException(e);
+            return false;
         }
+
+        return true;
     }
 
     /**
@@ -104,8 +112,7 @@ private static void serializeRuleListHelper(final List<RuleList> ruleListList,
             }
         }
 
-        @Override
-        public void serialize(final RuleList value, final JsonGenerator gen,
+        private void serializeRuleListAsObject(final RuleList value, final JsonGenerator gen,
                 final SerializerProvider serializers) throws IOException
         {
             gen.writeStartObject();
@@ -125,11 +132,46 @@ public void serialize(final RuleList value, final JsonGenerator gen,
             }
             gen.writeEndArray();
             gen.writeEndObject();
+            gen.flush();
+        }
+
+        /**
+         * Serialize a RuleList in a format that is compatible with the existing scala backend
+         * service. The end format must be a json escaped string and not an object. The deserializer
+         * supports either format: a json string or an object. <br>
+         * <br>
+         * For example:
+         * "highPriorityRule":"{\"condition\":\"AND\",\"rules\":[{\"value\":\"priority_pd.3\",\"type\":\"string\",\"operator\":\"equal\"}]}"
+         * {@inheritDoc}
+         */
+        @Override
+        public void serialize(final RuleList value, final JsonGenerator gen,
+                final SerializerProvider serializers) throws IOException
+        {
+            // If the RuleList is not "set", write an empty object string.
+            if (!value.isSet())
+            {
+                gen.writeString("{}");
+                return;
+            }
+
+            // First create a temporary jsongenerator to hold the serialized nested RuleList object.
+            final StringWriter stringWriter = new StringWriter();
+            final JsonGenerator tempGen = new JsonFactory().setCodec(gen.getCodec())
+                    .createGenerator(stringWriter);
+            serializeRuleListAsObject(value, tempGen, serializers);
+
+            // Now get the serialized RuleList as a string and write it as a plain string.
+            final String ruleListAsString = stringWriter.toString();
+            gen.writeString(ruleListAsString);
+            stringWriter.close();
+            tempGen.close();
         }
     }
 
     /**
-     * Deserialize a {@code RuleList}
+     * Deserialize a {@code RuleList}. The serialized format may be either a json escaped string or
+     * an object.
      */
     public static class RuleListDeserializer extends StdDeserializer<RuleList>
     {
@@ -146,9 +188,11 @@ public RuleListDeserializer(final Class<?> valueClass)
         private static RuleList buildRuleListHelper(final JsonNode node,
                 final DeserializationContext ctxt)
         {
+            // When the serialized format lacks required values, just create an empty RuleList to
+            // avoid NPE.
             if (node.get("condition") == null)
             {
-                return null;
+                return RuleList.builder().build();
             }
             final RuleList ret = RuleList.builder().condition(node.get("condition").asText())
                     .ruleList(new ArrayList<>()).rules(new ArrayList<>()).build();
@@ -175,12 +219,40 @@ private static RuleList buildRuleListHelper(final JsonNode node,
             return ret;
         }
 
+        /**
+         * Deserialize the escaped json string or object into a RuleList. <br>
+         * <br>
+         * For example the escaped json string looks like this
+         * "{\"condition\":\"AND\",\"rules\":[{\"value\":\"priority_pd.3\",\"type\":\"string\",\"operator\":\"equal\"}]}"
+         * or like this
+         * {"condition":"AND","rules":[{"value":"priority_pd.3","type":"string","operator":"equal"}]}
+         * {@inheritDoc}
+         */
         @Override
         public RuleList deserialize(final JsonParser jsonParser, final DeserializationContext ctxt)
                 throws IOException
         {
-            final JsonNode node = jsonParser.getCodec().readTree(jsonParser);
-            return buildRuleListHelper(node, ctxt);
+            // The json could be an escaped string representation or an object representation of a
+            // RuleList
+            final JsonNode tree = jsonParser.readValueAsTree();
+
+            if (tree.isContainerNode())
+            {
+                // It's an object representation, pass it on for parsing
+                return buildRuleListHelper(tree, ctxt);
+            }
+            else
+            {
+                // First read out the string which is the escaped json of the RuleList
+                final String ruleListString = jsonParser.getCodec().treeToValue(tree, String.class);
+
+                // Take the string and convert it to a JsonNode
+                final JsonNode ruleListNode = ((ObjectMapper) jsonParser.getCodec())
+                        .readTree(ruleListString);
+
+                // Recursively parse the node tree
+                return buildRuleListHelper(ruleListNode, ctxt);
+            }
         }
     }
 }

src/test/java/org/maproulette/client/connection/MapRouletteConnectionTest.java

@@ -9,6 +9,7 @@
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.maproulette.client.api.QueryConstants;
 import org.maproulette.client.exception.MapRouletteException;
@@ -19,6 +20,7 @@
 /**
  * @author mcuthbert
  */
+@Disabled
 public class MapRouletteConnectionTest
 {
     @Test

src/test/java/org/maproulette/client/model/ChallengeTest.java

@@ -27,9 +27,9 @@ public void challengeBuilderDefaultTest()
         Assertions.assertNull(challenge.getDescription());
         Assertions.assertEquals("", challenge.getCheckinComment());
         Assertions.assertEquals("", challenge.getCheckinSource());
-        Assertions.assertNull(challenge.getHighPriorityRule());
-        Assertions.assertNull(challenge.getMediumPriorityRule());
-        Assertions.assertNull(challenge.getLowPriorityRule());
+        Assertions.assertFalse(challenge.getHighPriorityRule().isSet());
+        Assertions.assertFalse(challenge.getMediumPriorityRule().isSet());
+        Assertions.assertFalse(challenge.getLowPriorityRule().isSet());
         Assertions.assertEquals(13, challenge.getDefaultZoom());
         Assertions.assertEquals(1, challenge.getMinZoom());
         Assertions.assertEquals(19, challenge.getMaxZoom());