From d43fbecbaa4ef0fdee435ed3c1f8f5e68b7016ef Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 11 Mar 2020 10:06:19 +0100 Subject: [PATCH] ProjectOperations: Allow arbitrary updates of project.config This allows tests to insert invalid entries in the project.config file. Signed-off-by: Edwin Kempin Change-Id: I01440867b295e843507ca41bcd703af659db32dd --- .../gerrit/acceptance/testsuite/project/BUILD | 1 + .../testsuite/project/ProjectOperations.java | 20 ++++++ .../project/ProjectOperationsImpl.java | 44 +++++++++++++ .../project/TestProjectInvalidation.java | 64 +++++++++++++++++++ .../acceptance/api/project/ProjectIT.java | 30 ++++----- 5 files changed, 140 insertions(+), 19 deletions(-) create mode 100644 java/com/google/gerrit/acceptance/testsuite/project/TestProjectInvalidation.java diff --git a/java/com/google/gerrit/acceptance/testsuite/project/BUILD b/java/com/google/gerrit/acceptance/testsuite/project/BUILD index 8a3a23a5b6..f9e2fb5a64 100644 --- a/java/com/google/gerrit/acceptance/testsuite/project/BUILD +++ b/java/com/google/gerrit/acceptance/testsuite/project/BUILD @@ -14,6 +14,7 @@ java_library( "//java/com/google/gerrit/server", "//lib:guava", "//lib:jgit", + "//lib:jgit-junit", "//lib/auto:auto-value", "//lib/auto:auto-value-annotations", "//lib/commons:lang", diff --git a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperations.java b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperations.java index 2db611baeb..738be4d82c 100644 --- a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperations.java +++ b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperations.java @@ -73,5 +73,25 @@ public interface ProjectOperations { * @return a builder to update the check. */ TestProjectUpdate.Builder forUpdate(); + + /** + * Starts the fluent chain to invalidate a project. The returned builder can be used to specify + * how the project should be made invalid. To invalidate the project for real, {@link + * TestProjectInvalidation.Builder#invalidate()} must be called. + * + *

Example: + * + * 

+     * projectOperations.forInvalidation()
+     *     .addProjectConfigUpdater(cfg -> cfg.setString("invalidSection", null, "foo", "bar"))
+     *     .invalidate();
+     *
+ * + *

Note: The invalidation will fail with an exception if the project to + * invalidate doesn't exist. + * + * @return a builder to invalidate the project + */ + TestProjectInvalidation.Builder forInvalidation(); } } diff --git a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java index 7797fe0153..dede7e0c68 100644 --- a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java +++ b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java @@ -19,6 +19,7 @@ import static com.google.gerrit.server.project.ProjectConfig.PROJECT_CONFIG; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; +import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.gerrit.acceptance.testsuite.project.TestProjectCreation.Builder; @@ -45,6 +46,7 @@ import java.util.ArrayList; import java.util.Collections; import org.apache.commons.lang.RandomStringUtils; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectLoader; @@ -255,6 +257,48 @@ public class ProjectOperationsImpl implements ProjectOperations { throw new IllegalStateException(e); } } + + private void setConfig(Config projectConfig) { + try (TestRepository repo = + new TestRepository<>(repoManager.openRepository(nameKey))) { + repo.update( + RefNames.REFS_CONFIG, + repo.commit() + .message("Update project.config from test") + .parent(getHead(RefNames.REFS_CONFIG)) + .add(ProjectConfig.PROJECT_CONFIG, projectConfig.toText())); + } catch (Exception e) { + throw new IllegalStateException( + "updating project.config of project " + nameKey + " failed", e); + } + } + + @Override + public TestProjectInvalidation.Builder forInvalidation() { + return TestProjectInvalidation.builder(this::invalidateProject); + } + + private void invalidateProject(TestProjectInvalidation testProjectInvalidation) + throws Exception { + if (!testProjectInvalidation.projectConfigUpdater().isEmpty()) { + Config projectConfig = new Config(); + projectConfig.fromText(getConfig().toText()); + testProjectInvalidation.projectConfigUpdater().forEach(c -> c.accept(projectConfig)); + setConfig(projectConfig); + try { + projectCache.evict(nameKey); + } catch (Exception e) { + // Evicting the project from the cache, also triggers a reindex of the project. + // The reindex step fails if the project config is invalid. That's fine, since it was our + // intention to make the project config invalid. Hence we ignore exceptions that are cause + // by an invalid project config here. + if (!Throwables.getCausalChain(e).stream() + .anyMatch(ConfigInvalidException.class::isInstance)) { + throw e; + } + } + } + } } private static PermissionRule newRule(ProjectConfig project, AccountGroup.UUID groupUUID) { diff --git a/java/com/google/gerrit/acceptance/testsuite/project/TestProjectInvalidation.java b/java/com/google/gerrit/acceptance/testsuite/project/TestProjectInvalidation.java new file mode 100644 index 0000000000..f070b5603f --- /dev/null +++ b/java/com/google/gerrit/acceptance/testsuite/project/TestProjectInvalidation.java @@ -0,0 +1,64 @@ +// Copyright (C) 2020 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.acceptance.testsuite.project; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.gerrit.acceptance.testsuite.ThrowingConsumer; +import java.util.function.Consumer; +import org.eclipse.jgit.lib.Config; + +/** + * API to invalidate projects in tests. + * + *

This allows to test Gerrit behavior when there is invalid project data in NoteDb (e.g. an + * invalid {@code project.config} file). + */ +@AutoValue +public abstract class TestProjectInvalidation { + public abstract ImmutableList> projectConfigUpdater(); + + abstract ThrowingConsumer projectInvalidator(); + + public static Builder builder(ThrowingConsumer projectInvalidator) { + return new AutoValue_TestProjectInvalidation.Builder().projectInvalidator(projectInvalidator); + } + + @AutoValue.Builder + public abstract static class Builder { + /** + * Adds a consumer that can update the project config. + * + *

This allows tests to set arbitrary values in the project config. + */ + public Builder addProjectConfigUpdater(Consumer projectConfigUpdater) { + projectConfigUpdaterBuilder().add(projectConfigUpdater); + return this; + } + + protected abstract ImmutableList.Builder> projectConfigUpdaterBuilder(); + + abstract Builder projectInvalidator( + ThrowingConsumer projectInvalidator); + + abstract TestProjectInvalidation autoBuild(); + + /** Executes the project invalidation as specified. */ + public void invalidate() { + TestProjectInvalidation projectInvalidation = autoBuild(); + projectInvalidation.projectInvalidator().acceptAndThrowSilently(projectInvalidation); + } + } +} diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java index 72ab2e61f9..b572757345 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java @@ -68,7 +68,6 @@ import com.google.gerrit.server.config.ProjectConfigEntry; import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.index.IndexExecutor; import com.google.gerrit.server.project.CommentLinkInfoImpl; -import com.google.gerrit.server.project.ProjectConfig; import com.google.inject.AbstractModule; import com.google.inject.Inject; import com.google.inject.Module; @@ -77,7 +76,6 @@ import java.util.Map; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Config; -import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.transport.PushResult; import org.eclipse.jgit.transport.RemoteRefUpdate.Status; @@ -881,23 +879,17 @@ public class ProjectIT extends AbstractDaemonTest { // Update the definition of the Code-Review label so that it has the value "+1 LGTM" twice. // This update bypasses all validation checks so that the duplicate label value doesn't get // rejected. - Config cfg = new Config(); - cfg.fromText(projectOperations.project(allProjects).getConfig().toText()); - cfg.setStringList( - "label", - "Code-Review", - "value", - ImmutableList.of("+1 LGTM", "1 LGTM", "0 No Value", "-1 Looks Bad")); - - try (TestRepository repo = - new TestRepository<>(repoManager.openRepository(allProjects))) { - repo.update( - RefNames.REFS_CONFIG, - repo.commit() - .message("Set label with duplicate value") - .parent(getHead(repo.getRepository(), RefNames.REFS_CONFIG)) - .add(ProjectConfig.PROJECT_CONFIG, cfg.toText())); - } + projectOperations + .project(allProjects) + .forInvalidation() + .addProjectConfigUpdater( + cfg -> + cfg.setStringList( + "label", + "Code-Review", + "value", + ImmutableList.of("+1 LGTM", "1 LGTM", "0 No Value", "-1 Looks Bad"))) + .invalidate(); // Verify that project info can be retrieved and that the label value "+1 LGTM" appears only // once.