Closes #20: Clean error message if document or file does not exist

sismics · Aug 26, 2015 · 86cae53 · 86cae53
1 parent 5cbdb5d
commit 86cae53
Show file tree

Hide file tree

Showing 9 changed files with 50 additions and 20 deletions.
diff --git a/docs-web/src/main/java/com/sismics/docs/rest/resource/AuditLogResource.java b/docs-web/src/main/java/com/sismics/docs/rest/resource/AuditLogResource.java
@@ -9,6 +9,7 @@
 import javax.ws.rs.QueryParam;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.Status;
 
 import org.codehaus.jettison.json.JSONException;
 import org.codehaus.jettison.json.JSONObject;
@@ -55,7 +56,7 @@ public Response list(@QueryParam("document") String documentId) throws JSONExcep
             // Check ACL on the document
             AclDao aclDao = new AclDao();
             if (!aclDao.checkPermission(documentId, PermType.READ, principal.getId())) {
-                throw new ForbiddenClientException();
+                return Response.status(Status.NOT_FOUND).build();
             }
             criteria.setDocumentId(documentId);
         }

diff --git a/docs-web/src/main/java/com/sismics/docs/rest/resource/DocumentResource.java b/docs-web/src/main/java/com/sismics/docs/rest/resource/DocumentResource.java
@@ -20,6 +20,7 @@
 import javax.ws.rs.QueryParam;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.Status;
 
 import org.apache.commons.lang.StringUtils;
 import org.codehaus.jettison.json.JSONException;
@@ -92,7 +93,7 @@ public Response get(
                 throw new ForbiddenClientException();
             }
         } catch (NoResultException e) {
-            throw new ClientException("DocumentNotFound", MessageFormat.format("Document not found: {0}", documentId));
+            return Response.status(Status.NOT_FOUND).build();
         }
 
         JSONObject document = new JSONObject();
