Merge "Properly handle removed res items when writing incremental data blob."jb-mr1.1-dev-plus-aosp

8 files changed, 100 insertions, 3 deletions

diff --git a/sdk-common/src/main/java/com/android/ide/common/res2/DataFile.java b/sdk-common/src/main/java/com/android/ide/common/res2/DataFile.java
index 85fcac8f05..29f871574b 100644
--- a/sdk-common/src/main/java/com/android/ide/common/res2/DataFile.java
+++ b/sdk-common/src/main/java/com/android/ide/common/res2/DataFile.java
@@ -91,6 +91,16 @@ public abstract class DataFile<I extends DataItem> {
         return mItems.values().iterator().next();
     }
 
+    boolean hasNotRemovedItems() {
+        for (I item : mItems.values()) {
+            if (!item.isRemoved()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     @NonNull
     public Collection<I> getItems() {
         return mItems.values();
diff --git a/sdk-common/src/main/java/com/android/ide/common/res2/DataMerger.java b/sdk-common/src/main/java/com/android/ide/common/res2/DataMerger.java
index 9259682752..05a52fa22f 100755
--- a/sdk-common/src/main/java/com/android/ide/common/res2/DataMerger.java
+++ b/sdk-common/src/main/java/com/android/ide/common/res2/DataMerger.java
@@ -317,9 +317,10 @@ abstract class DataMerger<I extends DataItem<F>, F extends DataFile<I>, S extend
      * @return true if the blob was loaded.
      * @throws IOException
      *
-     * @see #writeBlobTo(File)
+     * @see #writeBlobTo(File, MergeConsumer)
      */
-    public boolean loadFromBlob(File blobRootFolder, boolean incrementalState) throws IOException {
+    public boolean loadFromBlob(@NonNull File blobRootFolder, boolean incrementalState)
+            throws IOException {
         File file = new File(blobRootFolder, FN_MERGER_XML);
         if (!file.isFile()) {
             return false;
@@ -382,6 +383,13 @@ abstract class DataMerger<I extends DataItem<F>, F extends DataFile<I>, S extend
         }
     }
 
+    public void cleanBlob(@NonNull File blobRootFolder) {
+        File file = new File(blobRootFolder, FN_MERGER_XML);
+        if (file.isFile()) {
+            file.delete();
+        }
+    }
+
     /**
      * Sets the post blob load state to WRITTEN.
      *
diff --git a/sdk-common/src/main/java/com/android/ide/common/res2/DataSet.java b/sdk-common/src/main/java/com/android/ide/common/res2/DataSet.java
index d4b323a02d..63f704f751 100755
--- a/sdk-common/src/main/java/com/android/ide/common/res2/DataSet.java
+++ b/sdk-common/src/main/java/com/android/ide/common/res2/DataSet.java
@@ -266,6 +266,10 @@ abstract class DataSet<I extends DataItem<F>, F extends DataFile<I>> implements
             Collection<F> dataFiles = mSourceFileToDataFilesMap.get(sourceFile);
 
             for (F dataFile : dataFiles) {
+                if (!dataFile.hasNotRemovedItems()) {
+                    continue;
+                }
+
                 // the node for the file and its path and qualifiers attribute
                 Node fileNode = document.createElement(NODE_FILE);
                 sourceNode.appendChild(fileNode);
@@ -275,7 +279,7 @@ abstract class DataSet<I extends DataItem<F>, F extends DataFile<I>> implements
 
                 if (dataFile.getType() == DataFile.FileType.MULTI) {
                     for (I item : dataFile.getItems()) {
-                        if (consumer.ignoreItemInMerge(item)) {
+                        if (item.isRemoved()|| consumer.ignoreItemInMerge(item)) {
                             continue;
                         }
                         Node adoptedNode = item.getAdoptedNode(document);
@@ -284,6 +288,8 @@ abstract class DataSet<I extends DataItem<F>, F extends DataFile<I>> implements
                         }
                     }
                 } else {
+                    // no need to check for isRemoved here since it's checked
+                    // at the file level and there's only one item.
                     I dataItem = dataFile.getItem();
                     NodeUtils.addAttribute(document, fileNode, null, ATTR_NAME, dataItem.getName());
                     dataItem.addExtraAttributes(document, fileNode, null);
diff --git a/sdk-common/src/test/java/com/android/ide/common/res2/ResourceMergerTest.java b/sdk-common/src/test/java/com/android/ide/common/res2/ResourceMergerTest.java
index 3a7ff38028..eaff62e36f 100755
--- a/sdk-common/src/test/java/com/android/ide/common/res2/ResourceMergerTest.java
+++ b/sdk-common/src/test/java/com/android/ide/common/res2/ResourceMergerTest.java
@@ -812,6 +812,70 @@ public class ResourceMergerTest extends BaseTestCase {
         assertEquals(FileValidity.FileStatus.IGNORED_FILE, fileValidity.status);
     }
 
+    public void testIncDataForRemovedFile() throws Exception {
+        File root = TestUtils.getCanonicalRoot("resources", "removedFile");
+        File fakeBlobRoot = getMergedBlobFolder(root);
+
+        ResourceMerger resourceMerger = new ResourceMerger();
+        resourceMerger.loadFromBlob(fakeBlobRoot, true /*incrementalState*/);
+        checkSourceFolders(resourceMerger);
+
+        List<ResourceSet> sets = resourceMerger.getDataSets();
+        assertEquals(1, sets.size());
+
+        RecordingLogger logger = new RecordingLogger();
+
+        // ----------------
+        // Load the main set
+        ResourceSet mainSet = sets.get(0);
+        File resBase = new File(root, "res");
+        File resDrawable = new File(resBase, ResourceFolderType.DRAWABLE.getName());
+
+        // removed file
+        File resIconRemoved = new File(resDrawable, "removed.png");
+        mainSet.updateWith(resBase, resIconRemoved, FileStatus.REMOVED, logger);
+        checkLogger(logger);
+
+        // validate for duplicates
+        resourceMerger.validateDataSets();
+
+        // check the content.
+        ListMultimap<String, ResourceItem> mergedMap = resourceMerger.getDataMap();
+
+        // check layout/main is unchanged
+        List<ResourceItem> removedIcon = mergedMap.get("drawable/removed");
+        assertEquals(1, removedIcon.size());
+        assertTrue(removedIcon.get(0).isRemoved());
+        assertTrue(removedIcon.get(0).isWritten());
+        assertFalse(removedIcon.get(0).isTouched());
+
+        // write and check the result of writeResourceFolder
+        // copy the current resOut which serves as pre incremental update state.
+        File outFolder = getFolderCopy(new File(root, "out"));
+
+        // write the content of the resource merger.
+        MergedResourceWriter writer = new MergedResourceWriter(outFolder, null /*aaptRunner*/);
+        resourceMerger.mergeData(writer, false /*doCleanUp*/);
+
+        File outDrawableFolder = new File(outFolder, ResourceFolderType.DRAWABLE.getName());
+
+        // check the files are correct
+        assertFalse(new File(outDrawableFolder, "removed.png").isFile());
+        assertTrue(new File(outDrawableFolder, "icon.png").isFile());
+
+        // now write the blob
+        File outBlobFolder = Files.createTempDir();
+        resourceMerger.writeBlobTo(outBlobFolder, writer);
+
+        // check the removed icon is not present.
+        ResourceMerger resourceMerger2 = new ResourceMerger();
+        resourceMerger2.loadFromBlob(outBlobFolder, true /*incrementalState*/);
+
+        mergedMap = resourceMerger2.getDataMap();
+        removedIcon = mergedMap.get("drawable/removed");
+        assertTrue(removedIcon.isEmpty());
+    }
+
     /**
      * Creates a fake merge with given sets.
      *
diff --git a/sdk-common/src/test/resources/testData/resources/removedFile/merger.xml b/sdk-common/src/test/resources/testData/resources/removedFile/merger.xml
new file mode 100644
index 0000000000..ce93563e00
--- /dev/null
+++ b/sdk-common/src/test/resources/testData/resources/removedFile/merger.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<merger xmlns:ns1="urn:oasis:names:tc:xliff:document:1.2">
+    <dataSet config="main">
+        <source path="$TOP$$SEP$res">
+            <file name="icon" path="$TOP$$SEP$res$SEP$drawable$SEP$icon.png" qualifiers="" type="drawable"/>
+            <file name="removed" path="$TOP$$SEP$res$SEP$drawable$SEP$removed.png" qualifiers="" type="drawable"/>
+        </source>
+    </dataSet>
+</merger>
diff --git a/sdk-common/src/test/resources/testData/resources/removedFile/out/drawable/icon.png b/sdk-common/src/test/resources/testData/resources/removedFile/out/drawable/icon.png
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/sdk-common/src/test/resources/testData/resources/removedFile/out/drawable/icon.png
diff --git a/sdk-common/src/test/resources/testData/resources/removedFile/out/drawable/removed.png b/sdk-common/src/test/resources/testData/resources/removedFile/out/drawable/removed.png
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/sdk-common/src/test/resources/testData/resources/removedFile/out/drawable/removed.png
diff --git a/sdk-common/src/test/resources/testData/resources/removedFile/res/drawable/icon.png b/sdk-common/src/test/resources/testData/resources/removedFile/res/drawable/icon.png
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/sdk-common/src/test/resources/testData/resources/removedFile/res/drawable/icon.png