Avoid using Optionals as method parameters
As per the recommended guidelines [1], using Optional in method
parameters is discouraged. This change replaces such usages with more
appropriate alternatives
[1] https://gerrit-review.googlesource.com/Documentation/dev-crafting-changes.html#:~:text=to%20local%20variables-,Optional,-/%20Nullable
Change-Id: Ib80e35da25d652cdc2329cbf7c719fdb0da402ec
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameCommand.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameCommand.java
index 3e2c796..5ea32f6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameCommand.java
@@ -32,7 +32,6 @@
import java.io.InputStreamReader;
import java.util.List;
import java.util.NoSuchElementException;
-import java.util.Optional;
import org.kohsuke.args4j.Argument;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -62,11 +61,10 @@
input.name = newProjectName;
ProjectResource rsrc = new ProjectResource(projectState, self.get());
try (CommandProgressMonitor monitor = new CommandProgressMonitor(stdout)) {
- Optional<ProgressMonitor> optionalProgressMonitor = Optional.of(monitor);
- renameProject.assertCanRename(rsrc, input, optionalProgressMonitor);
- List<Change.Id> changeIds = renameProject.getChanges(rsrc, optionalProgressMonitor);
+ renameProject.assertCanRename(rsrc, input, monitor);
+ List<Change.Id> changeIds = renameProject.getChanges(rsrc, monitor);
if (!renameProject.startRename(
- rsrc, input, Optional.of(monitor), continueRename(changeIds, monitor), changeIds)) {
+ rsrc, input, monitor, continueRename(changeIds, monitor), changeIds)) {
stdout.flush();
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java
index b563109..aa85235 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java
@@ -50,14 +50,15 @@
import com.googlesource.gerrit.plugins.renameproject.database.DatabaseRenameHandler;
import com.googlesource.gerrit.plugins.renameproject.database.IndexUpdateHandler;
import com.googlesource.gerrit.plugins.renameproject.fs.FilesystemRenameHandler;
+import com.googlesource.gerrit.plugins.renameproject.monitor.NoopMonitor;
import com.googlesource.gerrit.plugins.renameproject.monitor.ProgressMonitor;
import java.io.IOException;
import java.io.OutputStream;
import java.net.URISyntaxException;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
-import java.util.Optional;
import java.util.Set;
import org.apache.http.auth.AuthenticationException;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -77,13 +78,13 @@
InterruptedException,
ConfigInvalidException,
RenameRevertException {
- Optional<ProgressMonitor> optionalProgressMonitor = Optional.empty();
- assertCanRename(resource, input, optionalProgressMonitor);
- List<Id> changeIds = getChanges(resource, optionalProgressMonitor);
+ ProgressMonitor progressMonitor = NoopMonitor.INSTANCE;
+ assertCanRename(resource, input, progressMonitor);
+ List<Id> changeIds = getChanges(resource, progressMonitor);
if (startRename(
resource,
input,
- Optional.empty(),
+ progressMonitor,
(changeIds == null || changeIds.size() <= WARNING_LIMIT || input.continueWithRename),
changeIds)) {
return Response.ok("");
@@ -94,7 +95,7 @@
public boolean startRename(
ProjectResource resource,
Input input,
- Optional<ProgressMonitor> optionalProgressMonitor,
+ ProgressMonitor progressMonitor,
boolean continueRename,
List<Change.Id> changeIds)
throws ResourceConflictException,
@@ -106,13 +107,13 @@
InterruptedException {
if (!isReplica) {
if (continueRename) {
- doRename(Optional.ofNullable(changeIds), resource, input, optionalProgressMonitor);
+ doRename(changeIds, resource, input, progressMonitor);
} else {
log.debug(CANCELLATION_MSG);
return false;
}
} else {
- doRename(Optional.empty(), resource, input, Optional.empty());
+ doRename(Collections.emptyList(), resource, input, NoopMonitor.INSTANCE);
}
return true;
}
@@ -245,10 +246,10 @@
return true;
}
- void assertCanRename(ProjectResource rsrc, Input input, Optional<ProgressMonitor> opm)
+ void assertCanRename(ProjectResource rsrc, Input input, ProgressMonitor pm)
throws ResourceConflictException, BadRequestException, AuthException {
try {
- opm.ifPresent(progressMonitor -> progressMonitor.beginTask("Checking preconditions"));
+ pm.beginTask("Checking preconditions");
assertNewNameNotNull(input);
assertNewNameMatchesRegex(input);
assertRenamePermission(rsrc);
@@ -261,40 +262,36 @@
}
}
- void doRename(
- Optional<List<Change.Id>> opChangeIds,
- ProjectResource rsrc,
- Input input,
- Optional<ProgressMonitor> opm)
+ void doRename(List<Change.Id> changeIds, ProjectResource rsrc, Input input, ProgressMonitor pm)
throws InterruptedException, ConfigInvalidException, IOException, RenameRevertException {
Project.NameKey oldProjectKey = rsrc.getNameKey();
Project.NameKey newProjectKey = Project.nameKey(input.name);
Exception ex = null;
stepsPerformed.clear();
try {
- fsRenameStep(oldProjectKey, newProjectKey, opm);
+ fsRenameStep(oldProjectKey, newProjectKey, pm);
if (!isReplica) {
cacheRenameStep(rsrc.getNameKey(), newProjectKey);
List<Change.Id> updatedChangeIds =
- dbRenameStep(opChangeIds.get(), oldProjectKey, newProjectKey, opm);
+ dbRenameStep(changeIds, oldProjectKey, newProjectKey, pm);
// if the DB update is successful, update the secondary index
- indexRenameStep(updatedChangeIds, oldProjectKey, newProjectKey, opm);
+ indexRenameStep(updatedChangeIds, oldProjectKey, newProjectKey, pm);
// no need to revert this since newProjectKey will be removed from project cache before
lockUnlockProject.unlock(newProjectKey);
log.debug("Unlocked the repo {} after rename operation.", newProjectKey.get());
// flush old changeId -> Project cache for given changeIds
- changeIdProjectCache.invalidateAll(opChangeIds.get());
+ changeIdProjectCache.invalidateAll(changeIds);
pluginEvent.fire(pluginName, pluginName, oldProjectKey.get() + ":" + newProjectKey.get());
// replicate rename-project operation to other replica instances
- replicateRename(sshHelper, httpSession, input, oldProjectKey, opm);
+ replicateRename(sshHelper, httpSession, input, oldProjectKey, pm);
}
} catch (Exception e) {
if (stepsPerformed.isEmpty()) {
@@ -307,7 +304,7 @@
}
try {
revertRenameProject.performRevert(
- stepsPerformed, opChangeIds.get(), oldProjectKey, newProjectKey, opm);
+ stepsPerformed, changeIds, oldProjectKey, newProjectKey, pm);
} catch (Exception revertEx) {
log.error(
"Failed to revert renaming procedure for {}. Exception caught: {}",
@@ -324,9 +321,9 @@
}
void fsRenameStep(
- Project.NameKey oldProjectKey, Project.NameKey newProjectKey, Optional<ProgressMonitor> opm)
+ Project.NameKey oldProjectKey, Project.NameKey newProjectKey, ProgressMonitor pm)
throws IOException {
- fsHandler.rename(oldProjectKey, newProjectKey, opm);
+ fsHandler.rename(oldProjectKey, newProjectKey, pm);
logPerformedStep(Step.FILESYSTEM, newProjectKey, oldProjectKey);
}
@@ -340,9 +337,9 @@
List<Change.Id> changeIds,
Project.NameKey oldProjectKey,
Project.NameKey newProjectKey,
- Optional<ProgressMonitor> opm)
+ ProgressMonitor pm)
throws IOException, ConfigInvalidException, RenameRevertException {
- List<Change.Id> updatedChangeIds = dbHandler.rename(changeIds, newProjectKey, opm);
+ List<Change.Id> updatedChangeIds = dbHandler.rename(changeIds, newProjectKey, pm);
logPerformedStep(Step.DATABASE, newProjectKey, oldProjectKey);
return updatedChangeIds;
}
@@ -351,9 +348,9 @@
List<Change.Id> updatedChangeIds,
Project.NameKey oldProjectKey,
Project.NameKey newProjectKey,
- Optional<ProgressMonitor> opm)
+ ProgressMonitor pm)
throws InterruptedException {
- indexHandler.updateIndex(updatedChangeIds, newProjectKey, opm);
+ indexHandler.updateIndex(updatedChangeIds, newProjectKey, pm);
logPerformedStep(Step.INDEX, newProjectKey, oldProjectKey);
}
@@ -387,9 +384,8 @@
return stepsPerformed;
}
- List<Change.Id> getChanges(ProjectResource rsrc, Optional<ProgressMonitor> opm)
- throws IOException {
- opm.ifPresent(pm -> pm.beginTask("Retrieving the list of changes from DB"));
+ List<Change.Id> getChanges(ProjectResource rsrc, ProgressMonitor pm) throws IOException {
+ pm.beginTask("Retrieving the list of changes from DB");
Project.NameKey oldProjectKey = rsrc.getNameKey();
return dbHandler.getChangeIds(oldProjectKey);
}
@@ -399,12 +395,9 @@
HttpSession httpSession,
Input input,
Project.NameKey oldProjectKey,
- Optional<ProgressMonitor> opm) {
- opm.ifPresent(
- pm ->
- pm.beginTask(
- String.format(
- "Replicating the rename of %s to %s", oldProjectKey.get(), input.name)));
+ ProgressMonitor pm) {
+ pm.beginTask(
+ String.format("Replicating the rename of %s to %s", oldProjectKey.get(), input.name));
Set<String> urls = cfg.getUrls();
int nbRetries = cfg.getRenameReplicationRetries();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProject.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProject.java
index 4dbf637..299bbf7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProject.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProject.java
@@ -27,7 +27,6 @@
import java.io.IOException;
import java.util.Collections;
import java.util.List;
-import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -57,13 +56,13 @@
List<Id> changeIds,
Project.NameKey oldProjectKey,
Project.NameKey newProjectKey,
- Optional<ProgressMonitor> opm)
+ ProgressMonitor pm)
throws IOException, RenameRevertException, ConfigInvalidException {
- opm.ifPresent(pm -> pm.beginTask("Reverting the rename procedure."));
+ pm.beginTask("Reverting the rename procedure.");
List<Change.Id> updatedChangeIds = Collections.emptyList();
if (stepsPerformed.contains(Step.FILESYSTEM)) {
try {
- fsHandler.rename(newProjectKey, oldProjectKey, opm);
+ fsHandler.rename(newProjectKey, oldProjectKey, pm);
log.debug("Reverted the git repo name to {} successfully.", oldProjectKey.get());
} catch (IOException e) {
log.error(
@@ -77,7 +76,7 @@
}
if (stepsPerformed.contains(Step.DATABASE)) {
try {
- updatedChangeIds = dbHandler.rename(changeIds, newProjectKey, opm);
+ updatedChangeIds = dbHandler.rename(changeIds, newProjectKey, pm);
log.debug(
"Reverted the changes in DB successfully from project {} to project {}.",
newProjectKey.get(),
@@ -93,7 +92,7 @@
}
if (stepsPerformed.contains(Step.INDEX)) {
try {
- indexHandler.updateIndex(updatedChangeIds, oldProjectKey, opm);
+ indexHandler.updateIndex(updatedChangeIds, oldProjectKey, pm);
log.debug(
"Reverted the secondary index successfully from project {} to project {}.",
newProjectKey.get(),
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/DatabaseRenameHandler.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/DatabaseRenameHandler.java
index 87e6ba2..3800875 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/DatabaseRenameHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/DatabaseRenameHandler.java
@@ -39,7 +39,6 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -92,9 +91,9 @@
}
public List<Change.Id> rename(
- List<Change.Id> changes, Project.NameKey newProjectKey, Optional<ProgressMonitor> opm)
+ List<Change.Id> changes, Project.NameKey newProjectKey, ProgressMonitor pm)
throws RenameRevertException, IOException, ConfigInvalidException {
- opm.ifPresent(pm -> pm.beginTask("Updating changes in the database"));
+ pm.beginTask("Updating changes in the database");
log.debug("Updating the changes in noteDb related to project {}", oldProjectKey.get());
try {
updateWatchEntries(newProjectKey);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/IndexUpdateHandler.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/IndexUpdateHandler.java
index 81fa156..5b01391 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/IndexUpdateHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/IndexUpdateHandler.java
@@ -24,7 +24,6 @@
import com.googlesource.gerrit.plugins.renameproject.monitor.ProgressMonitor;
import java.util.ArrayList;
import java.util.List;
-import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
@@ -47,17 +46,17 @@
}
public void updateIndex(
- List<Change.Id> changeIds, Project.NameKey newProjectKey, Optional<ProgressMonitor> opm)
+ List<Change.Id> changeIds, Project.NameKey newProjectKey, ProgressMonitor pm)
throws InterruptedException {
log.debug("Starting to index {} change(s).", changeIds.size());
ExecutorService executor =
Executors.newFixedThreadPool(
config.getIndexThreads(),
new ThreadFactoryBuilder().setNameFormat("Rename-Index-%d").build());
- opm.ifPresent(pm -> pm.beginTask("Indexing changes", changeIds.size()));
+ pm.beginTask("Indexing changes", changeIds.size());
List<Callable<Boolean>> callableTasks = new ArrayList<>(changeIds.size());
for (Change.Id id : changeIds) {
- callableTasks.add(new IndexTask(id, newProjectKey, opm));
+ callableTasks.add(new IndexTask(id, newProjectKey, pm));
}
List<Future<Boolean>> tasksCompleted = executor.invokeAll(callableTasks);
executor.shutdown();
@@ -84,10 +83,9 @@
private Change.Id changeId;
private Project.NameKey newProjectKey;
- private Optional<ProgressMonitor> monitor;
+ private ProgressMonitor monitor;
- IndexTask(
- Change.Id changeId, Project.NameKey newProjectKey, Optional<ProgressMonitor> monitor) {
+ IndexTask(Change.Id changeId, Project.NameKey newProjectKey, ProgressMonitor monitor) {
this.changeId = changeId;
this.newProjectKey = newProjectKey;
this.monitor = monitor;
@@ -96,7 +94,7 @@
@Override
public Boolean call() throws Exception {
indexer.index(newProjectKey, changeId);
- monitor.ifPresent(monitor -> monitor.update(1));
+ monitor.update(1);
return Boolean.TRUE;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/fs/FilesystemRenameHandler.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/fs/FilesystemRenameHandler.java
index aeff091..8b0b8db 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/fs/FilesystemRenameHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/fs/FilesystemRenameHandler.java
@@ -26,7 +26,6 @@
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.util.Comparator;
-import java.util.Optional;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Repository;
@@ -46,12 +45,12 @@
}
public void rename(
- Project.NameKey oldProjectKey, Project.NameKey newProjectKey, Optional<ProgressMonitor> opm)
+ Project.NameKey oldProjectKey, Project.NameKey newProjectKey, ProgressMonitor pm)
throws IOException, RepositoryNotFoundException {
try (Repository repository = repoManager.openRepository(oldProjectKey)) {
File repoFile = repository.getDirectory();
RepositoryCache.close(repository);
- opm.ifPresent(pm -> pm.beginTask("Renaming git repository"));
+ pm.beginTask("Renaming git repository");
renameGitRepository(repoFile, newProjectKey, oldProjectKey);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/renameproject/RenameIT.java b/src/test/java/com/googlesource/gerrit/plugins/renameproject/RenameIT.java
index 60bff28..ae91162 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/renameproject/RenameIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/renameproject/RenameIT.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.renameproject.RenameProject.Input;
+import com.googlesource.gerrit.plugins.renameproject.monitor.NoopMonitor;
import java.io.IOException;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
@@ -240,8 +241,7 @@
when(sshHelper.newErrorBufferStream()).thenReturn(errStream);
when(errStream.toString()).thenReturn("");
-
- renameProject.replicateRename(sshHelper, httpSession, input, project, Optional.empty());
+ renameProject.replicateRename(sshHelper, httpSession, input, project, NoopMonitor.INSTANCE);
verify(sshHelper, atMostOnce())
.executeRemoteSsh(eq(new URIish(URL)), eq(expectedCommand), eq(errStream));
}
@@ -260,7 +260,7 @@
Input input = new Input();
input.name = NEW_PROJECT_NAME;
when(sshHelper.connect(eq(urish))).thenReturn(session);
- renameProject.replicateRename(sshHelper, httpSession, input, project, Optional.empty());
+ renameProject.replicateRename(sshHelper, httpSession, input, project, NoopMonitor.INSTANCE);
String expectedCommand = PLUGIN_NAME + " " + project.get() + " " + NEW_PROJECT_NAME;
verify(sshHelper, times(3)).executeRemoteSsh(eq(urish), eq(expectedCommand), eq(errStream));
}
@@ -280,7 +280,7 @@
when(sshHelper.newErrorBufferStream()).thenReturn(errStream);
when(errStream.toString()).thenReturn("");
- renameProject.replicateRename(sshHelper, httpSession, input, project, Optional.empty());
+ renameProject.replicateRename(sshHelper, httpSession, input, project, NoopMonitor.INSTANCE);
verify(sshHelper, never())
.executeRemoteSsh(eq(new URIish(URL)), eq(expectedCommand), eq(errStream));
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProjectTest.java b/src/test/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProjectTest.java
index 8330748..b7d4ebd 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProjectTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProjectTest.java
@@ -46,7 +46,7 @@
private RevertRenameProject revertRenameProject;
private Project.NameKey oldProjectKey;
private Project.NameKey newProjectKey;
- private Optional<ProgressMonitor> pm;
+ private ProgressMonitor pm;
private ProjectResource oldRsrc;
@Before
@@ -57,7 +57,7 @@
oldProjectKey = project;
newProjectKey = Project.nameKey(NEW_PROJECT_NAME);
- pm = Optional.of(Mockito.mock(ProgressMonitor.class));
+ pm = Mockito.mock(ProgressMonitor.class);
oldRsrc = Mockito.mock(ProjectResource.class);
when(oldRsrc.getNameKey()).thenReturn(oldProjectKey);