@@ -430,7 +431,7 @@ public Response update(
         try {
             document = documentDao.getDocument(id, PermType.WRITE, principal.getId());
         } catch (NoResultException e) {
-            throw new ClientException("DocumentNotFound", MessageFormat.format("Document not found: {0}", id));
+            return Response.status(Status.NOT_FOUND).build();
         }
 
         // Update the document
@@ -514,7 +515,7 @@ public Response delete(
             document = documentDao.getDocument(id, PermType.WRITE, principal.getId());
             fileList = fileDao.getByDocumentId(principal.getId(), id);
         } catch (NoResultException e) {
-            throw new ClientException("DocumentNotFound", MessageFormat.format("Document not found: {0}", id));
+            return Response.status(Status.NOT_FOUND).build();
         }
 
         // Delete the document

diff --git a/docs-web/src/main/java/com/sismics/docs/rest/resource/FileResource.java b/docs-web/src/main/java/com/sismics/docs/rest/resource/FileResource.java
@@ -102,7 +102,7 @@ public Response add(
             try {
                 document = documentDao.getDocument(documentId, PermType.WRITE, principal.getId());
             } catch (NoResultException e) {
-                throw new ClientException("DocumentNotFound", MessageFormat.format("Document not found: {0}", documentId));
+                return Response.status(Status.NOT_FOUND).build();
             }
         }
 
@@ -199,7 +199,7 @@ public Response attach(
             file = fileDao.getFile(id, principal.getId());
             document = documentDao.getDocument(documentId, PermType.WRITE, principal.getId());
         } catch (NoResultException e) {
-            throw new ClientException("DocumentNotFound", MessageFormat.format("Document not found: {0}", documentId));
+            return Response.status(Status.NOT_FOUND).build();
         }
 
         // Check that the file is orphan
@@ -259,7 +259,7 @@ public Response reorder(
         try {
             documentDao.getDocument(documentId, PermType.WRITE, principal.getId());
         } catch (NoResultException e) {
-            throw new ClientException("DocumentNotFound", MessageFormat.format("Document not found: {0}", documentId));
+            return Response.status(Status.NOT_FOUND).build();
         }
 
         // Reorder files
@@ -295,13 +295,9 @@ public Response list(
 
         // Check document visibility
         if (documentId != null) {
-            try {
-                AclDao aclDao = new AclDao();
-                if (!aclDao.checkPermission(documentId, PermType.READ, shareId == null ? principal.getId() : shareId)) {
-                    throw new ForbiddenClientException();
-                }
-            } catch (NoResultException e) {
-                throw new ClientException("DocumentNotFound", MessageFormat.format("Document not found: {0}", documentId));
+            AclDao aclDao = new AclDao();
+            if (!aclDao.checkPermission(documentId, PermType.READ, shareId == null ? principal.getId() : shareId)) {
+                return Response.status(Status.NOT_FOUND).build();
             }
         } else if (!authenticated) {
             throw new ForbiddenClientException();
@@ -358,7 +354,7 @@ public Response delete(
                 documentDao.getDocument(file.getDocumentId(), PermType.WRITE, principal.getId());
             }
         } catch (NoResultException e) {
-            throw new ClientException("FileNotFound", MessageFormat.format("File not found: {0}", id));
+            return Response.status(Status.NOT_FOUND).build();
         }
 
         // Delete the file
@@ -498,7 +494,7 @@ public Response zip(
                 throw new ForbiddenClientException();
             }
         } catch (NoResultException e) {
-            throw new ClientException("DocumentNotFound", MessageFormat.format("Document not found: {0}", documentId));
+            return Response.status(Status.NOT_FOUND).build();
         }
 
         // Get files and user associated with this document

diff --git a/docs-web/src/main/webapp/src/app/docs/controller/DocumentView.js b/docs-web/src/main/webapp/src/app/docs/controller/DocumentView.js
@@ -7,6 +7,8 @@ angular.module('docs').controller('DocumentView', function ($scope, $state, $sta
   // Load document data from server
   Restangular.one('document', $stateParams.id).get().then(function(data) {
     $scope.document = data;
+  }, function(response) {
+    $scope.error = response;
   });
 
   // Load audit log data from server

diff --git a/docs-web/src/main/webapp/src/app/docs/directive/ImgError.js b/docs-web/src/main/webapp/src/app/docs/directive/ImgError.js
@@ -0,0 +1,16 @@
+'use strict';
+
+/**
+ * Image error event directive.
+ */
+angular.module('docs').directive('imgError', function() {
+  return {
+    restrict: 'A',
+    link: function(scope, element, attrs) {
+      element.bind('error', function() {
+        //call the function that was passed
+        scope.$apply(attrs.imgError);
+      });
+    }
+  };
+})
diff --git a/docs-web/src/main/webapp/src/index.html b/docs-web/src/main/webapp/src/index.html
@@ -64,6 +64,7 @@
     <script src="app/docs/directive/SelectTag.js" type="text/javascript"></script>
     <script src="app/docs/directive/AuditLog.js" type="text/javascript"></script>
     <script src="app/docs/directive/InlineEdit.js" type="text/javascript"></script>
+    <script src="app/docs/directive/ImgError.js" type="text/javascript"></script>
     <!-- endref -->
   </head>
   <body>

diff --git a/docs-web/src/main/webapp/src/partial/docs/document.view.html b/docs-web/src/main/webapp/src/partial/docs/document.view.html
@@ -1,4 +1,11 @@
-<img src="img/loader.gif" ng-show="!document" />
+<img src="img/loader.gif" ng-show="!document && !error" />
+
+<div ng-show="error" class="well-lg">
+  <p class="text-center" ng-show="error.status == 404">
+    <span class="glyphicon glyphicon-warning-sign"></span>
+    Document not found
+  </p>
+</div>
 
 <div ng-show="document">
 	<div class="text-right" ng-show="document.writable">

diff --git a/docs-web/src/main/webapp/src/partial/docs/file.view.html b/docs-web/src/main/webapp/src/partial/docs/file.view.html
@@ -22,5 +22,12 @@
 
 
 <div class="text-center" ng-if="$stateParams.fileId">
-  <img ng-src="../api/file/{{ $stateParams.fileId }}/data?size=web" />
+  <img ng-src="../api/file/{{ $stateParams.fileId }}/data?size=web"
+       ng-init="error = false"
+       img-error="error = true"
+       ng-show="!error" />
+  <p class="well-lg" ng-show="error">
+    <span class="glyphicon glyphicon-warning-sign"></span>
+    File not found
+  </p>
 </div>
diff --git a/docs-web/src/test/java/com/sismics/docs/rest/TestDocumentResource.java b/docs-web/src/test/java/com/sismics/docs/rest/TestDocumentResource.java
@@ -282,8 +282,7 @@ public void testDocumentResource() throws Exception {
         documentResource = resource().path("/document/" + document1Id);
         documentResource.addFilter(new CookieAuthenticationFilter(document1Token));
         response = documentResource.get(ClientResponse.class);
-        json = response.getEntity(JSONObject.class);
-        Assert.assertEquals(Status.BAD_REQUEST, Status.fromStatusCode(response.getStatus()));
+        Assert.assertEquals(Status.NOT_FOUND, Status.fromStatusCode(response.getStatus()));
     }
 
     /**