MapRouletteUploadCommand: Handle gzipped files (osmlab#138)

* Handle gzipped files cleanly

* spotlessapply and factor out a constant

* Address comments

* Add unit tests

* Remove hardcoded files

src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteCommand.java

@@ -2,6 +2,7 @@
 
 import java.io.UnsupportedEncodingException;
 import java.net.URISyntaxException;
+import java.util.function.Function;
 
 import org.openstreetmap.atlas.checks.maproulette.data.Challenge;
 import org.openstreetmap.atlas.checks.maproulette.data.ChallengeDifficulty;
@@ -78,20 +79,24 @@ protected MapRouletteClient getMapRouletteClient()
     @Override
     protected int onRun(final CommandMap commandMap)
     {
-        final MapRouletteConfiguration mapRoulette = this.fromCommandMap(commandMap);
+        return this.onRun(commandMap, MapRouletteClient::new);
+    }
 
+    protected int onRun(final CommandMap commandMap,
+            final Function<MapRouletteConfiguration, MapRouletteClient> clientConstructor)
+    {
+        final MapRouletteConfiguration mapRoulette = this.fromCommandMap(commandMap);
         if (mapRoulette != null)
         {
             try
             {
-                this.mapRouletteClient = new MapRouletteClient(mapRoulette);
+                this.mapRouletteClient = clientConstructor.apply(mapRoulette);
             }
             catch (final IllegalArgumentException e)
             {
                 logger.warn("Failed to initialize the MapRouletteClient", e);
             }
         }
-
         execute(commandMap, mapRoulette);
         return 0;
     }

src/main/java/org/openstreetmap/atlas/checks/commands/MapRouletteUploadCommand.java → src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommand.java

@@ -1,18 +1,20 @@
-package org.openstreetmap.atlas.checks.commands;
+package org.openstreetmap.atlas.checks.maproulette;
 
 import java.io.BufferedReader;
+import java.io.FileInputStream;
 import java.io.FileReader;
 import java.io.IOException;
+import java.io.InputStreamReader;
 import java.io.UnsupportedEncodingException;
 import java.net.URISyntaxException;
 import java.text.MessageFormat;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
+import java.util.zip.GZIPInputStream;
 
 import org.apache.commons.io.FilenameUtils;
-import org.openstreetmap.atlas.checks.maproulette.MapRouletteCommand;
-import org.openstreetmap.atlas.checks.maproulette.MapRouletteConfiguration;
 import org.openstreetmap.atlas.checks.maproulette.data.Challenge;
 import org.openstreetmap.atlas.checks.maproulette.data.Task;
 import org.openstreetmap.atlas.checks.maproulette.serializer.ChallengeDeserializer;
@@ -42,6 +44,7 @@ public class MapRouletteUploadCommand extends MapRouletteCommand
             Optionality.OPTIONAL, "config/configuration.json");
     private static final String PARAMETER_CHALLENGE = "challenge";
     private static final String LOG_EXTENSION = "log";
+    private static final String ZIPPED_LOG_EXTENSION = ".log.gz";
     private static final Logger logger = LoggerFactory.getLogger(MapRouletteUploadCommand.class);
     private final Map<String, Challenge> checkNameChallengeMap;
 
