#82: Tests, error handling of Signature's Bandana access and minor cleanup

Author: Tiliavir

src/main/java/com/baloise/confluence/digitalsignature/DigitalSignatureMacro.java

@@ -29,12 +29,12 @@
 
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.security.InvalidParameterException;
 import java.util.*;
 
 import static com.atlassian.confluence.renderer.radeox.macros.MacroUtils.defaultVelocityContext;
 import static com.atlassian.confluence.security.ContentPermission.*;
-import static com.atlassian.confluence.setup.bandana.ConfluenceBandanaContext.GLOBAL_CONTEXT;
 import static com.atlassian.confluence.util.velocity.VelocityUtils.getRenderedTemplate;
 import static java.util.Arrays.asList;
 import static java.util.stream.Collectors.toList;
@@ -296,7 +296,9 @@ private Signature sync(Signature signature, Set<String> signers) {
         save = true;
       }
 
-      if (save) save(loaded);
+      if (save) {
+        save(loaded);
+      }
     } else {
       signature.setMissingSignatures(signers);
       save(signature);
@@ -346,7 +348,7 @@ protected String getMailto(Collection<UserProfile> profiles, String subject, boo
 
   public String urlEncode(String string) {
     try {
-      return URLEncoder.encode(string, "UTF-8");
+      return URLEncoder.encode(string, StandardCharsets.UTF_8.name());
     } catch (UnsupportedEncodingException e) {
       throw new IllegalStateException(e);
     }

src/main/java/com/baloise/confluence/digitalsignature/Signature.java

@@ -41,12 +41,8 @@ public Signature(long pageId, String body, String title) {
 
   public static boolean isPetitionMode(Set<String> userGroups) {
     return userGroups != null
-               && userGroups.size() == 1
-               && userGroups.iterator().next().trim().equals("*");
-  }
-
-  String serialize() {
-    return GSON.toJson(this, Signature.class);
+           && userGroups.size() == 1
+           && userGroups.iterator().next().trim().equals("*");
   }
 
   static Signature deserialize(String serialization) {
@@ -57,7 +53,7 @@ public static Signature fromBandana(BandanaManager mgr, String key) {
     Object value = mgr.getValue(GLOBAL_CONTEXT, key);
 
     if (value == null) {
-      return null;
+      throw new IllegalArgumentException("Value is null in Bandana???");
     }
 
     if (value instanceof Signature) {
@@ -76,8 +72,7 @@ public static Signature fromBandana(BandanaManager mgr, String key) {
       }
     }
 
-    log.error("Could not deserialize {} value from Bandana", value.getClass().getName());
-    return null;
+    throw new IllegalArgumentException(String.format("Could not deserialize %s value from Bandana. Please clear the plugin-cache and reboot confluence. (https://github.com/baloise/digital-signature/issues/82)", value));
   }
 
   public static void toBandana(BandanaManager mgr, String key, Signature sig) {
@@ -88,6 +83,10 @@ public static void toBandana(BandanaManager mgr, Signature sig) {
     toBandana(mgr, sig.getKey(), sig);
   }
 
+  String serialize() {
+    return GSON.toJson(this, Signature.class);
+  }
+
   public String getHash() {
     if (hash == null) {
       hash = getKey().replace("signature.", "");

src/main/java/com/baloise/confluence/digitalsignature/rest/DigitalSignatureService.java

@@ -65,14 +65,13 @@ public class DigitalSignatureService {
   private final ContextHelper contextHelper = new ContextHelper();
   private final transient Markdown markdown = new Markdown();
 
-  public DigitalSignatureService(
-      @ComponentImport BandanaManager bandanaManager,
-      @ComponentImport SettingsManager settingsManager,
-      @ComponentImport UserManager userManager,
-      @ComponentImport LocalNotificationService notificationService,
-      @ComponentImport MailServerManager mailServerManager,
-      @ComponentImport PageManager pageManager,
-      @ComponentImport I18nResolver i18nResolver) {
+  public DigitalSignatureService(@ComponentImport BandanaManager bandanaManager,
+                                 @ComponentImport SettingsManager settingsManager,
+                                 @ComponentImport UserManager userManager,
+                                 @ComponentImport LocalNotificationService notificationService,
+                                 @ComponentImport MailServerManager mailServerManager,
+                                 @ComponentImport PageManager pageManager,
+                                 @ComponentImport I18nResolver i18nResolver) {
     this.bandanaManager = bandanaManager;
     this.settingsManager = settingsManager;
     this.userManager = userManager;

src/test/java/com/baloise/confluence/digitalsignature/SignatureSerialisationTest.java

@@ -1,18 +1,20 @@
 package com.baloise.confluence.digitalsignature;
 
-
 import org.junit.jupiter.api.Test;
 
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Date;
 
 import static org.junit.jupiter.api.Assertions.*;
 
-
 class SignatureSerialisationTest {
+  public static final String SIG_JSON = "{\"key\":\"signature.a077cdcc5bfcf275fe447ae2c609c1c361331b4e90cb85909582e0d824cbc5b3\",\"hash\":\"a077cdcc5bfcf275fe447ae2c609c1c361331b4e90cb85909582e0d824cbc5b3\",\"pageId\":123,\"title\":\"title\",\"body\":\"body\",\"maxSignatures\":-1,\"visibilityLimit\":-1,\"signatures\":{\"signed1\":\"Jan 1, 1970, 1:00:09 AM\"},\"missingSignatures\":[\"missing1\",\"missing2\"],\"notify\":[\"notify1\"]}";
+
   @Test
   void deserialize() throws IOException, ClassNotFoundException {
     ObjectInputStream in = new ObjectInputStream(getClass().getResourceAsStream("/signature.ser"));
@@ -24,7 +26,13 @@ void deserialize() throws IOException, ClassNotFoundException {
         () -> assertEquals("[missing1, missing2]", signature.getMissingSignatures().toString()),
         () -> assertEquals(1, signature.getSignatures().size()),
         () -> assertTrue(signature.getSignatures().containsKey("signed1")),
-        () -> assertEquals(9999, signature.getSignatures().get("signed1").getTime())
+        () -> assertEquals(9999, signature.getSignatures().get("signed1").getTime()),
+
+        // assert we can still read the old gson serialization
+        () -> assertEquals(signature, Signature.deserialize(SIG_JSON)),
+
+        // assert that deserialization of the serialization results in the original Signature
+        () -> assertEquals(signature, Signature.deserialize(signature.serialize()))
     );
   }
 
@@ -35,11 +43,18 @@ void serialize() throws IOException, ClassNotFoundException {
     signature.getMissingSignatures().add("missing1");
     signature.getMissingSignatures().add("missing2");
     signature.getSignatures().put("signed1", new Date(9999));
-    ObjectOutputStream out = new ObjectOutputStream(new FileOutputStream("src/test/resources/signature-test.ser"));
-    out.writeObject(signature);
-    out.close();
 
-    ObjectInputStream in = new ObjectInputStream(this.getClass().getResourceAsStream("/signature.ser"));
+    Path path = Paths.get("src/test/resources/signature-test.ser");
+    try(ObjectOutputStream out = new ObjectOutputStream(Files.newOutputStream(path))) {
+      out.writeObject(signature);
+    }
+
+    // assert the serialization we just wrote can be deserialized
+    ObjectInputStream in = new ObjectInputStream(Files.newInputStream(path));
+    assertEquals(signature, in.readObject());
+
+    // assert the historically serialized class can still be deserialized
+    in = new ObjectInputStream(this.getClass().getResourceAsStream("/signature.ser"));
     assertEquals(signature, in.readObject());
   }
 }

src/test/java/com/baloise/confluence/digitalsignature/SignatureTest.java

@@ -1,16 +1,17 @@
 package com.baloise.confluence.digitalsignature;
 
 import com.atlassian.bandana.BandanaManager;
+import com.atlassian.bandana.DefaultBandanaManager;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 
-import java.util.Set;
+import java.util.Collections;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.mockito.Matchers.any;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 class SignatureTest {
   @Nested
@@ -28,8 +29,8 @@ void serialize_empty() {
     void serialize_initializedObject() {
       Signature signature = new Signature(42L, "body text", "title text");
       signature.sign("max.mustermann");
-      signature.setMissingSignatures(Set.of("max.muster"));
-      signature.setNotify(Set.of("max.meier"));
+      signature.setMissingSignatures(Collections.singleton("max.muster"));
+      signature.setNotify(Collections.singleton("max.meier"));
 
       String json = signature.serialize();
 
@@ -46,8 +47,8 @@ void deserialize_empty() {
     void serializeAndDeserialize() {
       Signature signature = new Signature(42L, "body text", "title text");
       signature.sign("max.mustermann");
-      signature.setMissingSignatures(Set.of("max.muster"));
-      signature.setNotify(Set.of("max.meier"));
+      signature.setMissingSignatures(Collections.singleton("max.muster"));
+      signature.setNotify(Collections.singleton("max.meier"));
 
       String json = signature.serialize();
 
@@ -60,25 +61,51 @@ void serializeAndDeserialize() {
 
   @Nested
   class BandanaWrapperTest {
+    private final BandanaManager bandana = mock(DefaultBandanaManager.class);
     private final Signature signature = new Signature(1, "test", "title");
-    private final BandanaManager bandana = mock(BandanaManager.class);
+
+    @Test
+    void toBandanaFromBandana_readAsWritten() {
+      ArgumentCaptor<String> stringCapator = ArgumentCaptor.forClass(String.class);
+      ArgumentCaptor<Object> objectCapator = ArgumentCaptor.forClass(Object.class);
+
+      String key = signature.getKey();
+      assertNull(Signature.fromBandana(bandana, key), "Should not be there yet.");
+
+      doNothing().when(bandana).setValue(any(), stringCapator.capture(), objectCapator.capture());
+      when(bandana.getKeys(any())).thenReturn(Collections.singletonList(key));
+
+      Signature.toBandana(bandana, signature);
+      assertEquals(key, stringCapator.getValue());
+      assertEquals(signature.serialize(), objectCapator.getValue());
+
+      when(bandana.getValue(any(), any())).thenCallRealMethod();
+      when(bandana.getValue(any(), eq(key), eq(true))).thenReturn(signature);
+      assertEquals(signature, Signature.fromBandana(bandana, signature.getKey()));
+    }
 
     @Test
     void fromBandana_signature_signature() {
-      when(bandana.getValue(any(), any())).thenReturn(signature.serialize());
+      String key = signature.getKey();
+      assertNull(Signature.fromBandana(bandana, key), "Should not be there yet.");
 
-      Signature readSignature = Signature.fromBandana(bandana, null);
+      when(bandana.getKeys(any())).thenReturn(Collections.singletonList(key));
+      when(bandana.getValue(any(), any())).thenCallRealMethod();
+      when(bandana.getValue(any(), eq(key), eq(true))).thenReturn(signature);
 
-      assertEquals(signature, readSignature);
+      assertEquals(signature, Signature.fromBandana(bandana, signature.getKey()));
     }
 
     @Test
-    void fromBandana_string_signatur() {
-      when(bandana.getValue(any(), any())).thenReturn(signature);
+    void fromBandana_string_signature() {
+      String key = signature.getKey();
+      assertNull(Signature.fromBandana(bandana, key), "Should not be there yet.");
 
-      Signature readSignature = Signature.fromBandana(bandana, null);
+      when(bandana.getKeys(any())).thenReturn(Collections.singletonList(key));
+      when(bandana.getValue(any(), any())).thenCallRealMethod();
+      when(bandana.getValue(any(), eq(key), eq(true))).thenReturn(signature.serialize());
 
-      assertEquals(signature, readSignature);
+      assertEquals(signature, Signature.fromBandana(bandana, signature.getKey()));
     }
   }
 }