Replace use of static EMPTY ReadOnlyPathOwnersEntry with Optional<> The use of EMPTY PathOwnersEntry with everything empty inside was an artificial simulation of an optional which made the code more complex to understand and also more inefficient. Just skip the evaluation of owners when there are no OWNERS configuration, making the code clearer and faster. Change-Id: I99acdfd03672d2ff67fa03ecf4d168f95d4b1cf9
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 ce5834c..236633d 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
@@ -235,7 +235,7 @@ // Using a `map` would have needed a try/catch inside the lamba, resulting in more code List<ReadOnlyPathOwnersEntry> parentsPathOwnersEntries = getPathOwnersEntries(parentProjectsNames, RefNames.REFS_CONFIG, cache); - ReadOnlyPathOwnersEntry projectEntry = + Optional<ReadOnlyPathOwnersEntry> projectEntry = getPathOwnersEntryOrEmpty(project, repository, RefNames.REFS_CONFIG, cache); PathOwnersEntry rootEntry = getPathOwnersEntryOrNew(project, repository, branch, cache); @@ -247,7 +247,7 @@ project, path, branch, - projectEntry, + projectEntry.orElse(null), parentsPathOwnersEntries, rootEntry, entries, @@ -294,20 +294,20 @@ ImmutableList.Builder<ReadOnlyPathOwnersEntry> pathOwnersEntries = ImmutableList.builder(); for (Project.NameKey projectName : projectNames) { try (Repository repo = repositoryManager.openRepository(projectName)) { - pathOwnersEntries = - pathOwnersEntries.add( - getPathOwnersEntryOrEmpty(projectName.get(), repo, branch, cache)); + Optional<ReadOnlyPathOwnersEntry> pathOwnersEntry = + getPathOwnersEntryOrEmpty(projectName.get(), repo, branch, cache); + if (pathOwnersEntry.isPresent()) { + pathOwnersEntries = pathOwnersEntries.add(pathOwnersEntry.get()); + } } } return pathOwnersEntries.build(); } - private ReadOnlyPathOwnersEntry getPathOwnersEntryOrEmpty( + private Optional<ReadOnlyPathOwnersEntry> getPathOwnersEntryOrEmpty( String project, Repository repo, String branch, PathOwnersEntriesCache cache) throws InvalidOwnersFileException, ExecutionException { - return getPathOwnersEntry(project, repo, branch, cache) - .map(v -> (ReadOnlyPathOwnersEntry) v) - .orElse(PathOwnersEntry.EMPTY); + return getPathOwnersEntry(project, repo, branch, cache).map(v -> (ReadOnlyPathOwnersEntry) v); } private PathOwnersEntry getPathOwnersEntryOrNew( @@ -389,9 +389,9 @@ String project, String path, String branch, - ReadOnlyPathOwnersEntry projectEntry, + @Nullable ReadOnlyPathOwnersEntry projectEntry, List<ReadOnlyPathOwnersEntry> parentsPathOwnersEntries, - PathOwnersEntry rootEntry, + @Nullable PathOwnersEntry rootEntry, Map<String, PathOwnersEntry> entries, PathOwnersEntriesCache cache) throws InvalidOwnersFileException, ExecutionException { @@ -400,13 +400,13 @@ StringBuilder builder = new StringBuilder(); // Inherit from Project if OWNER in root enables inheritance - if (rootEntry.isInherited()) { + if (rootEntry == null || rootEntry.isInherited()) { calculateCurrentEntry(projectEntry, currentEntry); } // Inherit from Parent Project if OWNER in Project enables inheritance for (ReadOnlyPathOwnersEntry parentPathOwnersEntry : parentsPathOwnersEntries) { - if (projectEntry.isInherited()) { + if (projectEntry == null || projectEntry.isInherited()) { calculateCurrentEntry(parentPathOwnersEntry, currentEntry); } } @@ -459,7 +459,11 @@ } private void calculateCurrentEntry( - ReadOnlyPathOwnersEntry projectEntry, PathOwnersEntry currentEntry) { + @Nullable ReadOnlyPathOwnersEntry projectEntry, PathOwnersEntry currentEntry) { + if (projectEntry == null) { + return; + } + for (Matcher matcher : projectEntry.getMatchers().values()) { if (!currentEntry.hasMatcher(matcher.getPath())) { currentEntry.addMatcher(matcher);
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java index 8e4c53c..8633461 100644 --- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java +++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
@@ -32,8 +32,6 @@ * <p>Used internally by PathOwners to represent and compute the owners at a specific path. */ class PathOwnersEntry extends ReadOnlyPathOwnersEntry { - static final ReadOnlyPathOwnersEntry EMPTY = new ReadOnlyPathOwnersEntry(true) {}; - public PathOwnersEntry() { super(true); }