Address Sonar critical warnings

- End this switch case with an unconditional break, continue, return or throw statement - Use hasVersionLabel() instead of catching exception for version label check Resolves: https://www.pivotaltracker.com/story/show/70584818
fcrepo · May 7, 2014 · fb7e86c · fb7e86c
1 parent a2b4ba4
commit fb7e86c
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 2 deletions.
diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/DefaultFilter.java b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/observer/DefaultFilter.java
@@ -88,6 +88,7 @@ public boolean apply(final Event event) {
                     if (isFedoraObject.apply(n) || isFedoraDatastream.apply(n)) {
                         return true;
                     }
+                    break;
             }
         } catch (final PathNotFoundException e) {
             LOGGER.trace("Dropping event from outside our assigned workspace:\n", e);

diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/VersionServiceImpl.java b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/services/VersionServiceImpl.java
@@ -150,12 +150,16 @@ private Version getVersionForLabel(final Workspace workspace, final String absPa
                                        final String label) throws RepositoryException {
         // first see if there's a version label
         final VersionHistory history = workspace.getVersionManager().getVersionHistory(absPath);
-        try {
+
+        if (history.hasVersionLabel(label)) {
             return history.getVersionByLabel(label);
-        } catch (VersionException ex) {
+        } else {
             // there was no version with the given JCR Version Label, check to see if
             // there's a version whose UUID is equal to the label
             VersionIterator versionIt = history.getAllVersions();
+            if (versionIt == null) {
+                return null;
+            }
             while (versionIt.hasNext()) {
                 Version v = versionIt.nextVersion();
                 if (v.getFrozenNode().getIdentifier().equals(label)) {

diff --git a/fcrepo-kernel/src/test/java/org/fcrepo/kernel/services/VersionServiceImplTest.java b/fcrepo-kernel/src/test/java/org/fcrepo/kernel/services/VersionServiceImplTest.java
@@ -224,6 +224,7 @@ public void testRevertToVersionByLabel() throws RepositoryException {
         VersionManager mockVersionManager = mock(VersionManager.class);
         VersionHistory mockHistory = mock(VersionHistory.class);
         Version mockVersion1 = mock(Version.class);
+        when(mockHistory.hasVersionLabel(versionLabel)).thenReturn(true);
         when(mockHistory.getVersionByLabel(versionLabel)).thenReturn(mockVersion1);
         when(mockWorkspace.getVersionManager()).thenReturn(mockVersionManager);
         when(mockVersionManager.getVersionHistory("/example-versioned")).thenReturn(mockHistory);
@@ -277,6 +278,7 @@ public void testRevertToVersionByLabelWithAutoVersioning() throws RepositoryExce
         VersionManager mockVersionManager = mock(VersionManager.class);
         VersionHistory mockHistory = mock(VersionHistory.class);
         Version mockVersion1 = mock(Version.class);
+        when(mockHistory.hasVersionLabel(versionLabel)).thenReturn(true);
         when(mockHistory.getVersionByLabel(versionLabel)).thenReturn(mockVersion1);
         when(mockWorkspace.getVersionManager()).thenReturn(mockVersionManager);
         when(mockVersionManager.getVersionHistory("/example-auto-versioned")).thenReturn(mockHistory);