OAK-11774 : removed usage of guava splitter by rishabhdaim · Pull Request #2380 · apache/jackrabbit-oak

rishabhdaim · 2025-07-10T05:13:32Z

No description provided.

github-actions · 2025-07-10T05:13:58Z

Commit-Check ✔️

reschke · 2025-07-10T13:59:06Z

...-lucene/src/main/java/org/apache/jackrabbit/oak/scalability/suites/ScalabilityNodeSuite.java

-    protected static final List<String> NODE_LEVELS = Splitter.on(",").trimResults()
-            .omitEmptyStrings().splitToList(System.getProperty("nodeLevels", "10,5,2"));
+    protected static final List<String> NODE_LEVELS = Arrays.stream(System.getProperty("nodeLevels", "10,5,2").split(","))
+            .map(String::trim)


Of course the above should be refactored to be there just once, and to use the SystemPropertySupplies, but that would be a different ticket.

reschke · 2025-07-10T14:01:34Z

...ins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedDataStoreUtils.java

-                Splitter.on(DELIM).limit(2).splitToList(name).get(1)).get(0);
+            return Arrays.stream(name.split(DELIM, 2))
+                    .skip(1)
+                    .findFirst()


is this right as compared to "limit(2)?

name.split(DELIM, 2) would only get 2 elements.

reschke · 2025-07-10T14:02:35Z

oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java

+                        .map(String::trim)
+                        .filter(s -> !s.isEmpty())
+                        .findFirst()
+                        .orElse(null);


What's this? I assume it's ok but I do not understand :-)

I have asked the co-pilot to optimize this code, and it created this one.

oak-run/src/main/java/org/apache/jackrabbit/oak/run/DataStoreCommand.java

reschke · 2025-07-10T14:12:30Z

oak-run/src/test/java/org/apache/jackrabbit/oak/run/DataStoreCommandTest.java

                        temporaryFolder.newFolder().getAbsolutePath()));
        if (!StringUtils.isEmpty(additionalParams)) {
-            argsList.addAll(Splitter.on(" ").splitToList(additionalParams));
+            argsList.addAll(Arrays.stream(additionalParams.split(" ")).collect(Collectors.toList()));


that's a SP, right?

Space character.

reschke

(see comments)

...ent/src/main/java/org/apache/jackrabbit/oak/plugins/document/memory/MemoryDocumentStore.java

oak-run/src/main/java/org/apache/jackrabbit/oak/run/MetricsExporterFixtureProvider.java

mbaedke · 2025-07-15T11:20:21Z

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/JournalReader.java

+                        splits = Arrays.stream(line.split("")).map(String::trim).collect(Collectors.toList());
+                    } else {
+                        splits = Arrays.stream(line.split(" ")).collect(Collectors.toList());
+                    }


I do not think that the code does still the same in case the trimmed line is empty.

I quickly tried it in a Java console and the resulting arrays are of different length.

thanks for @mbaedke, could you please the test cases that you tried.
Also, this class has extensive test coverage and they all passed after these changes, but still we might have missed few cases, would be more than happy to add them.

This class must parse a file that contains either three empty spaces on a line or a line with three entries separated by a space. Moreover, the unit cases provide extensive coverage for that.

Well, I tried the following in a JShell console in Idea;

import org.apache.jackrabbit.guava.common.base.Splitter; import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; String separator = " "; String input = " "; List<String> guavaList = Splitter.on(separator).splitToList(input); System.out.println("Guava: " + guavaList.size()); List<String> jseList = Arrays.stream(" ".split("")).map(String::trim).collect(Collectors.toList()); System.out.println("Stream API: " + jseList.size());

The output was:

import org.apache.jackrabbit.guava.common.base.Splitter import java.util.Arrays import java.util.List import java.util.stream.Collectors field String separator = " " Defined field String input = " " Defined field List<String> guavaList = [, , , ] System.out.println("Guava: " + guavaList.size()); Guava: 4 Defined field List<String> jseList = [, , ] System.out.println("Stream API: " + jseList.size()); Stream API: 3

Since the lists are of different length, I didn't look further.

Thank @mbaedke for the review, the issue has been fixed in d7580c7

mbaedke · 2025-07-15T11:53:47Z

.../java/org/apache/jackrabbit/oak/scalability/suites/ScalabilityNodeRelationshipSuiteTest.java

I don't understand. Wrong PR?

they are added to improve the test coverage.

Yes, so something completely different :)

mbaedke

See inline comments.

…de using -1 limit in split method

sonarqubecloud · 2025-07-17T14:21:34Z

Quality Gate failed

Failed conditions
77.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

OAK-11774 : removed usage of guava splitter

b9a23bf

rishabhdaim requested review from dulceanu, mbaedke and reschke July 10, 2025 05:13

rishabhdaim requested a review from thomasmueller July 10, 2025 05:14

OAK-11774 : improved test coverage

372705b

reschke reviewed Jul 10, 2025

View reviewed changes

rishabhdaim requested a review from reschke July 10, 2025 15:05

reschke reviewed Jul 11, 2025

View reviewed changes

rishabhdaim requested a review from reschke July 11, 2025 12:26

mbaedke reviewed Jul 15, 2025

View reviewed changes

...ent/src/main/java/org/apache/jackrabbit/oak/plugins/document/memory/MemoryDocumentStore.java Show resolved Hide resolved

mbaedke reviewed Jul 15, 2025

View reviewed changes

oak-run/src/main/java/org/apache/jackrabbit/oak/run/MetricsExporterFixtureProvider.java Show resolved Hide resolved

mbaedke reviewed Jul 15, 2025

View reviewed changes

mbaedke requested changes Jul 15, 2025

View reviewed changes

rishabhdaim requested a review from mbaedke July 16, 2025 05:27

OAK-11774 : mimic exact behaviour of Guava's Splitter via Stream's co…

d7580c7

…de using -1 limit in split method

mbaedke approved these changes Jul 17, 2025

View reviewed changes

rishabhdaim merged commit 06b7ff2 into trunk Jul 17, 2025
4 of 5 checks passed

rishabhdaim deleted the OAK-11774 branch July 17, 2025 15:14

Conversation

rishabhdaim commented Jul 10, 2025

Uh oh!

github-actions bot commented Jul 10, 2025

Commit-Check ✔️

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reschke Jul 10, 2025 • 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!

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!

reschke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

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!

mbaedke left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jul 17, 2025

Quality Gate failed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reschke Jul 10, 2025 •

edited

Loading