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>
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java
index 689a556..c46f8e3 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java
@@ -293,7 +293,7 @@
}
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",
diff --git a/gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java b/gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java
index c73a85f..720d108 100644
--- a/gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java
+++ b/gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java
@@ -116,25 +116,15 @@
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) {
cfg.setStringList(section, subsection, name, values);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java
index df4ead8..a8600fe 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java
@@ -43,7 +43,7 @@
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 @@
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 @@
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 static class ClassData extends ClassVisitor {
int access;
String className;
+ String superName;
String annotationName;
String annotationValue;
String[] interfaces;
@@ -212,7 +218,7 @@
String superName, String[] interfaces) {
this.className = Type.getObjectType(name).getClassName();
this.access = access;
- this.interfaces = interfaces;
+ this.superName = superName;
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/securestore/DefaultSecureStore.java b/gerrit-server/src/main/java/com/google/gerrit/server/securestore/DefaultSecureStore.java
index 130ed0f..b852217 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/securestore/DefaultSecureStore.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/securestore/DefaultSecureStore.java
@@ -30,7 +30,7 @@
import java.util.List;
@Singleton
-public class DefaultSecureStore implements SecureStore {
+public class DefaultSecureStore extends SecureStore {
private final FileBasedConfig sec;
@Inject
@@ -45,26 +45,11 @@
}
@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) {
if (values != null) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStore.java b/gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStore.java
index b1f07ae..f3eeaf6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStore.java
+++ b/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 @@
}
}
- 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();
}
