Mapping format fixes (#294)

* add test to ensure mapping formats can read/write properly

* fix not baking last entries in TinyV2 reader

* fix broken filter in Enigma File writer

* Fix Enigma Zip writer ZipFS exception on non-matching filesystems

3 files changed, 150 insertions, 10 deletions

diff --git a/enigma/src/main/java/cuchaz/enigma/translation/mapping/serde/enigma/EnigmaMappingsWriter.java b/enigma/src/main/java/cuchaz/enigma/translation/mapping/serde/enigma/EnigmaMappingsWriter.java
index 7570d4b6..378ed984 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/mapping/serde/enigma/EnigmaMappingsWriter.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/mapping/serde/enigma/EnigmaMappingsWriter.java
@@ -55,8 +55,8 @@ public enum EnigmaMappingsWriter implements MappingsWriter {
                 @Override
                 public void write(EntryTree<EntryMapping> mappings, MappingDelta<EntryMapping> delta, Path path, ProgressListener progress, MappingSaveParameters saveParameters) {
                         Collection<ClassEntry> classes = mappings.getRootNodes()
-                                        .filter(entry -> entry instanceof ClassEntry)
-                                        .map(entry -> (ClassEntry) entry)
+                                        .filter(entry -> entry.getEntry() instanceof ClassEntry)
+                                        .map(entry -> (ClassEntry) entry.getEntry())
                                         .collect(Collectors.toList());
 
                         progress.init(classes.size(), I18n.translate("progress.mappings.enigma_file.writing"));
@@ -148,7 +148,7 @@ public enum EnigmaMappingsWriter implements MappingsWriter {
                 private void deleteDeadPackages(Path root, Path packagePath) throws IOException {
                         for (int i = packagePath.getNameCount() - 1; i >= 0; i--) {
                                 Path subPath = packagePath.subpath(0, i + 1);
-                                Path packagePart = root.resolve(subPath);
+                                Path packagePart = root.resolve(subPath.toString());
                                 if (isEmpty(packagePart)) {
                                         Files.deleteIfExists(packagePart);
                                 }
diff --git a/enigma/src/main/java/cuchaz/enigma/translation/mapping/serde/tinyv2/TinyV2Reader.java b/enigma/src/main/java/cuchaz/enigma/translation/mapping/serde/tinyv2/TinyV2Reader.java
index dc255693..dc3246de 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/mapping/serde/tinyv2/TinyV2Reader.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/mapping/serde/tinyv2/TinyV2Reader.java
@@ -71,13 +71,7 @@ public final class TinyV2Reader implements MappingsReader {
                                 for (int i = INDENT_CLEAR_START[indent]; i < STATE_SIZE; i++) {
                                         state.clear(i);
                                         if (holds[i] != null) {
-                                                RawEntryMapping mapping = holds[i].getMapping();
-                                                if (mapping != null) {
-                                                        EntryMapping baked = mapping.bake();
-                                                        if (baked != null) {
-                                                                mappings.insert(holds[i].getEntry(), baked);
-                                                        }
-                                                }
+                                                bakeHeld(mappings, holds[i]);
                                                 holds[i] = null;
                                         }
                                 }
@@ -187,9 +181,26 @@ public final class TinyV2Reader implements MappingsReader {
                         }
                 }
 
+                //bake any remainders
+                for (MappingPair<? extends Entry<?>, RawEntryMapping> hold : holds) {
+                        if (hold != null) {
+                                bakeHeld(mappings, hold);
+                        }
+                }
+
                 return mappings;
         }
 
+        private static void bakeHeld(EntryTree<EntryMapping> mappings, MappingPair<? extends Entry<?>, RawEntryMapping> hold2) {
+                RawEntryMapping mapping = hold2.getMapping();
+                if (mapping != null) {
+                        EntryMapping baked = mapping.bake();
+                        if (baked != null) {
+                                mappings.insert(hold2.getEntry(), baked);
+                        }
+                }
+        }
+
         private void unsupportKey(String[] parts) {
                 throw new IllegalArgumentException("Unsupported key " + parts[0]);
         }
diff --git a/enigma/src/test/java/cuchaz/enigma/translation/mapping/TestReadWriteCycle.java b/enigma/src/test/java/cuchaz/enigma/translation/mapping/TestReadWriteCycle.java
new file mode 100644
index 00000000..4f6654a4
--- /dev/null
+++ b/enigma/src/test/java/cuchaz/enigma/translation/mapping/TestReadWriteCycle.java
@@ -0,0 +1,129 @@
+package cuchaz.enigma.translation.mapping;
+
+import cuchaz.enigma.ProgressListener;
+import cuchaz.enigma.translation.mapping.serde.MappingFileNameFormat;
+import cuchaz.enigma.translation.mapping.serde.MappingFormat;
+import cuchaz.enigma.translation.mapping.serde.MappingParseException;
+import cuchaz.enigma.translation.mapping.serde.MappingSaveParameters;
+import cuchaz.enigma.translation.mapping.tree.EntryTree;
+import cuchaz.enigma.translation.mapping.tree.HashEntryTree;
+import cuchaz.enigma.translation.representation.entry.ClassEntry;
+import cuchaz.enigma.translation.representation.entry.Entry;
+import cuchaz.enigma.translation.representation.entry.FieldEntry;
+import cuchaz.enigma.translation.representation.entry.MethodEntry;
+import cuchaz.enigma.utils.Pair;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * Tests that a MappingFormat can write out a fixed set of mappings and read them back without losing any information.
+ * Javadoc skipped for Tiny (v1) as it doesn't support them.
+ */
+public class TestReadWriteCycle {
+
+        private final MappingSaveParameters parameters = new MappingSaveParameters(MappingFileNameFormat.BY_DEOBF);
+
+        private final Pair<ClassEntry, EntryMapping> testClazz = new Pair<>(
+                        new ClassEntry("a/b/c"),
+                        new EntryMapping("alpha/beta/charlie", "this is a test class")
+        );
+
+        private final Pair<FieldEntry, EntryMapping> testField1 = new Pair<>(
+                        FieldEntry.parse("a/b/c", "field1", "I"),
+                        new EntryMapping("mapped1", "this is field 1")
+        );
+
+        private final Pair<FieldEntry, EntryMapping> testField2 = new Pair<>(
+                        FieldEntry.parse("a/b/c", "field2", "I"),
+                        new EntryMapping("mapped2", "this is field 2")
+        );
+
+        private final Pair<MethodEntry, EntryMapping> testMethod1 = new Pair<>(
+                        MethodEntry.parse("a/b/c", "method1", "()V"),
+                        new EntryMapping("mapped3", "this is method1")
+        );
+
+        private final Pair<MethodEntry, EntryMapping> testMethod2 = new Pair<>(
+                        MethodEntry.parse("a/b/c", "method2", "()V"),
+                        new EntryMapping("mapped4", "this is method 2")
+        );
+
+        private void insertMapping(EntryTree<EntryMapping> mappings, Pair<? extends Entry<?>, EntryMapping> mappingPair){
+                mappings.insert(mappingPair.a, mappingPair.b);
+        }
+
+        private void testReadWriteCycle(MappingFormat mappingFormat, boolean supportsJavadoc, String tmpNameSuffix) throws IOException, MappingParseException {
+                //construct some known mappings to test with
+                EntryTree<EntryMapping> testMappings = new HashEntryTree<>();
+                insertMapping(testMappings, testClazz);
+                insertMapping(testMappings, testField1);
+                insertMapping(testMappings, testField2);
+                insertMapping(testMappings, testMethod1);
+                insertMapping(testMappings, testMethod2);
+
+                Assert.assertTrue("Test mapping insertion failed: testClazz", testMappings.contains(testClazz.a));
+                Assert.assertTrue("Test mapping insertion failed: testField1", testMappings.contains(testField1.a));
+                Assert.assertTrue("Test mapping insertion failed: testField2", testMappings.contains(testField2.a));
+                Assert.assertTrue("Test mapping insertion failed: testMethod1", testMappings.contains(testMethod1.a));
+                Assert.assertTrue("Test mapping insertion failed: testMethod2", testMappings.contains(testMethod2.a));
+
+                File tempFile = File.createTempFile("readWriteCycle", tmpNameSuffix);
+                tempFile.delete();//remove the auto created file
+
+
+                mappingFormat.write(testMappings, tempFile.toPath(), ProgressListener.none(), parameters);
+                Assert.assertTrue("Written file not created", tempFile.exists());
+
+                EntryTree<EntryMapping> loadedMappings = mappingFormat.read(tempFile.toPath(), ProgressListener.none(), parameters);
+
+                Assert.assertTrue("Loaded mappings don't contain testClazz", loadedMappings.contains(testClazz.a));
+                Assert.assertTrue("Loaded mappings don't contain testField1", loadedMappings.contains(testField1.a));
+                Assert.assertTrue("Loaded mappings don't contain testField2", loadedMappings.contains(testField2.a));
+                Assert.assertTrue("Loaded mappings don't contain testMethod1", loadedMappings.contains(testMethod1.a));
+                Assert.assertTrue("Loaded mappings don't contain testMethod2", loadedMappings.contains(testMethod2.a));
+
+                Assert.assertEquals("Incorrect mapping: testClazz", testClazz.b.getTargetName(), loadedMappings.get(testClazz.a).getTargetName());
+                Assert.assertEquals("Incorrect mapping: testField1", testField1.b.getTargetName(), loadedMappings.get(testField1.a).getTargetName());
+                Assert.assertEquals("Incorrect mapping: testField2", testField2.b.getTargetName(), loadedMappings.get(testField2.a).getTargetName());
+                Assert.assertEquals("Incorrect mapping: testMethod1", testMethod1.b.getTargetName(), loadedMappings.get(testMethod1.a).getTargetName());
+                Assert.assertEquals("Incorrect mapping: testMethod2", testMethod2.b.getTargetName(), loadedMappings.get(testMethod2.a).getTargetName());
+
+                if (supportsJavadoc) {
+                        Assert.assertEquals("Incorrect javadoc: testClazz", testClazz.b.getJavadoc(), loadedMappings.get(testClazz.a).getJavadoc());
+                        Assert.assertEquals("Incorrect javadoc: testField1", testField1.b.getJavadoc(), loadedMappings.get(testField1.a).getJavadoc());
+                        Assert.assertEquals("Incorrect javadoc: testField2", testField2.b.getJavadoc(), loadedMappings.get(testField2.a).getJavadoc());
+                        Assert.assertEquals("Incorrect javadoc: testMethod1", testMethod1.b.getJavadoc(), loadedMappings.get(testMethod1.a).getJavadoc());
+                        Assert.assertEquals("Incorrect javadoc: testMethod2", testMethod2.b.getJavadoc(), loadedMappings.get(testMethod2.a).getJavadoc());
+                }
+
+                tempFile.delete();
+        }
+
+        @Test
+        public void testEnigmaFile() throws IOException, MappingParseException {
+                testReadWriteCycle(MappingFormat.ENIGMA_FILE, true, ".enigma");
+        }
+
+        @Test
+        public void testEnigmaDir() throws IOException, MappingParseException {
+                testReadWriteCycle(MappingFormat.ENIGMA_DIRECTORY, true, ".tmp");
+        }
+
+        @Test
+        public void testEnigmaZip() throws IOException, MappingParseException {
+                testReadWriteCycle(MappingFormat.ENIGMA_ZIP, true, ".zip");
+        }
+
+        @Test
+        public void testTinyFile() throws IOException, MappingParseException {
+                testReadWriteCycle(MappingFormat.TINY_FILE, false, ".tiny");
+        }
+
+        @Test
+        public void testTinyV2() throws IOException, MappingParseException {
+                testReadWriteCycle(MappingFormat.TINY_V2, true, ".tinyv2");
+        }
+}