Merge branch 'stable-3.9'
* stable-3.9:
Bubble up OWNERS file parsing errors
Change-Id: I792d11268b896f15df89611685ecdf8f8e91108d
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
index d6fd574..9a8edb6 100644
--- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
@@ -256,6 +256,8 @@
logger.warn("Could not open change: {}", cId, e);
} catch (ReviewerManagerException e) {
logger.warn("Could not add reviewers for change: {}", cId, e);
+ } catch (InvalidOwnersFileException e) {
+ logger.warn("Could not add reviewers for change: {} due to rules error", cId, e);
}
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java
index 2c8c619..54c624e 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java
@@ -37,24 +37,19 @@
this.accounts = accounts;
}
- public Optional<OwnersConfig> getOwnersConfig(byte[] yamlBytes) {
- try {
- final OwnersConfig ret = new OwnersConfig();
- JsonNode jsonNode = new ObjectMapper(new YAMLFactory()).readValue(yamlBytes, JsonNode.class);
- Boolean inherited =
- Optional.ofNullable(jsonNode.get("inherited")).map(JsonNode::asBoolean).orElse(true);
- ret.setInherited(inherited);
- ret.setLabel(
- Optional.ofNullable(jsonNode.get("label"))
- .map(JsonNode::asText)
- .flatMap(LabelDefinition::parse));
- addClassicMatcher(jsonNode, ret);
- addMatchers(jsonNode, ret);
- return Optional.of(ret);
- } catch (IOException e) {
- log.warn("Unable to read YAML Owners file", e);
- return Optional.empty();
- }
+ public OwnersConfig getOwnersConfig(byte[] yamlBytes) throws IOException {
+ final OwnersConfig ret = new OwnersConfig();
+ JsonNode jsonNode = new ObjectMapper(new YAMLFactory()).readValue(yamlBytes, JsonNode.class);
+ Boolean inherited =
+ Optional.ofNullable(jsonNode.get("inherited")).map(JsonNode::asBoolean).orElse(true);
+ ret.setInherited(inherited);
+ ret.setLabel(
+ Optional.ofNullable(jsonNode.get("label"))
+ .map(JsonNode::asText)
+ .flatMap(LabelDefinition::parse));
+ addClassicMatcher(jsonNode, ret);
+ addMatchers(jsonNode, ret);
+ return ret;
}
private void addClassicMatcher(JsonNode jsonNode, OwnersConfig ret) {
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/InvalidOwnersFileException.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/InvalidOwnersFileException.java
new file mode 100644
index 0000000..1914625
--- /dev/null
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/InvalidOwnersFileException.java
@@ -0,0 +1,30 @@
+// Copyright (C) 2024 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.googlesource.gerrit.owners.common;
+
+public class InvalidOwnersFileException extends Exception {
+ private static final long serialVersionUID = 1L;
+
+ public InvalidOwnersFileException(
+ String project, String ownersPath, String branch, Throwable reason) {
+ super(exceptionMessage(project, ownersPath, branch), reason);
+ }
+
+ private static String exceptionMessage(String project, String ownersPath, String branch) {
+ return String.format(
+ "Invalid owners file: %s, in project: %s, on branch %s", ownersPath, project, branch);
+ }
+}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
index b904715..e7e437a 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
@@ -103,7 +103,8 @@
Map<String, FileDiffOutput> fileDiffMap,
boolean expandGroups,
String project,
- PathOwnersEntriesCache cache) {
+ PathOwnersEntriesCache cache)
+ throws InvalidOwnersFileException {
this(
accounts,
repositoryManager,
@@ -125,7 +126,8 @@
DiffSummary diffSummary,
boolean expandGroups,
String project,
- PathOwnersEntriesCache cache) {
+ PathOwnersEntriesCache cache)
+ throws InvalidOwnersFileException {
this(
accounts,
repositoryManager,
@@ -147,7 +149,8 @@
Set<String> modifiedPaths,
boolean expandGroups,
String project,
- PathOwnersEntriesCache cache) {
+ PathOwnersEntriesCache cache)
+ throws InvalidOwnersFileException {
this.repositoryManager = repositoryManager;
this.repository = repository;
this.parentProjectsNames = parentProjectsNames;
@@ -156,10 +159,12 @@
this.accounts = accounts;
this.expandGroups = expandGroups;
- OwnersMap map =
- branchWhenEnabled
- .map(branch -> fetchOwners(project, branch, cache))
- .orElse(new OwnersMap());
+ OwnersMap map;
+ if (branchWhenEnabled.isPresent()) {
+ map = fetchOwners(project, branchWhenEnabled.get(), cache);
+ } else {
+ map = new OwnersMap();
+ }
owners = Multimaps.unmodifiableSetMultimap(map.getPathOwners());
reviewers = Multimaps.unmodifiableSetMultimap(map.getPathReviewers());
matchers = map.getMatchers();
@@ -215,8 +220,10 @@
* Fetched the owners for the associated patch list.
*
* @return A structure containing matchers paths to owners
+ * @throws InvalidOwnersFileException when reading/parsing OWNERS file fails
*/
- private OwnersMap fetchOwners(String project, String branch, PathOwnersEntriesCache cache) {
+ private OwnersMap fetchOwners(String project, String branch, PathOwnersEntriesCache cache)
+ throws InvalidOwnersFileException {
OwnersMap ownersMap = new OwnersMap();
try {
// Using a `map` would have needed a try/catch inside the lamba, resulting in more code
@@ -267,16 +274,17 @@
}
}
ownersMap.setLabel(Optional.ofNullable(currentEntry).flatMap(PathOwnersEntry::getLabel));
- return ownersMap;
- } catch (IOException | ExecutionException e) {
- log.warn("Invalid OWNERS file", e);
- return ownersMap;
+ } catch (IOException e) {
+ log.warn("Opening repository {} failed", project, e);
+ } catch (ExecutionException e) {
+ log.warn("Reading OWNERS file for {} project, from cache failed", project, e);
}
+ return ownersMap;
}
private List<ReadOnlyPathOwnersEntry> getPathOwnersEntries(
List<Project.NameKey> projectNames, String branch, PathOwnersEntriesCache cache)
- throws IOException, ExecutionException {
+ throws IOException, InvalidOwnersFileException, ExecutionException {
ImmutableList.Builder<ReadOnlyPathOwnersEntry> pathOwnersEntries = ImmutableList.builder();
for (Project.NameKey projectName : projectNames) {
try (Repository repo = repositoryManager.openRepository(projectName)) {
@@ -290,7 +298,7 @@
private ReadOnlyPathOwnersEntry getPathOwnersEntryOrEmpty(
String project, Repository repo, String branch, PathOwnersEntriesCache cache)
- throws ExecutionException {
+ throws InvalidOwnersFileException, ExecutionException {
return getPathOwnersEntry(project, repo, branch, cache)
.map(v -> (ReadOnlyPathOwnersEntry) v)
.orElse(PathOwnersEntry.EMPTY);
@@ -298,27 +306,33 @@
private PathOwnersEntry getPathOwnersEntryOrNew(
String project, Repository repo, String branch, PathOwnersEntriesCache cache)
- throws ExecutionException {
+ throws InvalidOwnersFileException, ExecutionException {
return getPathOwnersEntry(project, repo, branch, cache).orElseGet(PathOwnersEntry::new);
}
private Optional<PathOwnersEntry> getPathOwnersEntry(
String project, Repository repo, String branch, PathOwnersEntriesCache cache)
- throws ExecutionException {
+ throws InvalidOwnersFileException, ExecutionException {
String rootPath = "OWNERS";
- return cache
- .get(project, branch, rootPath, () -> getOwnersConfig(repo, rootPath, branch))
- .map(
- conf ->
- new PathOwnersEntry(
+ return unwrapInvalidOwnersFileException(
+ () ->
+ cache
+ .get(
+ project,
+ branch,
rootPath,
- conf,
- accounts,
- Optional.empty(),
- Collections.emptySet(),
- Collections.emptySet(),
- Collections.emptySet(),
- Collections.emptySet()));
+ () -> getOwnersConfig(project, repo, rootPath, branch))
+ .map(
+ conf ->
+ new PathOwnersEntry(
+ rootPath,
+ conf,
+ accounts,
+ Optional.empty(),
+ Collections.emptySet(),
+ Collections.emptySet(),
+ Collections.emptySet(),
+ Collections.emptySet())));
}
private void processMatcherPerPath(
@@ -374,7 +388,7 @@
PathOwnersEntry rootEntry,
Map<String, PathOwnersEntry> entries,
PathOwnersEntriesCache cache)
- throws ExecutionException {
+ throws InvalidOwnersFileException, ExecutionException {
String[] parts = path.split("/");
PathOwnersEntry currentEntry = rootEntry;
StringBuilder builder = new StringBuilder();
@@ -401,31 +415,33 @@
String ownersPath = partial + "OWNERS";
PathOwnersEntry pathFallbackEntry = currentEntry;
currentEntry =
- cache
- .get(
- project,
- branch,
- ownersPath,
- () -> getOwnersConfig(repository, ownersPath, branch))
- .map(
- c -> {
- Optional<LabelDefinition> label = pathFallbackEntry.getLabel();
- final Set<Id> owners = pathFallbackEntry.getOwners();
- final Set<Id> reviewers = pathFallbackEntry.getReviewers();
- Collection<Matcher> inheritedMatchers =
- pathFallbackEntry.getMatchers().values();
- Set<String> groupOwners = pathFallbackEntry.getGroupOwners();
- return new PathOwnersEntry(
- ownersPath,
- c,
- accounts,
- label,
- owners,
- reviewers,
- inheritedMatchers,
- groupOwners);
- })
- .orElse(pathFallbackEntry);
+ unwrapInvalidOwnersFileException(
+ () ->
+ cache
+ .get(
+ project,
+ branch,
+ ownersPath,
+ () -> getOwnersConfig(project, repository, ownersPath, branch))
+ .map(
+ c -> {
+ Optional<LabelDefinition> label = pathFallbackEntry.getLabel();
+ final Set<Id> owners = pathFallbackEntry.getOwners();
+ final Set<Id> reviewers = pathFallbackEntry.getReviewers();
+ Collection<Matcher> inheritedMatchers =
+ pathFallbackEntry.getMatchers().values();
+ Set<String> groupOwners = pathFallbackEntry.getGroupOwners();
+ return new PathOwnersEntry(
+ ownersPath,
+ c,
+ accounts,
+ label,
+ owners,
+ reviewers,
+ inheritedMatchers,
+ groupOwners);
+ })
+ .orElse(pathFallbackEntry));
entries.put(partial, currentEntry);
}
}
@@ -480,12 +496,46 @@
/**
* Returns the parsed FileOwnersConfig file for the given path if it exists.
*
- * @param ownersPath path to OWNERS file in the git repo
- * @return config or null if it doesn't exist
- * @throws IOException
+ * @return Optional(config) or Optional.empty if it doesn't exist
+ * @throws InvalidOwnersFileException when reading/parsing of the OWNERS file fails
*/
- private Optional<OwnersConfig> getOwnersConfig(Repository repo, String ownersPath, String branch)
- throws IOException {
- return getBlobAsBytes(repo, branch, ownersPath).flatMap(parser::getOwnersConfig);
+ private Optional<OwnersConfig> getOwnersConfig(
+ String project, Repository repo, String ownersPath, String branch)
+ throws InvalidOwnersFileException {
+ try {
+ Optional<byte[]> configBytes = getBlobAsBytes(repo, branch, ownersPath);
+ if (configBytes.isEmpty()) {
+ return Optional.empty();
+ }
+ return Optional.of(parser.getOwnersConfig(configBytes.get()));
+ } catch (IOException e) {
+ throw new InvalidOwnersFileException(project, ownersPath, branch, e);
+ }
+ }
+
+ @FunctionalInterface
+ private interface Executable<T> {
+ T call() throws ExecutionException;
+ }
+
+ /**
+ * Unwraps the InvalidOwnersFileException from the ExecutionException so that callers can focus on
+ * handling InvalidOwnersFileException if/when needed. Note that ExecutionException is thrown only
+ * when loading values to PathOwnersEntriesCache fails.
+ *
+ * @throws InvalidOwnersFileException when call throws ExecutionException that is matchable with
+ * InvalidOwnersFileException
+ * @throws ExecutionException in all other cases
+ */
+ private <T> T unwrapInvalidOwnersFileException(Executable<T> action)
+ throws InvalidOwnersFileException, ExecutionException {
+ try {
+ return action.call();
+ } catch (ExecutionException e) {
+ if (e.getCause() instanceof InvalidOwnersFileException) {
+ throw (InvalidOwnersFileException) e.getCause();
+ }
+ throw e;
+ }
}
}
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/Config.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/Config.java
index ae3fad2..e531a3f 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/Config.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/Config.java
@@ -85,7 +85,7 @@
return entry;
}
- Optional<OwnersConfig> getOwnersConfig(String string) {
+ OwnersConfig getOwnersConfig(String string) throws IOException {
return parser.getOwnersConfig(string.getBytes(Charsets.UTF_8));
}
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java
index 2e23f33..542ff6d 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java
@@ -339,14 +339,11 @@
}
@Test
- public void testParsingYamlWithLabelWithScore() {
+ public void testParsingYamlWithLabelWithScore() throws IOException {
String yamlString =
"inherited: true\nlabel: " + EXPECTED_LABEL + ",1\nowners:\n- " + USER_C_EMAIL_COM;
- Optional<OwnersConfig> config = getOwnersConfig(yamlString);
+ OwnersConfig ownersConfig = getOwnersConfig(yamlString);
- assertTrue(config.isPresent());
-
- OwnersConfig ownersConfig = config.get();
assertTrue(ownersConfig.isInherited());
assertThat(ownersConfig.getLabel()).isPresent();
@@ -360,14 +357,11 @@
}
@Test
- public void testParsingYamlWithLabelWithoutScore() {
+ public void testParsingYamlWithLabelWithoutScore() throws IOException {
String yamlString =
"inherited: true\nlabel: " + EXPECTED_LABEL + "\nowners:\n- " + USER_C_EMAIL_COM;
- Optional<OwnersConfig> config = getOwnersConfig(yamlString);
+ OwnersConfig ownersConfig = getOwnersConfig(yamlString);
- assertTrue(config.isPresent());
-
- OwnersConfig ownersConfig = config.get();
assertTrue(ownersConfig.isInherited());
assertThat(ownersConfig.getLabel()).isPresent();
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java
index ac011b0..a035d93 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java
@@ -31,7 +31,6 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
-import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.junit.Before;
@@ -85,11 +84,8 @@
regexMatcher(".*/a.*", ACCOUNT_D),
partialRegexMatcher("Product.sql", ACCOUNT_A));
// the function to test
- Optional<OwnersConfig> configNullable = getOwnersConfig(fullConfig);
+ OwnersConfig config = getOwnersConfig(fullConfig);
// check classical configuration
- assertThat(configNullable).isPresent();
-
- OwnersConfig config = configNullable.get();
assertTrue(config.isInherited());
Set<String> owners = config.getOwners();
@@ -237,7 +233,7 @@
public void testMatchersOnlyConfig() throws Exception {
replayAll();
- Optional<OwnersConfig> ownersConfigOpt =
+ OwnersConfig ownersConfig =
getOwnersConfig(
createConfig(
false,
@@ -245,9 +241,6 @@
suffixMatcher(".txt", ACCOUNT_B),
genericMatcher(".*", ACCOUNT_A)));
- assertThat(ownersConfigOpt).isPresent();
- OwnersConfig ownersConfig = ownersConfigOpt.get();
-
assertThat(ownersConfig.getOwners()).isEmpty();
assertThat(ownersConfig.getMatchers()).isNotEmpty();
}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
index 9d5b3d9..7c532c6 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
@@ -25,6 +25,7 @@
import com.google.gerrit.server.rules.prolog.StoredValues;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlesource.gerrit.owners.common.Accounts;
+import com.googlesource.gerrit.owners.common.InvalidOwnersFileException;
import com.googlesource.gerrit.owners.common.PathOwners;
import com.googlesource.gerrit.owners.common.PathOwnersEntriesCache;
import com.googlesource.gerrit.owners.common.PluginSettings;
@@ -73,6 +74,10 @@
settings.expandGroups(),
projectState.getName(),
cache);
+ } catch (InvalidOwnersFileException e) {
+ // re-throw exception as it is already logged but more importantly it is nicely
+ // handled by the prolog rules evaluator and results in prolog rule error
+ throw new IllegalStateException(e);
}
}
};
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
index 3935a4b..0d8dac4 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
@@ -47,6 +47,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.owners.common.Accounts;
+import com.googlesource.gerrit.owners.common.InvalidOwnersFileException;
import com.googlesource.gerrit.owners.common.LabelDefinition;
import com.googlesource.gerrit.owners.common.PathOwners;
import com.googlesource.gerrit.owners.common.PathOwnersEntriesCache;
@@ -165,6 +166,9 @@
String.format(
"Missing approvals for path(s): [%s]",
Joiner.on(", ").join(missingApprovals))));
+ } catch (InvalidOwnersFileException e) {
+ logger.atSevere().withCause(e).log("Reading/parsing OWNERS file error.");
+ return Optional.of(ruleError(e.getMessage()));
} catch (IOException e) {
String msg =
String.format(
@@ -194,7 +198,7 @@
}
private PathOwners getPathOwners(ChangeData cd, ProjectState projectState)
- throws IOException, DiffNotAvailableException {
+ throws IOException, DiffNotAvailableException, InvalidOwnersFileException {
metrics.countConfigLoads.increment();
try (Timer0.Context ctx = metrics.loadConfig.start()) {
String branch = cd.change().getDest().branch();
@@ -365,4 +369,11 @@
submitRecord.requirements = List.of(SUBMIT_REQUIREMENT);
return submitRecord;
}
+
+ private static SubmitRecord ruleError(String err) {
+ SubmitRecord submitRecord = new SubmitRecord();
+ submitRecord.status = SubmitRecord.Status.RULE_ERROR;
+ submitRecord.errorMessage = err;
+ return submitRecord;
+ }
}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
index 5f5b1fb..a724239 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
@@ -39,6 +39,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.owners.common.Accounts;
+import com.googlesource.gerrit.owners.common.InvalidOwnersFileException;
import com.googlesource.gerrit.owners.common.PathOwners;
import com.googlesource.gerrit.owners.common.PathOwnersEntriesCache;
import com.googlesource.gerrit.owners.common.PluginSettings;
@@ -144,6 +145,9 @@
fileExpandedOwners.get(fileOwnerEntry.getKey()), ownersLabels, label));
return Response.ok(new FilesOwnersResponse(ownersLabels, filesWithPendingOwners));
+ } catch (InvalidOwnersFileException e) {
+ logger.atSevere().withCause(e).log("Reading/parsing OWNERS file error.");
+ throw new ResourceConflictException(e.getMessage(), e);
}
}
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java
index d7cea60..3f7362d 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java
@@ -394,6 +394,31 @@
assertThat(changeReady.requirements).containsExactly(READY);
}
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldIndicateRuleErrorForBrokenOwnersFile() throws Exception {
+ addBrokenOwnersFileToRoot();
+
+ PushOneCommit.Result r = createChange("Add a file", "README.md", "foo");
+ ChangeApi changeApi = forChange(r);
+ ChangeInfo changeNotReady = changeApi.get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).isEmpty();
+ verifyHasSubmitRecordWithRuleError(changeNotReady.submitRecords);
+ }
+
+ private void verifyHasSubmitRecordWithRuleError(Collection<SubmitRecordInfo> records) {
+ assertThat(
+ records.stream()
+ .filter(record -> SubmitRecordInfo.Status.RULE_ERROR == record.status)
+ .filter(record -> record.errorMessage.startsWith("Invalid owners file: OWNERS"))
+ .findAny())
+ .isPresent();
+ }
+
private void verifyHasSubmitRecord(
Collection<SubmitRecordInfo> records, String label, SubmitRecordInfo.Label.Status status) {
assertThat(
@@ -435,6 +460,10 @@
return gApi.changes().id(r.getChangeId());
}
+ private void addBrokenOwnersFileToRoot() throws Exception {
+ pushOwnersToMaster("{foo");
+ }
+
private void addOwnerFileWithMatchersToRoot(
boolean inherit, String extension, TestAccount... users) throws Exception {
// Add OWNERS file to root:
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java
index 936f3d3..c39974c 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java
@@ -36,6 +36,7 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.server.project.testing.TestLabels;
+import com.googlesource.gerrit.owners.common.InvalidOwnersFileException;
import com.googlesource.gerrit.owners.entities.FilesOwnersResponse;
import com.googlesource.gerrit.owners.entities.GroupOwner;
import com.googlesource.gerrit.owners.entities.Owner;
@@ -223,6 +224,20 @@
assertThat(thrown).hasCauseThat().isInstanceOf(LabelNotFoundException.class);
}
+ @Test
+ @UseLocalDisk
+ public void shouldThrowResourceConflictWhenOwnersFileIsBroken() throws Exception {
+ addBrokenOwnersFileToRoot();
+ String changeId = createChange().getChangeId();
+
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> ownersApi.apply(parseCurrentRevisionResource(changeId)));
+ assertThat(thrown).hasMessageThat().startsWith("Invalid owners file: OWNERS");
+ assertThat(thrown).hasCauseThat().isInstanceOf(InvalidOwnersFileException.class);
+ }
+
protected void replaceCodeReviewWithLabel(LabelType label) throws Exception {
try (ProjectConfigUpdate u = updateProject(allProjects)) {
u.getConfig().getLabelSections().remove(LabelId.CODE_REVIEW);
@@ -264,6 +279,10 @@
.containsExactly("a.txt", Sets.newHashSet(rootOwner, projectOwner));
}
+ private void addBrokenOwnersFileToRoot() throws Exception {
+ merge(createChange(testRepo, "master", "Add OWNER file", "OWNERS", "{foo", ""));
+ }
+
private void addOwnerFileToProjectConfig(Project.NameKey projectNameKey, boolean inherit)
throws Exception {
addOwnerFileToProjectConfig(projectNameKey, inherit, user);