@@ -60,9 +63,11 @@ protected void execute(final CommandMap commandMap,
         final Configuration instructions = this.loadConfiguration(commandMap);
         ((File) commandMap.get(INPUT_DIRECTORY)).listFilesRecursively().forEach(logFile ->
         {
-            if (FilenameUtils.getExtension(logFile.getName()).equals(LOG_EXTENSION))
+            // If this file is something we handle, read and upload the tasks contained within
+            final Optional<OutputFileType> optionalHandledFileType = getOptionalOutputType(logFile);
+            optionalHandledFileType.ifPresent(outputFileType ->
             {
-                try (BufferedReader reader = new BufferedReader(new FileReader(logFile.getPath())))
+                try (BufferedReader reader = this.getReader(logFile, outputFileType))
                 {
                     reader.lines().forEach(line ->
                     {
@@ -81,14 +86,68 @@ protected void execute(final CommandMap commandMap,
                 }
                 catch (final IOException error)
                 {
-                    logger.error(
-                            MessageFormat.format("Error while reading {0}: ", logFile.getName()),
-                            error);
+                    logger.error("Error while reading {}:", logFile, error);
                 }
-            }
+            });
         });
     }
 
+    /**
+     * An enum containing the different types of input files that we can handle.
+     */
+    private enum OutputFileType
+    {
+        LOG,
+        COMPRESSED_LOG
+    }
+
+    /**
+     * Determine whether or not this file is something we can handle, and classify it accordingly.
+     * 
+     * @param logFile
+     *            any file
+     * @return if this file is something this command can handle, the appropriate OutputFileType
+     *         enum value; otherwise, an empty optional.
+     */
+    private Optional<OutputFileType> getOptionalOutputType(final File logFile)
+    {
+        // Note that technically the true extension is just .gz, so we can't use the same method as
+        // below.
+        if (logFile.getName().endsWith(ZIPPED_LOG_EXTENSION))
+        {
+            return Optional.of(OutputFileType.COMPRESSED_LOG);
+        }
+        else if (FilenameUtils.getExtension(logFile.getName()).equals(LOG_EXTENSION))
+        {
+            return Optional.of(OutputFileType.LOG);
+        }
+        return Optional.empty();
+    }
+
+    /**
+     * Read a file that we know we should be able to handle
+     * 
+     * @param inputFile
+     *            Some file with a valid, appropriate extension.
+     * @param fileType
+     *            The type of file that inputFile is
+     * @return a BufferedReader to read inputFile
+     * @throws IOException
+     *             if the file is not found or is poorly formatted, given its extension. For
+     *             example, if this file is gzipped and something goes wrong in the unzipping
+     *             process, it might throw an error
+     */
+    private BufferedReader getReader(final File inputFile, final OutputFileType fileType)
+            throws IOException
+    {
+        if (fileType == OutputFileType.LOG)
+        {
+            return new BufferedReader(new FileReader(inputFile.getPath()));
+        }
+        return new BufferedReader(new InputStreamReader(
+                new GZIPInputStream(new FileInputStream(inputFile.getPath()))));
+    }
+
     /**
      * Given a command map, load the configuration defined by the CONFIG_LOCATION switch and return
      * it

src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTest.java

@@ -0,0 +1,83 @@
+package org.openstreetmap.atlas.checks.maproulette;
+
+import java.util.Collections;
+import java.util.Set;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.openstreetmap.atlas.checks.event.CheckFlagEvent;
+import org.openstreetmap.atlas.checks.event.CheckFlagFileProcessor;
+import org.openstreetmap.atlas.checks.event.FileProcessor;
+import org.openstreetmap.atlas.checks.event.ShutdownEvent;
+import org.openstreetmap.atlas.checks.maproulette.data.Challenge;
+import org.openstreetmap.atlas.checks.maproulette.data.Project;
+import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper;
+import org.openstreetmap.atlas.streaming.resource.File;
+import org.openstreetmap.atlas.utilities.runtime.CommandMap;
+
+/**
+ * Unit tests for MapRouletteUploadCommand.
+ *
+ * @author nachtm
+ */
+public class MapRouletteUploadCommandTest
+{
+    private static final String MAPROULETTE_CONFIG = "-maproulette=host:2222:project:api";
+    private static final File FOLDER = File.temporaryFolder();
+
+    @Rule
+    public final MapRouletteUploadCommandTestRule setup = new MapRouletteUploadCommandTestRule();
+
+    @Before
+    public void writeFiles()
+    {
+        // Create an unzipped file
+        final FileProcessor<CheckFlagEvent> unzippedProcessor = new CheckFlagFileProcessor(
+                new SparkFileHelper(Collections.emptyMap()),
+                FOLDER.child("unzipped.log").toString()).withCompression(false);
+        unzippedProcessor.process(setup.getOneBasicFlag());
+        unzippedProcessor.process(new ShutdownEvent());
+
+        // Create a zipped file
+        final FileProcessor<CheckFlagEvent> zippedProcessor = new CheckFlagFileProcessor(
+                new SparkFileHelper(Collections.emptyMap()),
+                FOLDER.child("zipped.log.gz").toString()).withCompression(true);
+        zippedProcessor.process(setup.getAnotherBasicFlag());
+        zippedProcessor.process(new ShutdownEvent());
+    }
+
+    @After
+    public void cleanup()
+    {
+        new File(FOLDER.getPath()).deleteRecursively();
+    }
+
+    @Test
+    public void testExecute()
+    {
+        // Set up some arguments
+        final MapRouletteCommand command = new MapRouletteUploadCommand();
+        final String[] arguments = { String.format("-logfiles=%s", FOLDER.getPath()),
+                MAPROULETTE_CONFIG };
+        final CommandMap map = command.getCommandMap(arguments);
+        final TestMapRouletteConnection connection = new TestMapRouletteConnection();
+
+        // Run the command
+        command.onRun(map, configuration -> new MapRouletteClient(configuration, connection));
+
+        // Test the results
+        final Set<Project> projects = connection.uploadedProjects();
+        Assert.assertEquals(1, projects.size());
+        projects.forEach(project ->
+        {
+            Assert.assertEquals("project", project.getName());
+            final Set<Challenge> challenges = connection.challengesForProject(project);
+            Assert.assertEquals(1, challenges.size());
+            challenges.forEach(challenge -> Assert.assertEquals(2,
+                    connection.tasksForChallenge(challenge).size()));
+        });
+    }
+}

src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTestRule.java

@@ -0,0 +1,45 @@
+package org.openstreetmap.atlas.checks.maproulette;
+
+import org.openstreetmap.atlas.checks.event.CheckFlagEvent;
+import org.openstreetmap.atlas.checks.flag.CheckFlag;
+import org.openstreetmap.atlas.geography.atlas.Atlas;
+import org.openstreetmap.atlas.utilities.testing.CoreTestRule;
+import org.openstreetmap.atlas.utilities.testing.TestAtlas;
+import org.openstreetmap.atlas.utilities.testing.TestAtlas.Loc;
+import org.openstreetmap.atlas.utilities.testing.TestAtlas.Point;
+
+/**
+ * Data for unit tests for MapRouletteUploadCommand
+ *
+ * @author nachtm
+ */
+public class MapRouletteUploadCommandTestRule extends CoreTestRule
+{
+    private static final String CENTER = "0,0";
+    private static final String IDENTIFIER_1 = "1";
+    private static final String IDENTIFIER_2 = "2";
+    private static final String CHALLENGE = "SomeCheck";
+    private static final String INSTRUCTIONS = "Instructions.";
+
+    @TestAtlas(points = { @Point(coordinates = @Loc(value = CENTER), id = "1") })
+    private Atlas basicAtlas;
+
+    private CheckFlagEvent getBasicFlag(final String identifier)
+    {
+        final CheckFlag flag = new CheckFlag(identifier);
+        flag.addObject(this.basicAtlas.point(1L));
+        flag.addInstruction(INSTRUCTIONS);
+
+        return new CheckFlagEvent(CHALLENGE, flag);
+    }
+
+    public CheckFlagEvent getOneBasicFlag()
+    {
+        return this.getBasicFlag(IDENTIFIER_1);
+    }
+
+    public CheckFlagEvent getAnotherBasicFlag()
+    {
+        return this.getBasicFlag(IDENTIFIER_2);
+    }
+}

src/test/java/org/openstreetmap/atlas/checks/maproulette/TestMapRouletteConnection.java

@@ -44,7 +44,7 @@ public boolean uploadBatchTasks(final long challengeId, final Set<Task> tasks)
         }
         else
         {
-            this.challengeToTasks.put(challengeId, tasks);
+            this.challengeToTasks.put(challengeId, new HashSet<>(tasks));
         }
         return true;
     }
@@ -93,6 +93,6 @@ public Set<Challenge> challengesForProject(final Project project)
 
     public Set<Task> tasksForChallenge(final Challenge challenge)
     {
-        return this.challengeToTasks.get(challenge);
+        return this.challengeToTasks.get(challenge.getId());
     }
 }