Make SecureStore an abstract class

This modification will make the implementation of a site program to
switch the secure store easier. The new SwitchSecureStore program will
be added by a follow-up change.

In switch-secure-store we need to migrate all values stored in the old
SecureStore to new one. Because we don't store (and also can't reliably
figure out) the value type the only possible way is to first try read
the value as String, then when it is null try to get it as List. But
this assumption will fail with jgit Config implementation which is used
in DefaultSecureStore. JGit Config during getString() method call will
call getRawStringList() and return its first element. Therefore migration
will be incorrect for all stored List values.

The solution here is to always use getList() and setList() in
switch-secure-store and repeat jgit behavior in SecureStore. Making the
get() method to return getList()[0] and set() to call setList() with
single element.

To provide default implementations of get() and set() method in
SecureStore we need to make it as an abstract class. Also those methods
are marked as final to prevent implementors from overriding them.

Also changes JarScanner.findImplementationsOf to findSubClassesOf(). This
method was introduced for SecureStore (and it was only used by it), so
now when SecureStore becomes abstract class its implementation (and
name) also should be updated.

Change-Id: Id13bc6ec170c31b43b5a1cd696ba746006e1f0b2
Signed-off-by: Dariusz Luksza <dariusz@luksza.org>

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java

@@ -293,7 +293,7 @@ public class BaseInit extends SiteProgram {
       }
       JarScanner scanner = new JarScanner(secureStoreLib);
       List<String> secureStores =
-          scanner.findImplementationsOf(SecureStore.class);
+          scanner.findSubClassesOf(SecureStore.class);
       if (secureStores.isEmpty()) {
         throw new InvalidSecureStoreException(String.format(
             "Cannot find class implementing %s interface in %s",

gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java

@@ -116,24 +116,14 @@ public class UpgradeFrom2_0_xTest extends InitTestCase {
     u.run();
   }
 
-  private static class InMemorySecureStore implements SecureStore {
+  private static class InMemorySecureStore extends SecureStore {
     private final Config cfg = new Config();
 
-    @Override
-    public String get(String section, String subsection, String name) {
-      return cfg.getString(section, subsection, name);
-    }
-
     @Override
     public String[] getList(String section, String subsection, String name) {
       return cfg.getStringList(section, subsection, name);
     }
 
-    @Override
-    public void set(String section, String subsection, String name, String value) {
-      cfg.setString(section, subsection, name, value);
-    }
-
     @Override
     public void setList(String section, String subsection, String name,
         List<String> values) {

gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java

@@ -43,7 +43,7 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.annotation.Annotation;
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -134,11 +134,14 @@ public class JarScanner implements PluginContentScanner {
     return result.build();
   }
 
-  public List<String> findImplementationsOf(Class<?> requestedInterface)
-      throws IOException {
-    List<String> result = Lists.newArrayList();
-    String name = requestedInterface.getName().replace('.', '/');
+  public List<String> findSubClassesOf(Class<?> superClass) throws IOException {
+    return findSubClassesOf(superClass.getName());
+  }
 
+  private List<String> findSubClassesOf(String superClass) throws IOException {
+    String name = superClass.replace('.', '/');
+
+    List<String> classes = new ArrayList<>();
     Enumeration<JarEntry> e = jarFile.entries();
     while (e.hasMoreElements()) {
       JarEntry entry = e.nextElement();
@@ -155,13 +158,15 @@ public class JarScanner implements PluginContentScanner {
         continue;
       }
 
-      if (def.isConcrete() && def.interfaces != null
-          && Iterables.contains(Arrays.asList(def.interfaces), name)) {
-        result.add(def.className);
+      if (name.equals(def.superName)) {
+        classes.addAll(findSubClassesOf(def.className));
+        if (def.isConcrete()) {
+          classes.add(def.className);
+        }
       }
     }
 
-    return result;
+    return classes;
   }
 
   private static boolean skip(JarEntry entry) {
@@ -192,6 +197,7 @@ public class JarScanner implements PluginContentScanner {
   public static class ClassData extends ClassVisitor {
     int access;
     String className;
+    String superName;
     String annotationName;
     String annotationValue;
     String[] interfaces;
@@ -212,7 +218,7 @@ public class JarScanner implements PluginContentScanner {
         String superName, String[] interfaces) {
       this.className = Type.getObjectType(name).getClassName();
       this.access = access;
-      this.interfaces = interfaces;
+      this.superName = superName;
     }
 
     @Override

gerrit-server/src/main/java/com/google/gerrit/server/securestore/DefaultSecureStore.java

@@ -30,7 +30,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 @Singleton
-public class DefaultSecureStore implements SecureStore {
+public class DefaultSecureStore extends SecureStore {
   private final FileBasedConfig sec;
 
   @Inject
@@ -44,26 +44,11 @@ public class DefaultSecureStore implements SecureStore {
     }
   }
 
-  @Override
-  public String get(String section, String subsection, String name) {
-    return sec.getString(section, subsection, name);
-  }
-
   @Override
   public String[] getList(String section, String subsection, String name) {
     return sec.getStringList(section, subsection, name);
   }
 
-  @Override
-  public void set(String section, String subsection, String name, String value) {
-    if (value != null) {
-      sec.setString(section, subsection, name, value);
-    } else {
-      sec.unset(section, subsection, name);
-    }
-    save();
-  }
-
   @Override
   public void setList(String section, String subsection, String name,
       List<String> values) {

gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStore.java

@@ -14,9 +14,11 @@
 
 package com.google.gerrit.server.securestore;
 
+import com.google.common.collect.Lists;
+
 import java.util.List;
 
-public interface SecureStore {
+public abstract class SecureStore {
   public static class EntryKey {
     public final String name;
     public final String section;
@@ -29,15 +31,23 @@ public interface SecureStore {
     }
   }
 
-  String get(String section, String subsection, String name);
+  public final String get(String section, String subsection, String name) {
+    String[] values = getList(section, subsection, name);
+    if (values != null && values.length > 0) {
+      return values[0];
+    }
+    return null;
+  }
 
-  String[] getList(String section, String subsection, String name);
+  public abstract String[] getList(String section, String subsection, String name);
 
-  void set(String section, String subsection, String name, String value);
+  public final void set(String section, String subsection, String name, String value) {
+    setList(section, subsection, name, Lists.newArrayList(value));
+  }
 
-  void setList(String section, String subsection, String name, List<String> values);
+  public abstract void setList(String section, String subsection, String name, List<String> values);
 
-  void unset(String section, String subsection, String name);
+  public abstract void unset(String section, String subsection, String name);
 
-  Iterable<EntryKey> list();
+  public abstract Iterable<EntryKey> list();
 }