Improves Error Handling

pom.xml

@@ -16,8 +16,8 @@
     <url>https://s3ninja.net</url>
 
     <properties>
-        <sirius.kernel>dev-35.8.3</sirius.kernel>
-        <sirius.web>dev-66.7.2</sirius.web>
+        <sirius.kernel>dev-38.2.0</sirius.kernel>
+        <sirius.web>dev-70.2.2</sirius.web>
     </properties>
 
     <repositories>

src/main/java/ninja/Bucket.java

@@ -136,22 +136,26 @@ public boolean exists() {
      * <p>
      * If the underlying directory already exists, nothing happens.
      *
-     * @return <b>true</b> if the folder for the bucket was created successfully and if it was missing before
+     * @return <b>true</b> if the folder for the bucket was created successfully or existed before, <b>false</b> else
      */
     public boolean create() {
-        if (folder.exists() || !folder.mkdirs()) {
+        if (folder.exists()) {
+            return true;
+        }
+
+        if (!folder.mkdirs()) {
             return false;
         }
 
         // having successfully created the folder, write the version marker
         writeVersion();
-        return true;
+        return folder.isDirectory();
     }
 
     /**
      * Deletes the bucket and all of its contents.
      *
-     * @return true if all files of the bucket and the bucket itself was deleted successfully, false otherwise.
+     * @return <b>true</b> if all files of the bucket and the bucket itself were deleted successfully, <b>false</b> else
      */
     public boolean delete() {
         if (!folder.exists()) {
@@ -161,8 +165,8 @@ public boolean delete() {
         try {
             sirius.kernel.commons.Files.delete(folder.toPath());
             return true;
-        } catch (IOException e) {
-            Exceptions.handle(e);
+        } catch (IOException exception) {
+            Exceptions.handle(Storage.LOG, exception);
             return false;
         }
     }
@@ -189,8 +193,8 @@ public void outputObjectsV1(XMLStructuredOutput output,
         output.property("Prefix", prefix);
         try {
             walkFileTreeOurWay(folder.toPath(), visitor);
-        } catch (IOException e) {
-            throw Exceptions.handle(e);
+        } catch (IOException exception) {
+            throw Exceptions.handle(Storage.LOG, exception);
         }
         output.property("IsTruncated", limit > 0 && visitor.getCount() > limit);
         output.endOutput();
@@ -218,8 +222,8 @@ public void outputObjectsV2(XMLStructuredOutput output,
         output.property("Prefix", prefix);
         try {
             walkFileTreeOurWay(folder.toPath(), visitor);
-        } catch (IOException e) {
-            throw Exceptions.handle(e);
+        } catch (IOException exception) {
+            throw Exceptions.handle(Storage.LOG, exception);
         }
         output.property("IsTruncated", limit > 0 && visitor.getCount() > limit);
         output.property("KeyCount", visitor.getCount());
@@ -239,32 +243,33 @@ private void walkFileTreeOurWay(Path path, FileVisitor<? super Path> visitor) th
         }
 
         try (Stream<Path> children = Files.list(path)) {
-            children.filter(p -> filterObjects(p.toFile())).sorted(Bucket::compareUtf8Binary).forEach(p -> {
-                try {
-                    BasicFileAttributes attrs = Files.readAttributes(p, BasicFileAttributes.class);
-                    visitor.visitFile(p, attrs);
-                } catch (IOException e) {
-                    throw Exceptions.handle(e);
-                }
-            });
+            children.filter(childPath -> filterObjects(childPath.toFile()))
+                    .sorted(Bucket::compareUtf8Binary)
+                    .forEach(childPath -> {
+                        try {
+                            visitor.visitFile(childPath, Files.readAttributes(childPath, BasicFileAttributes.class));
+                        } catch (IOException exception) {
+                            throw Exceptions.handle(Storage.LOG, exception);
+                        }
+                    });
         }
     }
 
-    private static int compareUtf8Binary(Path p1, Path p2) {
-        String s1 = StoredObject.decodeKey(p1.getFileName().toString());
-        String s2 = StoredObject.decodeKey(p2.getFileName().toString());
+    private static int compareUtf8Binary(Path path1, Path path2) {
+        String string1 = StoredObject.decodeKey(path1.getFileName().toString());
+        String string2 = StoredObject.decodeKey(path2.getFileName().toString());
 
-        byte[] b1 = s1.getBytes(StandardCharsets.UTF_8);
-        byte[] b2 = s2.getBytes(StandardCharsets.UTF_8);
+        byte[] bytes1 = string1.getBytes(StandardCharsets.UTF_8);
+        byte[] bytes2 = string2.getBytes(StandardCharsets.UTF_8);
 
         // unless we upgrade to java 9+ offering Arrays.compare(...), we need to compare the arrays manually :(
-        int length = Math.min(b1.length, b2.length);
-        for (int i = 0; i < length; ++i) {
-            if (b1[i] != b2[i]) {
-                return Byte.compare(b1[i], b2[i]);
+        int length = Math.min(bytes1.length, bytes2.length);
+        for (int index = 0; index < length; ++index) {
+            if (bytes1[index] != bytes2[index]) {
+                return Byte.compare(bytes1[index], bytes2[index]);
             }
         }
-        return b1.length - b2.length;
+        return bytes1.length - bytes2.length;
     }
 
     /**
@@ -278,26 +283,35 @@ public boolean isPrivate() {
 
     /**
      * Marks the bucket as only privately accessible, i.e. non-public.
+     *
+     * @return <b>true</b> if the bucket is now only privately accessible, <b>false</b> else
      */
-    public void makePrivate() {
+    public boolean makePrivate() {
         if (publicMarker.exists()) {
             sirius.kernel.commons.Files.delete(publicMarker);
             publicAccessCache.put(getName(), false);
         }
+
+        return !publicMarker.exists();
     }
 
     /**
      * Marks the bucket as publicly accessible.
+     *
+     * @return <b>true</b> if the bucket is now publicly accessible, <b>false</b> else
      */
-    public void makePublic() {
+    public boolean makePublic() {
         if (!publicMarker.exists()) {
             try {
                 new FileOutputStream(publicMarker).close();
-            } catch (IOException e) {
-                throw Exceptions.handle(Storage.LOG, e);
+            } catch (IOException exception) {
+                Exceptions.handle(Storage.LOG, exception);
+                return false;
             }
         }
         publicAccessCache.put(getName(), true);
+
+        return publicMarker.exists();
     }
 
     /**
@@ -334,15 +348,15 @@ public StoredObject getObject(String key) {
      */
     public List<StoredObject> getObjects(@Nullable String query, Limit limit) {
         try (Stream<Path> stream = Files.list(folder.toPath())) {
-            return stream.filter(p -> filterObjects(p.toFile()))
+            return stream.filter(path -> filterObjects(path.toFile()))
                          .sorted(Bucket::compareUtf8Binary)
                          .map(Path::toFile)
                          .filter(currentFile -> isMatchingObject(query, currentFile))
                          .filter(limit.asPredicate())
                          .map(StoredObject::new)
                          .toList();
-        } catch (IOException e) {
-            throw Exceptions.handle(e);
+        } catch (IOException exception) {
+            throw Exceptions.handle(Storage.LOG, exception);
         }
     }
 
@@ -357,8 +371,8 @@ public int countObjects(@Nullable String query) {
             return Math.toIntExact(stream.map(Path::toFile)
                                          .filter(currentFile -> isMatchingObject(query, currentFile))
                                          .count());
-        } catch (IOException e) {
-            throw Exceptions.handle(e);
+        } catch (IOException exception) {
+            throw Exceptions.handle(Storage.LOG, exception);
         }
     }
 
@@ -392,8 +406,8 @@ private int parseVersion() {
         try {
             // parse the version from the version marker file
             return Integer.parseInt(Strings.join(Files.readAllLines(versionMarker.toPath()), "\n").trim());
-        } catch (IOException e) {
-            throw Exceptions.handle(Storage.LOG, e);
+        } catch (IOException exception) {
+            throw Exceptions.handle(Storage.LOG, exception);
         }
     }
 
@@ -410,8 +424,8 @@ protected void writeVersion() {
         try {
             // write the version into the version marker file
             Files.write(versionMarker.toPath(), Collections.singletonList(String.valueOf(version)));
-        } catch (IOException e) {
-            throw Exceptions.handle(Storage.LOG, e);
+        } catch (IOException exception) {
+            throw Exceptions.handle(Storage.LOG, exception);
         }
     }
 
@@ -470,7 +484,7 @@ public static boolean isValidName(@Nullable String name) {
             if (IP_ADDRESS_PATTERN.matcher(name).matches() && InetAddress.getByName(name) != null) {
                 return false;
             }
-        } catch (Exception e) {
+        } catch (Exception ignored) {
             // ignore this, we want the conversion to fail and thus to end up here
         }
 

src/main/java/ninja/NinjaController.java

@@ -157,7 +157,12 @@ public void bucket(WebContext webContext, String bucketName) {
                 return;
             }
 
-            bucket.create();
+            if (!bucket.create()) {
+                throw Exceptions.createHandled()
+                                .to(Storage.LOG)
+                                .withDirectMessage("Failed creating bucket. Missing file system permission?")
+                                .handle();
+            }
 
             UserContext.message(Message.info().withTextMessage("Bucket successfully created."));
             webContext.respondWith().redirectTemporarily("/ui/" + bucket.getEncodedName());
@@ -181,7 +186,12 @@ public void bucket(WebContext webContext, String bucketName) {
 
         // handle /ui/[bucket]?make-public
         if (webContext.hasParameter("make-public")) {
-            bucket.makePublic();
+            if (!bucket.makePublic()) {
+                throw Exceptions.createHandled()
+                                .to(Storage.LOG)
+                                .withDirectMessage("Failed making bucket public. Missing file system permission?")
+                                .handle();
+            }
 
             UserContext.message(Message.info().withTextMessage("ACLs successfully changed"));
             webContext.respondWith().redirectTemporarily(address);
@@ -190,7 +200,12 @@ public void bucket(WebContext webContext, String bucketName) {
 
         // handle /ui/[bucket]?make-private
         if (webContext.hasParameter("make-private")) {
-            bucket.makePrivate();
+            if (!bucket.makePrivate()) {
+                throw Exceptions.createHandled()
+                                .to(Storage.LOG)
+                                .withDirectMessage("Failed making bucket private. Missing file system permission?")
+                                .handle();
+            }
 
             UserContext.message(Message.info().withTextMessage("ACLs successfully changed"));
             webContext.respondWith().redirectTemporarily(address);
@@ -199,7 +214,12 @@ public void bucket(WebContext webContext, String bucketName) {
 
         // handle /ui/[bucket]?delete
         if (webContext.hasParameter("delete")) {
-            bucket.delete();
+            if (!bucket.delete()) {
+                throw Exceptions.createHandled()
+                                .to(Storage.LOG)
+                                .withDirectMessage("Failed deleting bucket. Missing file system permission?")
+                                .handle();
+            }
 
             UserContext.message(Message.info().withTextMessage("Bucket successfully deleted."));
             webContext.respondWith().redirectTemporarily("/ui");