opendev/gerrit
Code Issues Proposed changes

ConfigUtil: Allow default null values for string attributes

Browse Source 
AccountGeneralPreferences has downloadScheme attribute that has
default value null, so we need to support it.

Change-Id: I1b43c517f2742b5b56526b3911dd9fb8217f2509
This commit is contained in:
David Ostrovsky
2015-11-01 09:45:41 +01:00
committed by David Pursehouse
parent 7523c33cd6
commit a8ba8665a5
2 changed files with 17 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.server.config; package com.google.gerrit.server.config;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -282,7 +281,9 @@ public class ConfigUtil {
f.setAccessible(true); f.setAccessible(true);
Object c = f.get(s); Object c = f.get(s);
Object d = f.get(defaults); Object d = f.get(defaults);
Preconditions.checkNotNull(d, "Default cannot be null"); if (!isString(t)) {
Preconditions.checkNotNull(d, "Default cannot be null for: " + n);
}
if (c == null || c.equals(d)) { if (c == null || c.equals(d)) {
cfg.unset(section, sub, n); cfg.unset(section, sub, n);
} else { } else {
@@ -336,9 +337,15 @@ public class ConfigUtil {
String n = f.getName(); String n = f.getName();
f.setAccessible(true); f.setAccessible(true);
Object d = f.get(defaults); Object d = f.get(defaults);
Preconditions.checkNotNull(d, "Default cannot be null"); if (!isString(t)) {
Preconditions.checkNotNull(d, "Default cannot be null for: " + n);
}
if (isString(t)) { if (isString(t)) {
f.set(s, MoreObjects.firstNonNull(cfg.getString(section, sub, n), d)); String v = cfg.getString(section, sub, n);
if (v == null) {
v = (String)d;
}
f.set(s, v);
} else if (isInteger(t)) { } else if (isInteger(t)) {
f.set(s, cfg.getInt(section, sub, n, (Integer) d)); f.set(s, cfg.getInt(section, sub, n, (Integer) d));
} else if (isLong(t)) { } else if (isLong(t)) {

6
gerrit-server/src/test/java/com/google/gerrit/server/config/ConfigUtilTest.java
View File

@@ -47,6 +47,7 @@ public class ConfigUtilTest {
public Boolean bd; public Boolean bd;
public String s; public String s;
public String sd; public String sd;
public String nd;
public Theme t; public Theme t;
public Theme td; public Theme td;
static SectionInfo defaults() { static SectionInfo defaults() {
@@ -62,6 +63,9 @@ public class ConfigUtilTest {
i.bd = true; i.bd = true;
i.s = "foo"; i.s = "foo";
i.sd = "bar"; i.sd = "bar";
// This line is not needed, as it's null per default.
// Put it here to be explicit.
i.nd = null;
i.t = Theme.DEFAULT; i.t = Theme.DEFAULT;
i.td = Theme.DEFAULT; i.td = Theme.DEFAULT;
return i; return i;
@@ -96,6 +100,7 @@ public class ConfigUtilTest {
assertThat(cfg.getLong(SECT, SUB, "ll", 0L)).isEqualTo(in.ll); assertThat(cfg.getLong(SECT, SUB, "ll", 0L)).isEqualTo(in.ll);
assertThat(cfg.getString(SECT, SUB, "s")).isEqualTo(in.s); assertThat(cfg.getString(SECT, SUB, "s")).isEqualTo(in.s);
assertThat(cfg.getString(SECT, SUB, "sd")).isNull(); assertThat(cfg.getString(SECT, SUB, "sd")).isNull();
assertThat(cfg.getString(SECT, SUB, "nd")).isNull();
SectionInfo out = new SectionInfo(); SectionInfo out = new SectionInfo();
ConfigUtil.loadSection(cfg, SECT, SUB, out, d, null); ConfigUtil.loadSection(cfg, SECT, SUB, out, d, null);
@@ -110,6 +115,7 @@ public class ConfigUtilTest {
assertThat(out.bd).isNull(); assertThat(out.bd).isNull();
assertThat(out.s).isEqualTo(in.s); assertThat(out.s).isEqualTo(in.s);
assertThat(out.sd).isEqualTo(d.sd); assertThat(out.sd).isEqualTo(d.sd);
assertThat(out.nd).isNull();
assertThat(out.t).isEqualTo(in.t); assertThat(out.t).isEqualTo(in.t);
assertThat(out.td).isEqualTo(d.td); assertThat(out.td).isEqualTo(d.td);
} }