Merge pull request mongeez#32 from exell-christopher/master

Fix NPE issues in xml file parsing
mabegon · Oct 25, 2013 · 660c137 · 660c137
2 parents 6bf91e0 + ea7e75e
commit 660c137
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 13 deletions.
diff --git a/src/main/java/org/mongeez/commands/ChangeSetList.java b/src/main/java/org/mongeez/commands/ChangeSetList.java
@@ -19,6 +19,9 @@ public class ChangeSetList {
     List<ChangeSet> list = null;
 
     public List<ChangeSet> getList() {
+        if (list == null) {
+            list = new ArrayList<ChangeSet>();
+        }
         return list;
     }
 
@@ -27,9 +30,6 @@ public void setList(List<ChangeSet> list) {
     }
 
     public void add(ChangeSet changeSet) {
-        if (list == null) {
-            list = new ArrayList<ChangeSet>();
-        }
-        this.list.add(changeSet);
+        getList().add(changeSet);
     }
 }
diff --git a/src/main/java/org/mongeez/reader/FilesetXMLReader.java b/src/main/java/org/mongeez/reader/FilesetXMLReader.java
@@ -41,11 +41,17 @@ public List<Resource> getFiles(Resource file) {
             digester.addSetProperties("changeFiles/file");
             digester.addSetNext("changeFiles/file", "add");
 
+            logger.info("Parsing XML Fileset file {}", file.getFilename());
             ChangeFileSet changeFileSet = (ChangeFileSet) digester.parse(file.getInputStream());
-            logger.info("Num of changefiles " + changeFileSet.getChangeFiles().size());
-
-            for (ChangeFile changeFile : changeFileSet.getChangeFiles()) {
-                files.add(file.createRelative(changeFile.getPath()));
+            if (changeFileSet != null) {
+                logger.info("Num of changefiles found " + changeFileSet.getChangeFiles().size());
+                for (ChangeFile changeFile : changeFileSet.getChangeFiles()) {
+                    files.add(file.createRelative(changeFile.getPath()));
+                }
+            }
+            else {
+                logger.error("The file {} doesn't seem to contain a changeFiles declaration. Are you "
+                        + "using the correct file to initialize Mongeez?", file.getFilename());
             }
         } catch (IOException e) {
             logger.error("IOException", e);

diff --git a/src/main/java/org/mongeez/reader/XmlChangeSetReader.java b/src/main/java/org/mongeez/reader/XmlChangeSetReader.java
@@ -51,21 +51,26 @@ public boolean supports(Resource file) {
     }
 
     @Override
-	public List<ChangeSet> getChangeSets(Resource file) {
+    public List<ChangeSet> getChangeSets(Resource file) {
         List<ChangeSet> changeSets = new ArrayList<ChangeSet>();
 
         try {
+            logger.info("Parsing XML Change Set File {}", file.getFilename());
             ChangeSetList changeFileSet = (ChangeSetList) digester.parse(file.getInputStream());
-            for (ChangeSet changeSet : changeFileSet.getList()) {
-                ChangeSetReaderUtil.populateChangeSetResourceInfo(changeSet, file);
+            if (changeFileSet == null) {
+                logger.warn("Ignoring change file {}, the parser returned null. Please check your formatting.", file.getFilename());
+            }
+            else {
+                for (ChangeSet changeSet : changeFileSet.getList()) {
+                    ChangeSetReaderUtil.populateChangeSetResourceInfo(changeSet, file);
+                }
+                changeSets.addAll(changeFileSet.getList());
             }
-            changeSets.addAll(changeFileSet.getList());
         } catch (IOException e) {
             logger.error("IOException", e);
         } catch (org.xml.sax.SAXException e) {
             logger.error("SAXException", e);
         }
-
         return changeSets;
     }
 }
diff --git a/src/test/java/org/mongeez/MongeezTest.java b/src/test/java/org/mongeez/MongeezTest.java
@@ -84,4 +84,23 @@ public void testNoFiles() throws Exception {
 
         assertEquals(db.getCollection("mongeez").count(), 1);
     }
+
+    @Test(groups = "dao")
+    public void testNoFailureOnEmptyChangeLog() throws Exception {
+        assertEquals(db.getCollection("mongeez").count(), 0);
+
+        Mongeez mongeez = create("mongeez_empty_changelog.xml");
+        mongeez.process();
+
+        assertEquals(db.getCollection("mongeez").count(), 1);
+    }
+
+    @Test(groups = "dao")
+    public void testNoFailureOnNoChangeFilesBlock() throws Exception {
+        assertEquals(db.getCollection("mongeez").count(), 0);
+
+        Mongeez mongeez = create("mongeez_no_changefiles_declared.xml");
+        mongeez.process();
+        assertEquals(db.getCollection("mongeez").count(), 1);
+    }
 }
diff --git a/src/test/resources/empty_changeset.xml b/src/test/resources/empty_changeset.xml
@@ -0,0 +1,4 @@
+<!--A changelog with no changes. This can occur on first setup of new projects-->
+<mongoChangeLog>
+</mongoChangeLog> 
+
diff --git a/src/test/resources/mongeez_empty_changelog.xml b/src/test/resources/mongeez_empty_changelog.xml
@@ -0,0 +1,6 @@
+<!--
+A mongeez setup with a change file that has no changes in the changelog
+-->
+<changeFiles>
+  <file path="empty_changeset.xml"/>
+</changeFiles>
diff --git a/src/test/resources/mongeez_no_changefiles_declared.xml b/src/test/resources/mongeez_no_changefiles_declared.xml
@@ -0,0 +1,5 @@
+<!--
+A mongeez setup with valid xml that does not contain changeFiles root element. Not normal, but causes an NPE
+-->
+<test>
+</test>