opendev/gerrit
Code Issues Proposed changes

Add ConfigSubject over JGit Configs

Browse Source 
The general pattern for check methods is Config#getFoo becomes
ConfigSubject#fooValues, with the same argument list. A more fluent API
using intermediate types may be possible, but the goal to start with is
to keep the API close to the underlying Config APIs.

Change-Id: I09e6e04a5a77fd440ff34d3d284b1d69f8440695
This commit is contained in:
Dave Borowitz
2019-05-14 11:24:14 +02:00
parent 38c42ee6e0
commit 61b835d906
11 changed files with 166 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
java/com/google/gerrit/acceptance/BUILD
View File

@@ -35,6 +35,7 @@ java_library(
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/sshd",
"//java/com/google/gerrit/testing:gerrit-test-util",
"//java/com/google/gerrit/truth",
"//lib:args4j",
"//lib:gson",
"//lib:guava-retrying",

1
java/com/google/gerrit/truth/BUILD
View File

@@ -6,6 +6,7 @@ java_library(
deps = [
"//java/com/google/gerrit/common:annotations",
"//lib:guava",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/truth",
],
)

129
java/com/google/gerrit/truth/ConfigSubject.java Normal file
View File

@@ -0,0 +1,129 @@
// Copyright (C) 2019 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.truth;
import static com.google.common.truth.Truth.assertAbout;
import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.truth.BooleanSubject;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.IntegerSubject;
import com.google.common.truth.IterableSubject;
import com.google.common.truth.ListMultimapSubject;
import com.google.common.truth.LongSubject;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
import com.google.gerrit.common.Nullable;
import java.util.Arrays;
import org.eclipse.jgit.lib.Config;
public class ConfigSubject extends Subject<ConfigSubject, Config> {
public static ConfigSubject assertThat(Config config) {
return assertAbout(ConfigSubject::new).that(config);
}
private final Config config;
private ConfigSubject(FailureMetadata metadata, Config actual) {
super(metadata, actual);
this.config = actual;
}
public IterableSubject sections() {
isNotNull();
return check("getSections()").that(config.getSections());
}
public IterableSubject subsections(String section) {
requireNonNull(section);
isNotNull();
return check("getSubsections(%s)", section).that(config.getSubsections(section));
}
public ListMultimapSubject sectionValues(String section) {
requireNonNull(section);
return sectionValuesImpl(section, null);
}
public ListMultimapSubject subsectionValues(String section, String subsection) {
requireNonNull(section);
requireNonNull(subsection);
return sectionValuesImpl(section, subsection);
}
private ListMultimapSubject sectionValuesImpl(String section, @Nullable String subsection) {
isNotNull();
ImmutableListMultimap.Builder<String, String> b = ImmutableListMultimap.builder();
config
.getNames(section, subsection, true)
.forEach(
n ->
Arrays.stream(config.getStringList(section, subsection, n))
.forEach(v -> b.put(n, v)));
return check("getSection(%s, %s)", section, subsection).that(b.build());
}
public void isEmpty() {
sections().isEmpty();
}
public StringSubject text() {
isNotNull();
return check("toText()").that(config.toText());
}
public IterableSubject stringValues(String section, @Nullable String subsection, String name) {
requireNonNull(section);
requireNonNull(name);
isNotNull();
return check("getStringList(%s, %s, %s)", section, subsection, name)
.that(Arrays.asList(config.getStringList(section, subsection, name)));
}
public StringSubject stringValue(String section, @Nullable String subsection, String name) {
requireNonNull(section);
requireNonNull(name);
isNotNull();
return check("getString(%s, %s, %s)", section, subsection, name)
.that(config.getString(section, subsection, name));
}
public IntegerSubject intValue(
String section, @Nullable String subsection, String name, int defaultValue) {
requireNonNull(section);
requireNonNull(name);
isNotNull();
return check("getInt(%s, %s, %s, %s)", section, subsection, name, defaultValue)
.that(config.getInt(section, subsection, name, defaultValue));
}
public LongSubject longValue(String section, String subsection, String name, long defaultValue) {
requireNonNull(section);
requireNonNull(name);
isNotNull();
return check("getLong(%s, %s, %s, %s)", section, subsection, name, defaultValue)
.that(config.getLong(section, subsection, name, defaultValue));
}
public BooleanSubject booleanValue(
String section, String subsection, String name, boolean defaultValue) {
requireNonNull(section);
requireNonNull(name);
isNotNull();
return check("getBoolean(%s, %s, %s, %s)", section, subsection, name, defaultValue)
.that(config.getBoolean(section, subsection, name, defaultValue));
}
}

1
javatests/com/google/gerrit/acceptance/BUILD
View File

@@ -7,6 +7,7 @@ junit_tests(
"//java/com/google/gerrit/acceptance:lib",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/testing:gerrit-test-util",
"//java/com/google/gerrit/truth",
"//lib:guava",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/truth",

3
javatests/com/google/gerrit/acceptance/MergeableFileBasedConfigTest.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.ImmutableList;
@@ -111,7 +112,7 @@ public class MergeableFileBasedConfigTest {
}
private void assertConfig(MergeableFileBasedConfig cfg, String expected) throws Exception {
assertThat(cfg.toText()).isEqualTo(expected);
assertThat(cfg).text().isEqualTo(expected);
cfg.save();
assertThat(new String(Files.readAllBytes(cfg.getFile().toPath()), UTF_8)).isEqualTo(expected);
}

8
javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
View File

@@ -33,6 +33,7 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPG
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Objects.requireNonNull;
import static java.util.concurrent.TimeUnit.SECONDS;
@@ -477,10 +478,11 @@ public class AccountIT extends AbstractDaemonTest {
assertThat(tw).isNotNull();
Config cfg = new Config();
cfg.fromText(new String(or.open(tw.getObjectId(0), OBJ_BLOB).getBytes(), UTF_8));
assertThat(
cfg.getString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_FULL_NAME))
assertThat(cfg)
.stringValue(AccountProperties.ACCOUNT, null, AccountProperties.KEY_FULL_NAME)
.isEqualTo(name);
assertThat(cfg.getString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS))
assertThat(cfg)
.stringValue(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS)
.isEqualTo(status);
} else {
// No account properties were set, hence an 'account.config' file was not created.

5
javatests/com/google/gerrit/acceptance/rest/change/ConfigChangeIT.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil;
@@ -71,7 +72,7 @@ public class ConfigChangeIT extends AbstractDaemonTest {
private String testUpdateProjectConfig() throws Exception {
Config cfg = projectOperations.project(project).getConfig();
assertThat(cfg.getString("project", null, "description")).isNull();
assertThat(cfg).stringValue("project", null, "description").isNull();
String desc = "new project description";
cfg.setString("project", null, "description", desc);
@@ -108,7 +109,7 @@ public class ConfigChangeIT extends AbstractDaemonTest {
requestScopeOperations.setApiUser(user.id());
Config cfg = projectOperations.project(project).getConfig();
assertThat(cfg.getString("access", null, "inheritFrom")).isAnyOf(null, allProjects.get());
assertThat(cfg).stringValue("access", null, "inheritFrom").isAnyOf(null, allProjects.get());
cfg.setString("access", null, "inheritFrom", parent.name);
PushOneCommit.Result r = createConfigChange(cfg);

5
javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
View File

@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil;
@@ -568,7 +569,7 @@ public class AccessIT extends AbstractDaemonTest {
.file(ProjectConfig.PROJECT_CONFIG)
.asString();
cfg.fromText(config);
assertThat(cfg.getString(access, refsFor, unknownPermission)).isEqualTo(registeredUsers);
assertThat(cfg).stringValue(access, refsFor, unknownPermission).isEqualTo(registeredUsers);
// Make permission change through API
ProjectAccessInput accessInput = newProjectAccessInput();
@@ -587,7 +588,7 @@ public class AccessIT extends AbstractDaemonTest {
.file(ProjectConfig.PROJECT_CONFIG)
.asString();
cfg.fromText(config);
assertThat(cfg.getString(access, refsFor, unknownPermission)).isEqualTo(registeredUsers);
assertThat(cfg).stringValue(access, refsFor, unknownPermission).isEqualTo(registeredUsers);
}
@Test

10
javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.testsuite.project;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import static java.util.stream.Collectors.toList;
import com.google.common.collect.ImmutableList;
@@ -108,7 +109,7 @@ public class ProjectOperationsImplTest extends AbstractDaemonTest {
Project.NameKey key = projectOperations.newProject().create();
Config config = projectOperations.project(key).getConfig();
assertThat(config).isNotInstanceOf(StoredConfig.class);
assertThat(config.toText()).isEmpty();
assertThat(config).text().isEmpty();
ConfigInput input = new ConfigInput();
input.description = "my fancy project";
@@ -116,7 +117,9 @@ public class ProjectOperationsImplTest extends AbstractDaemonTest {
config = projectOperations.project(key).getConfig();
assertThat(config).isNotInstanceOf(StoredConfig.class);
assertThat(config.toText()).isEqualTo("[project]\n\tdescription = my fancy project\n");
assertThat(config).sections().containsExactly("project");
assertThat(config).subsections("project").isEmpty();
assertThat(config).sectionValues("project").containsExactly("description", "my fancy project");
}
@Test
@@ -125,9 +128,8 @@ public class ProjectOperationsImplTest extends AbstractDaemonTest {
deleteRefsMetaConfig(key);
Config config = projectOperations.project(key).getConfig();
assertThat(config).isNotNull();
assertThat(config).isNotInstanceOf(StoredConfig.class);
assertThat(config.toText()).isEmpty();
assertThat(config).isEmpty();
}
private void deleteRefsMetaConfig(Project.NameKey key) throws Exception {

23
javatests/com/google/gerrit/server/config/ConfigUtilTest.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.config;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import static java.util.concurrent.TimeUnit.DAYS;
import static java.util.concurrent.TimeUnit.HOURS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
@@ -90,17 +91,17 @@ public class ConfigUtilTest {
Config cfg = new Config();
ConfigUtil.storeSection(cfg, SECT, SUB, in, d);
assertThat(cfg.getString(SECT, SUB, "CONSTANT")).isNull();
assertThat(cfg.getString(SECT, SUB, "missing")).isNull();
assertThat(cfg.getBoolean(SECT, SUB, "b", false)).isEqualTo(in.b);
assertThat(cfg.getBoolean(SECT, SUB, "bb", false)).isEqualTo(in.bb);
assertThat(cfg.getInt(SECT, SUB, "i", 0)).isEqualTo(0);
assertThat(cfg.getInt(SECT, SUB, "ii", 0)).isEqualTo(in.ii);
assertThat(cfg.getLong(SECT, SUB, "l", 0L)).isEqualTo(0L);
assertThat(cfg.getLong(SECT, SUB, "ll", 0L)).isEqualTo(in.ll);
assertThat(cfg.getString(SECT, SUB, "s")).isEqualTo(in.s);
assertThat(cfg.getString(SECT, SUB, "sd")).isNull();
assertThat(cfg.getString(SECT, SUB, "nd")).isNull();
assertThat(cfg).stringValue(SECT, SUB, "CONSTANT").isNull();
assertThat(cfg).stringValue(SECT, SUB, "missing").isNull();
assertThat(cfg).booleanValue(SECT, SUB, "b", false).isEqualTo(in.b);
assertThat(cfg).booleanValue(SECT, SUB, "bb", false).isEqualTo(in.bb);
assertThat(cfg).intValue(SECT, SUB, "i", 0).isEqualTo(0);
assertThat(cfg).intValue(SECT, SUB, "ii", 0).isEqualTo(in.ii);
assertThat(cfg).longValue(SECT, SUB, "l", 0L).isEqualTo(0L);
assertThat(cfg).longValue(SECT, SUB, "ll", 0L).isEqualTo(in.ll);
assertThat(cfg).stringValue(SECT, SUB, "s").isEqualTo(in.s);
assertThat(cfg).stringValue(SECT, SUB, "sd").isNull();
assertThat(cfg).stringValue(SECT, SUB, "nd").isNull();
SectionInfo out = new SectionInfo();
ConfigUtil.loadSection(cfg, SECT, SUB, out, d, null);

5
javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java
View File

@@ -21,6 +21,7 @@ import static com.google.gerrit.server.schema.testing.AllProjectsCreatorTestUtil
import static com.google.gerrit.server.schema.testing.AllProjectsCreatorTestUtil.getAllProjectsWithoutDefaultAcls;
import static com.google.gerrit.server.schema.testing.AllProjectsCreatorTestUtil.getDefaultAllProjectsWithAllDefaultSections;
import static com.google.gerrit.server.schema.testing.AllProjectsCreatorTestUtil.readAllProjectsConfig;
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.data.GroupReference;
@@ -126,7 +127,7 @@ public class AllProjectsCreatorTest {
allProjectsCreator.create(allProjectsInput);
Config config = readAllProjectsConfig(repoManager, allProjectsName);
assertThat(config.getString("project", null, "description")).isEqualTo(testDescription);
assertThat(config).stringValue("project", null, "description").isEqualTo(testDescription);
}
@Test
@@ -142,7 +143,7 @@ public class AllProjectsCreatorTest {
allProjectsCreator.create(allProjectsInput);
Config config = readAllProjectsConfig(repoManager, allProjectsName);
assertThat(config.getBoolean("submit", null, "rejectEmptyCommit", false)).isTrue();
assertThat(config).booleanValue("submit", null, "rejectEmptyCommit", false).isTrue();
}
@Test