Merge "Drop CodeOwnersInternalServerErrorException"
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackend.java b/java/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackend.java
index 0e4e9e7..4837ee0 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackend.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackend.java
@@ -14,7 +14,6 @@
package com.google.gerrit.plugins.codeowners.backend;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.PLUGIN;
import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.VERSIONED_META_DATA_CHANGE;
import static java.util.Objects.requireNonNull;
@@ -25,6 +24,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginProjectConfigSnapshot;
import com.google.gerrit.server.GerritPersonIdent;
@@ -121,10 +121,10 @@
fileName, codeOwnerConfigParser, revWalk, revision, codeOwnerConfigKey);
}
} catch (IOException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format("failed to load code owner config %s", codeOwnerConfigKey), e);
} catch (ConfigInvalidException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"invalid code owner config file %s (project = %s, branch = %s)",
codeOwnerConfigKey.filePath(defaultFileName),
@@ -221,7 +221,7 @@
.call();
} catch (Exception e) {
Throwables.throwIfUnchecked(e);
- throw newInternalServerError(
+ throw new StorageException(
String.format("failed to upsert code owner config %s", codeOwnerConfigKey), e);
}
}
@@ -271,7 +271,7 @@
return codeOwnerConfigFile.getLoadedCodeOwnerConfig();
} catch (IOException | ConfigInvalidException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format("failed to upsert code owner config %s", codeOwnerConfigKey), e);
}
}
@@ -293,7 +293,7 @@
}
return metaDataUpdate;
} catch (Exception e) {
- throw newInternalServerError("Failed to create MetaDataUpdate", e);
+ throw new StorageException("Failed to create MetaDataUpdate", e);
} finally {
metaDataUpdate.close();
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index 6abc820..afaf7ad 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -17,7 +17,6 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
@@ -33,6 +32,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.metrics.Timer1;
@@ -194,7 +194,7 @@
.orElse(null)))
.collect(toImmutableList());
} catch (IOException | DiffNotAvailableException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"failed to compute owned paths of patch set %s for account %d",
patchSet.id(), accountId.get()),
@@ -438,7 +438,8 @@
codeOwnersConfig, codeOwnerResolver, changeNotes, accountIds);
ChangedFilesByPatchSetCache changedFilesByPatchSetCache =
changedFilesByPatchSetCacheFactory.create(codeOwnersConfig, changeNotes);
- return changedFiles.getFromDiffCache(changeNotes.getProjectName(), patchSet.commitId())
+ return changedFiles
+ .getFromDiffCache(changeNotes.getProjectName(), patchSet.commitId())
.stream()
.map(
changedFile ->
@@ -459,7 +460,7 @@
return changeNotes.getChange().getRevertOf() != null
&& pureRevertCache.isPureRevert(changeNotes);
} catch (BadRequestException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"failed to check if change %s in project %s is a pure revert",
changeNotes.getChangeId(), changeNotes.getProjectName()),
@@ -775,7 +776,7 @@
implicitApprover, reviewerAccountIds, approverAccountIds, absolutePath, reason);
}
- throw newInternalServerError(
+ throw new StorageException(
String.format("unknown fallback code owners configured: %s", fallbackCodeOwners));
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFileUpdateScanner.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFileUpdateScanner.java
index 0fa31e1..24a8b9f 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFileUpdateScanner.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFileUpdateScanner.java
@@ -14,13 +14,13 @@
package com.google.gerrit.plugins.codeowners.backend;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.PLUGIN;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Objects.requireNonNull;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.git.RefUpdateUtil;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.server.GerritPersonIdent;
@@ -148,7 +148,7 @@
updateBranch(repository, branchNameKey, revision, commitId);
return Optional.of(rw.parseCommit(commitId));
} catch (IOException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"Failed to scan for code owner configs in branch %s of project %s",
branchNameKey.branch(), branchNameKey.project()),
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigHierarchy.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigHierarchy.java
index dfd4fee..49c06bb 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigHierarchy.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigHierarchy.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.codeowners.backend;
import static com.google.common.base.Preconditions.checkState;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import static java.util.Objects.requireNonNull;
import com.google.common.flogger.FluentLogger;
@@ -23,6 +22,7 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
import java.io.IOException;
@@ -294,7 +294,7 @@
logger.atFine().log("code owner config %s not found", metaCodeOwnerConfigKey);
}
} catch (IOException e) {
- throw newInternalServerError(String.format("failed to read %s", metaCodeOwnerConfigKey), e);
+ throw new StorageException(String.format("failed to read %s", metaCodeOwnerConfigKey), e);
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java
index d55ff8f..f5356c5 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java
@@ -15,13 +15,13 @@
package com.google.gerrit.plugins.codeowners.backend;
import static com.google.gerrit.plugins.codeowners.backend.CodeOwners.getInvalidCodeOwnerConfigCause;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import static java.util.Objects.requireNonNull;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
@@ -143,12 +143,12 @@
CodeOwnerConfig codeOwnerConfig;
try {
codeOwnerConfig = treeWalk.getCodeOwnerConfig();
- } catch (CodeOwnersInternalServerErrorException codeOwnersInternalServerErrorException) {
+ } catch (RuntimeException e) {
Optional<InvalidCodeOwnerConfigException> invalidCodeOwnerConfigException =
- getInvalidCodeOwnerConfigCause(codeOwnersInternalServerErrorException);
+ getInvalidCodeOwnerConfigCause(e);
if (!invalidCodeOwnerConfigException.isPresent()) {
// Propagate any failure that is not related to the contents of the code owner config.
- throw codeOwnersInternalServerErrorException;
+ throw e;
}
// The code owner config is invalid and cannot be parsed.
@@ -163,7 +163,7 @@
}
}
} catch (IOException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"Failed to scan for code owner configs in branch %s of project %s",
branchNameKey.branch(), branchNameKey.project()),
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
index 07d1c7c..38ce3e0 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
@@ -17,7 +17,6 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
@@ -30,6 +29,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
@@ -604,8 +604,7 @@
});
return extIdsByEmail;
} catch (IOException e) {
- throw newInternalServerError(
- String.format("cannot resolve code owner emails: %s", emails), e);
+ throw new StorageException(String.format("cannot resolve code owner emails: %s", emails), e);
}
}
@@ -643,8 +642,8 @@
createDebugMessageForNonResolvableEmail(
e.getKey(),
String.format(
- "cannot resolve account %s for email %s: account does not"
- + " exists",
+ "cannot resolve account %s for email %s: account does"
+ + " not exists",
accountId, e.getKey())));
}
return accountState;
@@ -837,8 +836,8 @@
messages.add(
DebugMessage.createAdminOnlyMessage(
String.format(
- "email %s is visible to user %s: email is a secondary email that is owned by this"
- + " user",
+ "email %s is visible to user %s: email is a secondary email that is owned by"
+ + " this user",
email, user.getLoggableName())));
return true;
}
@@ -882,8 +881,8 @@
createDebugMessageForNonResolvableEmail(
email,
String.format(
- "cannot resolve code owner email %s: account %s is referenced by secondary email"
- + " but the calling user %s cannot see secondary emails",
+ "cannot resolve code owner email %s: account %s is referenced by secondary"
+ + " email but the calling user %s cannot see secondary emails",
email, accountState.account().id(), currentUser.get().getLoggableName())));
return false;
} else {
@@ -896,7 +895,7 @@
return true;
}
} catch (PermissionBackendException ex) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"failed to test the %s global capability", GlobalPermission.VIEW_SECONDARY_EMAILS),
ex);
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java
index 80300d6..ac570c0 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java
@@ -14,13 +14,13 @@
package com.google.gerrit.plugins.codeowners.backend;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.LegacySubmitRequirement;
import com.google.gerrit.entities.SubmitRecord;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
@@ -151,7 +151,7 @@
logger.atWarning().log("%s", errorMessage);
return Optional.of(ruleError(errorMessage));
}
- throw newInternalServerError(errorMessage, e);
+ throw new StorageException(errorMessage, e);
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java
index a965468..bc35a36 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java
@@ -87,14 +87,6 @@
return ImmutableList.of(invalidPathException.get().getMessage());
}
- // This must be done last since some of the exceptions we handle above may be wrapped in a
- // CodeOwnersInternalServerErrorException.
- Optional<CodeOwnersInternalServerErrorException> codeOwnersInternalServerErrorException =
- getCodeOwnersInternalServerErrorException(throwable);
- if (codeOwnersInternalServerErrorException.isPresent()) {
- return ImmutableList.of(codeOwnersInternalServerErrorException.get().getUserVisibleMessage());
- }
-
return ImmutableList.of();
}
@@ -106,11 +98,6 @@
return Optional.empty();
}
- private static Optional<CodeOwnersInternalServerErrorException>
- getCodeOwnersInternalServerErrorException(Throwable throwable) {
- return getCause(CodeOwnersInternalServerErrorException.class, throwable);
- }
-
public static boolean isCausedByConfigurationError(Throwable throwable) {
return isInvalidPluginConfigurationException(throwable)
|| isInvalidCodeOwnerConfigException(throwable)
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersInternalServerErrorException.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersInternalServerErrorException.java
deleted file mode 100644
index 580414a..0000000
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersInternalServerErrorException.java
+++ /dev/null
@@ -1,78 +0,0 @@
-// Copyright (C) 2021 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.plugins.codeowners.backend;
-
-import com.google.common.base.Throwables;
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.server.patch.DiffNotAvailableException;
-
-/** Exception signaling an internal server error in the code-owners plugin. */
-public class CodeOwnersInternalServerErrorException extends RuntimeException {
- private static final long serialVersionUID = 1L;
-
- private static final String USER_MESSAGE = "Internal server in code-owners plugin";
-
- /**
- * Creates a {@link CodeOwnersInternalServerErrorException} to signal an internal server error
- * caused by an issue in the code-owners plugin.
- *
- * @param message the exception message
- * @return the created exception
- */
- public static CodeOwnersInternalServerErrorException newInternalServerError(String message) {
- return new CodeOwnersInternalServerErrorException(message);
- }
-
- /**
- * Creates a {@link RuntimeException} to signal an internal server error.
- *
- * <p>By default it is assumed that the internal server error is caused by an issue in the
- * code-owners plugin and a {@link CodeOwnersInternalServerErrorException} is returned.
- *
- * <p>However for some known causes that are unrelated to code owners a {@link StorageException}
- * is thrown. This is to avoid that the code-owners plugin is mistakenly assumed to be the cause
- * of these errors.
- *
- * @param message the exception message
- * @param cause the exception cause
- * @return the created exception
- */
- public static RuntimeException newInternalServerError(String message, Throwable cause) {
- if (isNonCodeOwnersCause(cause)) {
- return new StorageException(message, cause);
- }
- return new CodeOwnersInternalServerErrorException(message, cause);
- }
-
- private CodeOwnersInternalServerErrorException(String message) {
- super(message);
- }
-
- private CodeOwnersInternalServerErrorException(String message, Throwable cause) {
- super(message, cause);
- }
-
- public String getUserVisibleMessage() {
- return USER_MESSAGE;
- }
-
- private static boolean isNonCodeOwnersCause(Throwable throwable) {
- return hasCause(DiffNotAvailableException.class, throwable);
- }
-
- private static boolean hasCause(Class<?> exceptionClass, Throwable throwable) {
- return Throwables.getCausalChain(throwable).stream().anyMatch(exceptionClass::isInstance);
- }
-}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java b/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java
index 2ce34dd..14f0b98 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java
@@ -14,12 +14,11 @@
package com.google.gerrit.plugins.codeowners.backend;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
-
import com.google.auto.value.AutoValue;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -126,7 +125,7 @@
}
return Optional.of(ref.getObjectId());
} catch (IOException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"failed to get revision of branch %s in project %s",
branchNameKey.shortName(), branchNameKey.project()),
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshot.java b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshot.java
index 786d3e4..08005b8 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshot.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshot.java
@@ -14,7 +14,6 @@
package com.google.gerrit.plugins.codeowners.backend.config;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import static java.util.Objects.requireNonNull;
@@ -29,6 +28,7 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.LabelType;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerReference;
import com.google.gerrit.plugins.codeowners.backend.EnableImplicitApprovals;
@@ -379,7 +379,7 @@
return ImmutableSet.copyOf(exemptedAccounts.values());
} catch (IOException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"Failed to resolve exempted users %s on project %s", exemptedUsers, projectName),
e);
@@ -615,8 +615,8 @@
LabelType requiredLabel = getRequiredApproval().labelType();
if (requiredLabel.isIgnoreSelfApproval()) {
logger.atFine().log(
- "ignoring implicit approval configuration on project %s since the label of the required"
- + " approval (%s) is configured to ignore self approvals",
+ "ignoring implicit approval configuration on project %s since the label of the"
+ + " required approval (%s) is configured to ignore self approvals",
projectName, requiredLabel);
return false;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
index 0e97a25..bc202da 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
@@ -19,7 +19,6 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore.IS_EXPLICITLY_MENTIONED_SCORING_VALUE;
import static com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore.NOT_EXPLICITLY_MENTIONED_SCORING_VALUE;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
@@ -32,6 +31,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.client.ListAccountsOption;
import com.google.gerrit.extensions.client.ListOption;
import com.google.gerrit.extensions.common.AccountVisibility;
@@ -564,8 +564,10 @@
// ask for 2 times the number of users that we need so that we still have enough
// suggestions when some users are removed on the filter step later or if the returned users
// were already present in codeOwners
- getRandomVisibleUsers(2 * limit - codeOwners.size()).map(CodeOwner::create)
- .collect(toImmutableSet()).stream()
+ getRandomVisibleUsers(2 * limit - codeOwners.size())
+ .map(CodeOwner::create)
+ .collect(toImmutableSet())
+ .stream()
.filter(codeOwner -> !codeOwners.contains(codeOwner))
.collect(toImmutableSet());
codeOwners.addAll(codeOwnersToAdd);
@@ -604,7 +606,7 @@
throw new IllegalStateException("unknown account visibility setting: " + accountVisibility);
} catch (IOException | PermissionBackendException e) {
- throw newInternalServerError("failed to get visible users", e);
+ throw new StorageException("failed to get visible users", e);
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index 4acae29..91a8c56 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -18,7 +18,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.plugins.codeowners.backend.CodeOwners.getInvalidCodeOwnerConfigCause;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
import static java.util.Objects.requireNonNull;
import com.google.auto.value.AutoValue;
@@ -30,6 +29,7 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -40,7 +40,6 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigReference;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerReference;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
-import com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException;
import com.google.gerrit.plugins.codeowners.backend.InvalidCodeOwnerConfigException;
import com.google.gerrit.plugins.codeowners.backend.PathCodeOwners;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
@@ -403,7 +402,7 @@
/* isBranchCreation= */ true,
refReceivedEvent.pushOptions);
} catch (IOException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"failed to validate code owner config files in revision %s"
+ " (project = %s, branch = %s)",
@@ -482,7 +481,8 @@
logger.atFine().log("force = %s", force);
if (!force && codeOwnersConfig.isDisabled(branchNameKey.branch())) {
logger.atFine().log(
- "skipping validation of code owner config files due to code-owners functionality is disabled");
+ "skipping validation of code owner config files due to code-owners functionality is"
+ + " disabled");
return Optional.empty();
}
@@ -575,7 +575,7 @@
"failed to validate code owner config files in revision %s"
+ " (project = %s, branch = %s)",
revCommit.getName(), branchNameKey.project(), branchNameKey.branch());
- throw newInternalServerError(errorMessage, e);
+ throw new StorageException(errorMessage, e);
}
}
@@ -677,13 +677,13 @@
String.format(
"code owner config %s not found in revision %s",
codeOwnerConfigKey, revCommit.name())));
- } catch (CodeOwnersInternalServerErrorException codeOwnersInternalServerErrorException) {
+ } catch (RuntimeException e) {
// Loading the code owner config has failed.
Optional<InvalidCodeOwnerConfigException> invalidCodeOwnerConfigException =
- getInvalidCodeOwnerConfigCause(codeOwnersInternalServerErrorException);
+ getInvalidCodeOwnerConfigCause(e);
if (!invalidCodeOwnerConfigException.isPresent()) {
// Propagate any failure that is not related to the contents of the code owner config.
- throw codeOwnersInternalServerErrorException;
+ throw e;
}
// The exception was caused by a ConfigInvalidException. This means loading the code owner
@@ -707,8 +707,8 @@
try {
baseCodeOwnerConfig =
getBaseCodeOwnerConfig(codeOwnerBackend, branchNameKey, changedFile, revCommit);
- } catch (CodeOwnersInternalServerErrorException codeOwnersInternalServerErrorException) {
- if (getInvalidCodeOwnerConfigCause(codeOwnersInternalServerErrorException).isPresent()) {
+ } catch (RuntimeException e) {
+ if (getInvalidCodeOwnerConfigCause(e).isPresent()) {
// The base code owner config is non-parseable. Since the update makes the code owner
// config parseable, it is a good update even if the code owner config still contains
// issues. Hence in this case we downgrade all validation errors in the new version to
@@ -718,7 +718,7 @@
}
// Propagate any exception that was not caused by the content of the code owner config.
- throw codeOwnersInternalServerErrorException;
+ throw e;
}
// Validate the parsed code owner config.
@@ -748,9 +748,8 @@
/**
* Loads and returns the base code owner config if it exists.
*
- * <p>Throws a {@link ConfigInvalidException} (wrapped in a {@link
- * CodeOwnersInternalServerErrorException} if the base code owner config exists, but is not
- * parseable.
+ * <p>Throws a {@link ConfigInvalidException} (wrapped in a {@link StorageException} if the base
+ * code owner config exists, but is not parseable.
*
* @param codeOwnerBackend the code owner backend from which the base code owner config can be
* loaded
@@ -829,16 +828,16 @@
// is introduced by the new commit and we should block uploading it, which we achieve by
// setting the validation message type to fatal.
return ValidationMessage.Type.FATAL;
- } catch (CodeOwnersInternalServerErrorException codeOwnersInternalServerErrorException) {
+ } catch (RuntimeException e) {
// Loading the base code owner config has failed.
- if (getInvalidCodeOwnerConfigCause(codeOwnersInternalServerErrorException).isPresent()) {
+ if (getInvalidCodeOwnerConfigCause(e).isPresent()) {
// The code owner config was already non-parseable before, hence we do not need to
// block the upload if the code owner config is still non-parseable.
// Using warning as type means that uploads are not blocked.
return ValidationMessage.Type.WARNING;
}
// Propagate any failure that is not related to the contents of the code owner config.
- throw codeOwnersInternalServerErrorException;
+ throw e;
}
}
@@ -1064,7 +1063,7 @@
.filter(Optional::isPresent)
.map(Optional::get);
} catch (IOException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format("Failed to validate imports for %s in ", codeOwnerConfig.key()), e);
}
}
@@ -1189,8 +1188,8 @@
keyOfImportedCodeOwnerConfig.branchNameKey().shortName(),
revision.get().name()));
}
- } catch (CodeOwnersInternalServerErrorException codeOwnersInternalServerErrorException) {
- if (getInvalidCodeOwnerConfigCause(codeOwnersInternalServerErrorException).isPresent()) {
+ } catch (RuntimeException e) {
+ if (getInvalidCodeOwnerConfigCause(e).isPresent()) {
// The imported code owner config is non-parseable.
return nonResolvableImport(
codeOwnerConfigRevision,
@@ -1206,7 +1205,7 @@
}
// Propagate any exception that was not caused by the content of the code owner config.
- throw codeOwnersInternalServerErrorException;
+ throw e;
}
// no issue found
@@ -1238,7 +1237,7 @@
.project(keyOfImportedCodeOwnerConfig.project())
.test(ProjectPermission.ACCESS);
} catch (PermissionBackendException e) {
- throw newInternalServerError(
+ throw new StorageException(
"failed to check read permission for project of imported code owner config", e);
}
}
@@ -1252,7 +1251,7 @@
.ref(keyOfImportedCodeOwnerConfig.ref())
.test(RefPermission.READ);
} catch (PermissionBackendException e) {
- throw newInternalServerError(
+ throw new StorageException(
"failed to check read permission for branch of imported code owner config", e);
}
}
@@ -1273,7 +1272,7 @@
return Optional.ofNullable(repo.exactRef(keyOfImportedCodeOwnerConfig.ref()))
.map(Ref::getObjectId);
} catch (IOException e) {
- throw newInternalServerError("failed to read revision of import code owner config", e);
+ throw new StorageException("failed to read revision of import code owner config", e);
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationPushOption.java b/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationPushOption.java
index 049cf11..7d46516 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationPushOption.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/SkipCodeOwnerConfigValidationPushOption.java
@@ -14,12 +14,11 @@
package com.google.gerrit.plugins.codeowners.validation;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
-
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.plugins.codeowners.backend.ChangedFiles;
@@ -112,7 +111,7 @@
changeNotes.getProjectName(),
Path.of(changedFile.newPath().get().toString()).getFileName().toString()));
} catch (IOException | DiffNotAvailableException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"Failed to check changed files of change %s in project %s",
changeNotes.getChangeId(), changeNotes.getProjectName()),
@@ -167,7 +166,7 @@
.currentUser()
.check(skipCodeOwnerConfigValidationCapability.getPermission());
} catch (PermissionBackendException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"Failed to check %s~%s capability",
pluginName, SkipCodeOwnerConfigValidationCapability.ID),
@@ -181,7 +180,7 @@
.currentUser()
.test(skipCodeOwnerConfigValidationCapability.getPermission());
} catch (PermissionBackendException e) {
- throw newInternalServerError(
+ throw new StorageException(
String.format(
"Failed to check %s~%s capability",
pluginName, SkipCodeOwnerConfigValidationCapability.ID),
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerHasOperandsIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerHasOperandsIT.java
index f8b21aa..4e3e74f 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerHasOperandsIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerHasOperandsIT.java
@@ -34,6 +34,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.Changes.QueryRequest;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -43,7 +44,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerApprovalHasOperand;
-import com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.inject.Inject;
@@ -158,9 +158,9 @@
when(changeDataWithoutChangeNotes.change()).thenReturn(changeData.change());
when(changeDataWithoutChangeNotes.currentPatchSet()).thenReturn(changeData.currentPatchSet());
- CodeOwnersInternalServerErrorException exception =
+ StorageException exception =
assertThrows(
- CodeOwnersInternalServerErrorException.class,
+ StorageException.class,
() ->
codeOwnerApprovalHasOperand
.create(changeQueryBuilder)
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java
index 5281391..a5fb56f 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java
@@ -21,6 +21,7 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.testing.backend.TestCodeOwnerConfigStorage;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
@@ -193,9 +194,9 @@
@Test
public void cannotGetCodeOwnerConfigFromNonExistingRevision() throws Exception {
CodeOwnerConfig.Key codeOwnerConfigKey = CodeOwnerConfig.Key.create(project, "master", "/");
- CodeOwnersInternalServerErrorException exception =
+ StorageException exception =
assertThrows(
- CodeOwnersInternalServerErrorException.class,
+ StorageException.class,
() ->
codeOwnerBackend.getCodeOwnerConfig(
codeOwnerConfigKey,
@@ -427,9 +428,9 @@
}
// Try to update the code owner config.
- CodeOwnersInternalServerErrorException exception =
+ StorageException exception =
assertThrows(
- CodeOwnersInternalServerErrorException.class,
+ StorageException.class,
() ->
codeOwnerBackend.upsertCodeOwnerConfig(
codeOwnerConfigKey,
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRuleTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRuleTest.java
index 08c45f1..9843fa9 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRuleTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRuleTest.java
@@ -24,6 +24,7 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.acceptance.testsuite.CodeOwnerConfigOperations;
@@ -131,9 +132,9 @@
when(changeDataWithoutChangeNotes.change()).thenReturn(changeData.change());
when(changeDataWithoutChangeNotes.currentPatchSet()).thenReturn(changeData.currentPatchSet());
- CodeOwnersInternalServerErrorException exception =
+ StorageException exception =
assertThrows(
- CodeOwnersInternalServerErrorException.class,
+ StorageException.class,
() -> codeOwnerSubmitRule.evaluate(changeDataWithoutChangeNotes));
assertThat(exception)
.hasMessageThat()
@@ -145,10 +146,9 @@
@Test
public void internalServerError_changeDataIsNull() throws Exception {
- CodeOwnersInternalServerErrorException exception =
+ StorageException exception =
assertThrows(
- CodeOwnersInternalServerErrorException.class,
- () -> codeOwnerSubmitRule.evaluate(/* changeData= */ null));
+ StorageException.class, () -> codeOwnerSubmitRule.evaluate(/* changeData= */ null));
assertThat(exception).hasMessageThat().isEqualTo("Failed to evaluate code owner statuses.");
}
@@ -178,8 +178,8 @@
.hasErrorMessageThat()
.isEqualTo(
String.format(
- "Failed to evaluate code owner statuses for patch set %d of change %d"
- + " (cause: invalid code owner config file '%s' (project = %s, branch = master):\n"
+ "Failed to evaluate code owner statuses for patch set %d of change %d (cause:"
+ + " invalid code owner config file '%s' (project = %s, branch = master):\n"
+ " %s).%s",
changeData.change().currentPatchSetId().get(),
changeData.change().getId().get(),
@@ -209,8 +209,8 @@
.hasErrorMessageThat()
.isEqualTo(
String.format(
- "Failed to evaluate code owner statuses for patch set %d of change %d"
- + " (cause: invalid code owner config file '%s' (project = %s, branch = master):\n"
+ "Failed to evaluate code owner statuses for patch set %d of change %d (cause:"
+ + " invalid code owner config file '%s' (project = %s, branch = master):\n"
+ " %s).",
changeData.change().currentPatchSetId().get(),
changeData.change().getId().get(),
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java
index 2f64510..df8f0cd 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java
@@ -49,16 +49,6 @@
assertThat(skipRetryWithTrace(newInvalidPathException())).isTrue();
assertThat(skipRetryWithTrace(newExceptionWithCause(newInvalidPathException()))).isTrue();
- assertThat(
- skipRetryWithTrace(
- CodeOwnersInternalServerErrorException.newInternalServerError("msg")))
- .isFalse();
- assertThat(
- skipRetryWithTrace(
- newExceptionWithCause(
- CodeOwnersInternalServerErrorException.newInternalServerError("msg"))))
- .isFalse();
-
assertThat(skipRetryWithTrace(new Exception())).isFalse();
assertThat(skipRetryWithTrace(newExceptionWithCause(new Exception()))).isFalse();
}
@@ -85,13 +75,6 @@
assertThat(getUserMessages(newExceptionWithCause(invalidPathException)))
.containsExactly(invalidPathException.getMessage());
- CodeOwnersInternalServerErrorException codeOwnersInternalServerErrorException =
- CodeOwnersInternalServerErrorException.newInternalServerError("msg");
- assertThat(getUserMessages(codeOwnersInternalServerErrorException))
- .containsExactly(codeOwnersInternalServerErrorException.getUserVisibleMessage());
- assertThat(getUserMessages(newExceptionWithCause(codeOwnersInternalServerErrorException)))
- .containsExactly(codeOwnersInternalServerErrorException.getUserVisibleMessage());
-
assertThat(getUserMessages(new Exception())).isEmpty();
assertThat(getUserMessages(newExceptionWithCause(new Exception()))).isEmpty();
}
@@ -139,15 +122,6 @@
OptionalSubject.assertThat(getStatus(new Exception())).isEmpty();
OptionalSubject.assertThat(getStatus(newExceptionWithCause(new Exception()))).isEmpty();
-
- OptionalSubject.assertThat(
- getStatus(CodeOwnersInternalServerErrorException.newInternalServerError("msg")))
- .isEmpty();
- OptionalSubject.assertThat(
- getStatus(
- newExceptionWithCause(
- CodeOwnersInternalServerErrorException.newInternalServerError("msg"))))
- .isEmpty();
}
private boolean skipRetryWithTrace(Exception exception) {
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersInternalServerErrorExceptionTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersInternalServerErrorExceptionTest.java
deleted file mode 100644
index ac1e583..0000000
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersInternalServerErrorExceptionTest.java
+++ /dev/null
@@ -1,49 +0,0 @@
-// Copyright (C) 2022 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.plugins.codeowners.backend;
-
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
-
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
-import com.google.gerrit.server.patch.DiffNotAvailableException;
-import org.junit.Test;
-
-/** Tests for {@link CodeOwnersInternalServerErrorException}. */
-public class CodeOwnersInternalServerErrorExceptionTest extends AbstractCodeOwnersTest {
- @Test
- public void codeOwnersInternalServerErrorExceptionIsCreatedByDefault() {
- assertThat(newInternalServerError("foo", new NullPointerException("bar")))
- .isInstanceOf(CodeOwnersInternalServerErrorException.class);
- assertThat(
- newInternalServerError("foo", newExceptionWithCause(new NullPointerException("bar"))))
- .isInstanceOf(CodeOwnersInternalServerErrorException.class);
- }
-
- @Test
- public void storageExceptionIsCreatedForNonCodeOwnerErrors() {
- assertThat(newInternalServerError("foo", new DiffNotAvailableException("bar")))
- .isInstanceOf(StorageException.class);
- assertThat(
- newInternalServerError(
- "foo", newExceptionWithCause(new DiffNotAvailableException("bar"))))
- .isInstanceOf(StorageException.class);
- }
-
- private Exception newExceptionWithCause(Exception cause) {
- return new Exception("exception1", new Exception("exception2", cause));
- }
-}
