OAK-11024 : removed usage of Sets.newHashSet by rishabhdaim · Pull Request #1678 · apache/jackrabbit-oak

rishabhdaim · 2024-09-02T06:04:08Z

No description provided.

reschke

some comments; will do full review later on

reschke · 2024-09-05T09:56:27Z

.../apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncHandlerTest.java


        // membership-nesting is 1 => expect only 'USER_ID' plus the declared group-membership
-        Set<String> expected = Sets.newHashSet(USER_ID);
+        Set<String> expected = new HashSet<>();


maybe:

Set expected = Stream.of(USER_ID).collect(Collectors.toSet());

?

reschke · 2024-09-05T10:01:38Z

...b-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java


-        Set<String> retrieved = newHashSet(Iterables.transform(newHashSet(dataStore.getAllRecords()),
-            input -> input.getIdentifier().toString()));
+        Set<DataRecord> recordSet = new HashSet<>();


Maybe

CollectionUtils.toSet(...)

?

reschke · 2024-09-05T10:02:04Z

...b-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java


-        Set<String> retrieved = newHashSet(Iterables.transform(newHashSet(dataStore.getAllRecords()),
-                input -> input.getIdentifier().toString()));
+        Set<DataRecord> recordSet = new HashSet<>();


reschke · 2024-09-05T10:02:22Z

...b-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java

        }

-        Set<DataRecord> retrieved = newHashSet((dataStore.getAllRecords()));
+        Set<DataRecord> retrieved = new HashSet<>();


rishabhdaim · 2024-09-10T05:16:01Z

@reschke @mbaedke Updated this PR with utils from oak-commons. Could you please review and share feedback?

reschke

Thanks a lot :-)

I believe you missed a few cases where you touched StreamSupport (maybe just because of removing static imports) where we could now use CollectionUtils
furthermore it would be good to converge on a strategy whether Collectors.something should be a static import (right now it seems to me mixed)

reschke · 2024-09-10T13:01:13Z

oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/Namespaces.java

        // that's not stored along with the other mappings
-        Set<String> prefixes = newHashSet("");
-        Set<String> uris = newHashSet("");
+        Set<String> prefixes = new HashSet<>();


Use CollectionUtils?

reschke · 2024-09-10T13:03:03Z

...st/java/org/apache/jackrabbit/oak/security/authorization/composite/LimitedScopeProvider.java

 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;

+


reschke · 2024-09-10T13:05:45Z

...java/org/apache/jackrabbit/oak/plugins/index/lucene/ActiveDeletedBlobCollectorMBeanImpl.java

    private boolean waitForRunningIndexCycles() {
        Map<IndexStatsMBean, Long> origIndexLaneToExecutinoCountMap = Maps.asMap(
-                Sets.newHashSet(StreamSupport.stream(asyncIndexInfoService.getAsyncLanes().spliterator(), false)
+                new HashSet<>(StreamSupport.stream(asyncIndexInfoService.getAsyncLanes().spliterator(), false)


use CollectionUtils?

I don't think this would give us any additional benefit.

reschke · 2024-09-10T13:11:01Z

oak-run/src/main/java/org/apache/jackrabbit/oak/index/IndexDefinitionUpdater.java


    private List<String> getReindexIndexPaths() {
-        return stream(store.getRoot().getChildNode(INDEX_DEFINITIONS_NAME).getChildNodeEntries().spliterator(), false)
+        return StreamSupport.stream(store.getRoot().getChildNode(INDEX_DEFINITIONS_NAME).getChildNodeEntries().spliterator(), false)


Use CollectionUtils?

It is returning List not Set

reschke · 2024-09-10T13:11:47Z

oak-run/src/main/java/org/apache/jackrabbit/oak/index/IndexDefinitionUpdater.java

                val.append(" = {").append(v).append("}");
            } else if (ps.getType() == BINARIES) {
-                String v = stream(ps.getValue(BINARIES).spliterator(), false)
+                String v = StreamSupport.stream(ps.getValue(BINARIES).spliterator(), false)


CollectionUtils?

reschke · 2024-09-10T13:14:16Z

...c/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/Permissions.java

-import java.util.stream.Collectors;
+
+import static java.util.stream.Collectors.toSet;



We should agree on a consistent strategy whether the Collector imports should be static (I'm leaning towards "no")..

reschke · 2024-09-10T13:19:11Z

...tore-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java

     */
    public Iterable<NodeDocument> getPossiblyDeletedDocs(final long fromModified, final long toModified) {
-        return stream(getSelectedDocuments(store, NodeDocument.DELETED_ONCE, 1).spliterator(), false)
+        return StreamSupport.stream(getSelectedDocuments(store, NodeDocument.DELETED_ONCE, 1).spliterator(), false)


CollectionUtils?

reschke · 2024-09-10T13:19:47Z

...tore-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCSupport.java

                                                  @NotNull final Set<String> excludePaths) {
        // (_modified = fromModified && _id > fromId || _modified > fromModified && _modified < toModified)
-        final Stream<NodeDocument> s1 = stream(getSelectedDocuments(store,
+        final Stream<NodeDocument> s1 = StreamSupport.stream(getSelectedDocuments(store,


CollectionUtils?

rishabhdaim · 2024-09-10T16:37:42Z

thanks for the review @reschke. I only changed where we are using Guava's newHashSet.

Let's open separate Jiras for other changes instead of piggybacking this one.

For now, I will merge this PR.

reschke · 2024-09-10T17:47:28Z

Works for me; at some point we need final cleanup sweep anyway.

rishabhdaim requested a review from reschke September 2, 2024 06:04

rishabhdaim force-pushed the OAK-11024 branch from 2fda374 to 61319de Compare September 5, 2024 08:47

reschke reviewed Sep 5, 2024

View reviewed changes

rishabhdaim force-pushed the OAK-11024 branch 10 times, most recently from 30a5630 to 3db5fe3 Compare September 10, 2024 03:51

rishabhdaim requested a review from reschke September 10, 2024 05:07

rishabhdaim requested a review from mbaedke September 10, 2024 05:16

Rishabh Kumar added 4 commits September 10, 2024 17:12

OAK-11024 : removed usage of Sets.newHashSet

2a274ff

OAK-11024 : used oak-commons method to convert iterable/iterator to set

360188b

OAK-11024 : used oak-commons method to convert arrays/varargs to set

e7d6367

OAK-11024 : fixed unit cases

71c627a

reschke approved these changes Sep 10, 2024

View reviewed changes

OAK-11024 : addressed review comments

041e4ca

rishabhdaim force-pushed the OAK-11024 branch from fd911f3 to 041e4ca Compare September 10, 2024 16:42

rishabhdaim merged commit c6bd519 into trunk Sep 10, 2024

rishabhdaim deleted the OAK-11024 branch September 10, 2024 17:39

		import org.jetbrains.annotations.NotNull;
		import org.jetbrains.annotations.Nullable;

		import java.util.stream.Collectors;

		import static java.util.stream.Collectors.toSet;

Conversation

rishabhdaim commented Sep 2, 2024

Uh oh!

reschke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rishabhdaim commented Sep 10, 2024

Uh oh!

reschke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rishabhdaim Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rishabhdaim commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reschke commented Sep 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

rishabhdaim Sep 10, 2024 •

edited

Loading

rishabhdaim commented Sep 10, 2024 •

edited

Loading