You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "risdenk (via GitHub)" <gi...@apache.org> on 2023/03/16 18:10:11 UTC
[GitHub] [solr] risdenk opened a new pull request, #1466: WIP Remove Guava usages
risdenk opened a new pull request, #1466:
URL: https://github.com/apache/solr/pull/1466
https://issues.apache.org/jira/browse/SOLR-XXXXX
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1466:
URL: https://github.com/apache/solr/pull/1466#issuecomment-1476261941
ugh - [890a6cc](https://github.com/apache/solr/pull/1466/commits/890a6cca9ff0a48022338e1c2448797801a482d1) broke some tests. I need to figure out which change did it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1142197268
##########
solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java:
##########
@@ -422,6 +423,7 @@ private Collection<String> deleteObjects(Collection<String> paths) throws S3Exce
* @param batchSize number of deletes to send to S3 at a time
*/
@VisibleForTesting
+ @SuppressForbidden(reason = "Lists.partition")
Review Comment:
I'd love to use Java Streams of this but doesn't seem to be a good fit. Will look at it again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144018575
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1558,7 +1556,7 @@ Solution is q(k) = 1/2 (k^2+k+2)
public Builder<E, M> addSubset(Collection<E> subset, M matchValue) {
if (!subset.isEmpty()) {
TrieSubsetMatcher.Node<E, M> node = root;
- for (E e : ImmutableSortedSet.copyOf(subset)) {
+ for (E e : Collections.unmodifiableSortedSet(new TreeSet<>(subset))) {
Review Comment:
foreach doesn't allow setting assignment from within the lambda and must be effectively final. The logic `node = node.getOrCreateChild(e);` therefore can't be done in the stream at least from what I can see.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1142197268
##########
solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java:
##########
@@ -422,6 +423,7 @@ private Collection<String> deleteObjects(Collection<String> paths) throws S3Exce
* @param batchSize number of deletes to send to S3 at a time
*/
@VisibleForTesting
+ @SuppressForbidden(reason = "Lists.partition")
Review Comment:
I'd love to use Java Streams for this but doesn't seem to be a good fit. Will look at it again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144038630
##########
solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java:
##########
@@ -885,7 +884,7 @@ private static SolrInputDocument toSolrInputDocument(SolrDocument doc, IndexSche
if (!fieldArrayListCreated && doc.getFieldValue(fname) instanceof Collection) {
// previous value was array so we must return as an array even if was a single value
// array
- out.setField(fname, Lists.newArrayList(val));
+ out.setField(fname, new ArrayList<>(List.of(val)));
Review Comment:
Ok I see.
This method looks more complicated than it needs to be. I don't think this condition needs to put "val" into the "out" doc, it could probably put a new ArrayList and then at the end of the loop over values, addField will be called, and this doc will be added to the existing value. If I'm too confusing, I could try contributing this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] bruno-roustant commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "bruno-roustant (via GitHub)" <gi...@apache.org>.
bruno-roustant commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144355359
##########
solr/core/src/java/org/apache/solr/cloud/SizeLimitedDistributedMap.java:
##########
@@ -77,7 +77,7 @@ protected boolean lessThan(Long a, Long b) {
}
};
- Map<String, Long> childToModificationZxid = Maps.newHashMapWithExpectedSize(children.size());
+ Map<String, Long> childToModificationZxid = new HashMap<>(children.size());
Review Comment:
We can use org.apache.lucene.util.CollectionUtil.newHashMap(int size) to avoid resizing and rehashing during the construction.
And later with JDK 19, we will be able to use HashMap.newHashMap(int numMappings).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk merged pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk merged PR #1466:
URL: https://github.com/apache/solr/pull/1466
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144830743
##########
solr/core/src/java/org/apache/solr/cloud/SizeLimitedDistributedMap.java:
##########
@@ -77,7 +77,7 @@ protected boolean lessThan(Long a, Long b) {
}
};
- Map<String, Long> childToModificationZxid = Maps.newHashMapWithExpectedSize(children.size());
+ Map<String, Long> childToModificationZxid = new HashMap<>(children.size());
Review Comment:
I like David's idea as well. In the meantime - 30e502199996d815c2acdade8f0f3be58afc0618 adds CollectionUtil to at least handle these two cases. We can follow up and fix the other places too. I think thats a reasonable middle ground.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1142246317
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1221,7 +1217,7 @@ public Elevation(Set<BytesRef> elevatedIds, Set<BytesRef> excludedIds, String qu
includeQuery = EMPTY_QUERY;
this.elevatedIds = Collections.emptySet();
} else {
- this.elevatedIds = ImmutableSet.copyOf(elevatedIds);
+ this.elevatedIds = Set.copyOf(elevatedIds);
Review Comment:
The issue is here :( `ImmutableSet.copyOf` keeps things in order it looks like. `Set.copyOf` doesn't care.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] justinrsweeney commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "justinrsweeney (via GitHub)" <gi...@apache.org>.
justinrsweeney commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1143411837
##########
solr/core/src/java/org/apache/solr/api/Api.java:
##########
@@ -44,9 +43,7 @@ public Map<String, JsonSchemaValidator> getCommandSchema() {
if (commandSchema == null) {
ValidatingJsonMap commands = getSpec().getMap("commands", null);
commandSchema =
- commands != null
- ? ImmutableMap.copyOf(ApiBag.getParsedSchema(commands))
- : ImmutableMap.of();
+ commands != null ? Map.copyOf(ApiBag.getParsedSchema(commands)) : Map.of();
Review Comment:
Should we consider using `Collections.unmodifiableMap()` to get closer behavior to ImmutableMap? I don't think `Collection.unmodifiableMap` makes all of the same guarantees, but might make more clear that it is read only.
This seems to be true for a number of places in these changes where we go from an ImmutableCollection to just a regular Map or Set, wondering if that was intentional or if we should try to consistently keep those unmodifiable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144010766
##########
solr/core/src/java/org/apache/solr/core/backup/BackupManager.java:
##########
@@ -254,7 +254,10 @@ public void uploadConfigDir(
String sourceConfigName, String targetConfigName, ConfigSetService configSetService)
throws IOException {
URI source = repository.resolveDirectory(getZkStateDir(), CONFIG_STATE_DIR, sourceConfigName);
- Preconditions.checkState(repository.exists(source), "Path %s does not exist", source);
+ if (!repository.exists(source)) {
+ throw new IllegalArgumentException(
+ String.format(Locale.ROOT, "Path %s does not exist", source));
Review Comment:
Fixed in 2319828417e3e5bf89d478391cd3a24faee8d7ef
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -2192,6 +2191,9 @@ public SolrCore getCore(String name) {
* @see SolrCore#close()
*/
public SolrCore getCore(String name, UUID id) {
+ if (name == null) {
+ return null;
+ }
Review Comment:
I want to say this caused issues. Let me take a second look.
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1265,15 +1261,13 @@ protected Elevation mergeWith(Elevation elevation) {
return this;
}
Set<BytesRef> elevatedIds =
- ImmutableSet.<BytesRef>builder()
- .addAll(this.elevatedIds)
- .addAll(elevation.elevatedIds)
- .build();
+ Stream.concat(this.elevatedIds.stream(), elevation.elevatedIds.stream())
+ .collect(Collectors.toCollection(LinkedHashSet::new));
boolean overlappingElevatedIds =
elevatedIds.size() != (this.elevatedIds.size() + elevation.elevatedIds.size());
BooleanQuery.Builder includeQueryBuilder = new BooleanQuery.Builder();
Set<BooleanClause> clauseSet =
- (overlappingElevatedIds ? Sets.newHashSetWithExpectedSize(elevatedIds.size()) : null);
+ (overlappingElevatedIds ? new HashSet<>(elevatedIds.size()) : null);
Review Comment:
I don't really think its going to matter. I looked through this when I made this change. There are so many other sets and collections created in here with not perfect sizes. It seems weird to care about this one exact size.
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1558,7 +1556,7 @@ Solution is q(k) = 1/2 (k^2+k+2)
public Builder<E, M> addSubset(Collection<E> subset, M matchValue) {
if (!subset.isEmpty()) {
TrieSubsetMatcher.Node<E, M> node = root;
- for (E e : ImmutableSortedSet.copyOf(subset)) {
+ for (E e : Collections.unmodifiableSortedSet(new TreeSet<>(subset))) {
Review Comment:
I wrapped in unmodifiable just to match the existing ImmutableSortedSet - new TreeSet would be mutable by itself. I don't think mutability matters here and might be easier to loop like you said. I'll take a look.
##########
solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java:
##########
@@ -885,7 +884,7 @@ private static SolrInputDocument toSolrInputDocument(SolrDocument doc, IndexSche
if (!fieldArrayListCreated && doc.getFieldValue(fname) instanceof Collection) {
// previous value was array so we must return as an array even if was a single value
// array
- out.setField(fname, Lists.newArrayList(val));
+ out.setField(fname, new ArrayList<>(List.of(val)));
Review Comment:
Can't just create new ArrayList with a single value. Interestingly this setField requires the list to be mutable - this might be a bug by itself. I wouldn't expect this parameter needs to be mutable. I'll see if I can clean it up. I'd prefer just `List.of()` with no ArrayList.
##########
solr/modules/analytics/src/test/org/apache/solr/analytics/legacy/LegacyAbstractAnalyticsTest.java:
##########
@@ -227,18 +229,25 @@ public <T extends Comparable<T>> Long calculateMissing(ArrayList<T> list, String
}
public static SolrQueryRequest request(String... args) {
- return SolrTestCaseJ4.req(ObjectArrays.concat(BASEPARMS, args, String.class));
+ return SolrTestCaseJ4.req(
+ Stream.concat(Arrays.stream(BASEPARMS), Arrays.stream(args))
+ .collect(Collectors.toUnmodifiableList())
+ .toArray(String[]::new));
}
public static SolrQueryRequest request(String[] args, String... additional) {
- return SolrTestCaseJ4.req(ObjectArrays.concat(BASEPARMS, args, String.class), additional);
+ return SolrTestCaseJ4.req(
+ Stream.concat(Arrays.stream(BASEPARMS), Arrays.stream(args))
+ .collect(Collectors.toUnmodifiableList())
+ .toArray(String[]::new),
Review Comment:
Fixed in 2319828417e3e5bf89d478391cd3a24faee8d7ef
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1234,12 +1230,12 @@ public Elevation(Set<BytesRef> elevatedIds, Set<BytesRef> excludedIds, String qu
this.excludedIds = Collections.emptySet();
excludeQueries = null;
} else {
- this.excludedIds = ImmutableSet.copyOf(excludedIds);
- List<TermQuery> excludeQueriesBuilder = new ArrayList<>(excludedIds.size());
- for (BytesRef excludedId : excludedIds) {
- excludeQueriesBuilder.add(new TermQuery(new Term(queryFieldName, excludedId)));
- }
- excludeQueries = excludeQueriesBuilder.toArray(new TermQuery[0]);
+ this.excludedIds = Collections.unmodifiableSet(new LinkedHashSet<>(excludedIds));
+ this.excludeQueries =
+ this.excludedIds.stream()
+ .map(excludedId -> new TermQuery(new Term(queryFieldName, excludedId)))
+ .collect(Collectors.toUnmodifiableList())
Review Comment:
Fixed in 2319828417e3e5bf89d478391cd3a24faee8d7ef
##########
solr/core/src/test/org/apache/solr/cloud/NestedShardedAtomicUpdateTest.java:
##########
@@ -64,7 +72,10 @@ public static void beforeClass() throws Exception {
@AfterClass
public static void afterClass() throws Exception {
- IOUtils.close(clients);
+ if (clients != null) {
+ IOUtils.close(clients);
+ clients = null;
+ }
Review Comment:
Fixed in 2319828417e3e5bf89d478391cd3a24faee8d7ef
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1299,14 +1291,16 @@ protected Elevation mergeWith(Elevation elevation) {
excludedIds.size() != (this.excludedIds.size() + elevation.excludedIds.size());
if (overlappingExcludedIds) {
excludeQueries =
- ImmutableSet.<TermQuery>builder()
- .add(this.excludeQueries)
- .add(elevation.excludeQueries)
- .build()
- .toArray(new TermQuery[0]);
+ Stream.concat(
+ Arrays.stream(this.excludeQueries), Arrays.stream(elevation.excludeQueries))
+ .collect(Collectors.toUnmodifiableSet())
Review Comment:
Fixed in 2319828417e3e5bf89d478391cd3a24faee8d7ef and 735623be5eea82a70693aee39819f2baf151aa75
##########
solr/modules/analytics/src/test/org/apache/solr/analytics/legacy/LegacyAbstractAnalyticsTest.java:
##########
@@ -227,18 +229,25 @@ public <T extends Comparable<T>> Long calculateMissing(ArrayList<T> list, String
}
public static SolrQueryRequest request(String... args) {
- return SolrTestCaseJ4.req(ObjectArrays.concat(BASEPARMS, args, String.class));
+ return SolrTestCaseJ4.req(
+ Stream.concat(Arrays.stream(BASEPARMS), Arrays.stream(args))
+ .collect(Collectors.toUnmodifiableList())
+ .toArray(String[]::new));
Review Comment:
Fixed in 2319828417e3e5bf89d478391cd3a24faee8d7ef
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1299,14 +1291,16 @@ protected Elevation mergeWith(Elevation elevation) {
excludedIds.size() != (this.excludedIds.size() + elevation.excludedIds.size());
if (overlappingExcludedIds) {
excludeQueries =
- ImmutableSet.<TermQuery>builder()
- .add(this.excludeQueries)
- .add(elevation.excludeQueries)
- .build()
- .toArray(new TermQuery[0]);
+ Stream.concat(
+ Arrays.stream(this.excludeQueries), Arrays.stream(elevation.excludeQueries))
+ .collect(Collectors.toUnmodifiableSet())
+ .toArray(TermQuery[]::new);
} else {
excludeQueries =
- ObjectArrays.concat(this.excludeQueries, elevation.excludeQueries, TermQuery.class);
+ Stream.concat(
+ Arrays.stream(this.excludeQueries), Arrays.stream(elevation.excludeQueries))
+ .collect(Collectors.toUnmodifiableSet())
Review Comment:
Fixed in 2319828417e3e5bf89d478391cd3a24faee8d7ef and 735623be5eea82a70693aee39819f2baf151aa75
##########
solr/modules/analytics/src/test/org/apache/solr/analytics/legacy/facet/LegacyAbstractAnalyticsFacetTest.java:
##########
@@ -306,7 +307,10 @@ public <T extends Comparable<T>> Long calculateMissing(ArrayList<T> list, String
}
public static SolrQueryRequest request(String... args) {
- return SolrTestCaseJ4.req(ObjectArrays.concat(BASEPARMS, args, String.class));
+ return SolrTestCaseJ4.req(
+ Stream.concat(Arrays.stream(BASEPARMS), Arrays.stream(args))
+ .collect(Collectors.toUnmodifiableList())
+ .toArray(String[]::new));
Review Comment:
Fixed in 2319828417e3e5bf89d478391cd3a24faee8d7ef
##########
solr/core/src/test/org/apache/solr/cloud/NestedShardedAtomicUpdateTest.java:
##########
@@ -64,7 +72,10 @@ public static void beforeClass() throws Exception {
@AfterClass
public static void afterClass() throws Exception {
- IOUtils.close(clients);
+ if (clients != null) {
+ IOUtils.close(clients);
+ clients = null;
+ }
Review Comment:
Eh left over from debugging - will remove.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1143437955
##########
solr/core/src/java/org/apache/solr/api/Api.java:
##########
@@ -44,9 +43,7 @@ public Map<String, JsonSchemaValidator> getCommandSchema() {
if (commandSchema == null) {
ValidatingJsonMap commands = getSpec().getMap("commands", null);
commandSchema =
- commands != null
- ? ImmutableMap.copyOf(ApiBag.getParsedSchema(commands))
- : ImmutableMap.of();
+ commands != null ? Map.copyOf(ApiBag.getParsedSchema(commands)) : Map.of();
Review Comment:
This also goes for List.of and Set.of - the only case I found so far is that ImmutableSet from Guava cared about the order and that was an issue in one case. All other cases there was no difference that I found.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] justinrsweeney commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "justinrsweeney (via GitHub)" <gi...@apache.org>.
justinrsweeney commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1143485079
##########
solr/core/src/java/org/apache/solr/api/Api.java:
##########
@@ -44,9 +43,7 @@ public Map<String, JsonSchemaValidator> getCommandSchema() {
if (commandSchema == null) {
ValidatingJsonMap commands = getSpec().getMap("commands", null);
commandSchema =
- commands != null
- ? ImmutableMap.copyOf(ApiBag.getParsedSchema(commands))
- : ImmutableMap.of();
+ commands != null ? Map.copyOf(ApiBag.getParsedSchema(commands)) : Map.of();
Review Comment:
Oh, didn't realize that. That looks good then, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] bruno-roustant commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "bruno-roustant (via GitHub)" <gi...@apache.org>.
bruno-roustant commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144360207
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1265,15 +1260,13 @@ protected Elevation mergeWith(Elevation elevation) {
return this;
}
Set<BytesRef> elevatedIds =
- ImmutableSet.<BytesRef>builder()
- .addAll(this.elevatedIds)
- .addAll(elevation.elevatedIds)
- .build();
+ Stream.concat(this.elevatedIds.stream(), elevation.elevatedIds.stream())
+ .collect(Collectors.toCollection(LinkedHashSet::new));
boolean overlappingElevatedIds =
elevatedIds.size() != (this.elevatedIds.size() + elevation.elevatedIds.size());
BooleanQuery.Builder includeQueryBuilder = new BooleanQuery.Builder();
Set<BooleanClause> clauseSet =
- (overlappingElevatedIds ? Sets.newHashSetWithExpectedSize(elevatedIds.size()) : null);
+ (overlappingElevatedIds ? new HashSet<>(elevatedIds.size()) : null);
Review Comment:
We should use org.apache.lucene.util.CollectionUtil.newHashSet(int).
(and later with JDK 19 HashSet.newHashSet)
##########
solr/core/src/java/org/apache/solr/cloud/SizeLimitedDistributedMap.java:
##########
@@ -77,7 +77,7 @@ protected boolean lessThan(Long a, Long b) {
}
};
- Map<String, Long> childToModificationZxid = Maps.newHashMapWithExpectedSize(children.size());
+ Map<String, Long> childToModificationZxid = new HashMap<>(children.size());
Review Comment:
This is not equivalent to Maps.newHashMapWithExpectedSize.
In this HashMap constructor, the parameter is the initial capacity. But we should take into account the load factor (0.75) on this capacity. For example if children.size() == 10, then the HashMap will resize (and rehash) when it contains >= 7.5 entries; this means it will always resize.
Maps.newHashMapWithExpectedSize takes that into account to set the Map initial capacity to (children.size() / 0.75) + 1, ensuring an optimal capacity and no rehash during the construction.
We can use org.apache.lucene.util.CollectionUtil.newHashMap(int size).
Later with JDK 19, we will be able to use HashMap.newHashMap(int numMappings).
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1558,7 +1556,7 @@ Solution is q(k) = 1/2 (k^2+k+2)
public Builder<E, M> addSubset(Collection<E> subset, M matchValue) {
if (!subset.isEmpty()) {
TrieSubsetMatcher.Node<E, M> node = root;
- for (E e : ImmutableSortedSet.copyOf(subset)) {
+ for (E e : Collections.unmodifiableSortedSet(new TreeSet<>(subset))) {
Review Comment:
We can remove the immutability. I chose that originally just for performance with the Guava collection.
Here what is important is to 1- remove duplicates and 2- sort. Using a TreeSet corresponds, my only concern is the poor performance of TreeSet iteration. But that's ok.
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1590,10 +1587,10 @@ public int getSubsetCount() {
* Returns an iterator over all the subsets that are contained by the provided set. The returned
* iterator does not support removal.
*
- * @param set This set is copied to a new {@link ImmutableSortedSet} with natural ordering.
+ * @param set This set is copied to a new {@link SortedSet} with natural ordering.
*/
public Iterator<M> findSubsetsMatching(Collection<E> set) {
- return new MatchIterator(ImmutableSortedSet.copyOf(set));
+ return new MatchIterator(Collections.unmodifiableSortedSet(new TreeSet<>(set)));
Review Comment:
Immutability can be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144145801
##########
solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java:
##########
@@ -885,7 +884,7 @@ private static SolrInputDocument toSolrInputDocument(SolrDocument doc, IndexSche
if (!fieldArrayListCreated && doc.getFieldValue(fname) instanceof Collection) {
// previous value was array so we must return as an array even if was a single value
// array
- out.setField(fname, Lists.newArrayList(val));
+ out.setField(fname, new ArrayList<>(List.of(val)));
Review Comment:
Your PR is faithful to the PR's purpose and it works; lets merge it. Fiddling with this method too much should be separate if we want to bother.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144820135
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1558,7 +1556,7 @@ Solution is q(k) = 1/2 (k^2+k+2)
public Builder<E, M> addSubset(Collection<E> subset, M matchValue) {
if (!subset.isEmpty()) {
TrieSubsetMatcher.Node<E, M> node = root;
- for (E e : ImmutableSortedSet.copyOf(subset)) {
+ for (E e : Collections.unmodifiableSortedSet(new TreeSet<>(subset))) {
Review Comment:
Fixed in f75201354ee64df0fc739e6f128ebe0739e8b884
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1590,10 +1587,10 @@ public int getSubsetCount() {
* Returns an iterator over all the subsets that are contained by the provided set. The returned
* iterator does not support removal.
*
- * @param set This set is copied to a new {@link ImmutableSortedSet} with natural ordering.
+ * @param set This set is copied to a new {@link SortedSet} with natural ordering.
*/
public Iterator<M> findSubsetsMatching(Collection<E> set) {
- return new MatchIterator(ImmutableSortedSet.copyOf(set));
+ return new MatchIterator(Collections.unmodifiableSortedSet(new TreeSet<>(set)));
Review Comment:
Fixed in f75201354ee64df0fc739e6f128ebe0739e8b884
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144831967
##########
solr/core/src/java/org/apache/solr/cloud/SizeLimitedDistributedMap.java:
##########
@@ -77,7 +77,7 @@ protected boolean lessThan(Long a, Long b) {
}
};
- Map<String, Long> childToModificationZxid = Maps.newHashMapWithExpectedSize(children.size());
+ Map<String, Long> childToModificationZxid = new HashMap<>(children.size());
Review Comment:
> I'd like to propose that we have a separate dedicated work item that has the goal of nobody calling the integer single-arg constructor to HashMap or HashSet, as it's a trap/gotcha (with probably minor down-side, granted). We can add a utility method for creating pre-sized maps or sets.
sounds good and we can enforce this with ForbiddenAPI as well. I'll create a jira for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1142287314
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1221,7 +1217,7 @@ public Elevation(Set<BytesRef> elevatedIds, Set<BytesRef> excludedIds, String qu
includeQuery = EMPTY_QUERY;
this.elevatedIds = Collections.emptySet();
} else {
- this.elevatedIds = ImmutableSet.copyOf(elevatedIds);
+ this.elevatedIds = Set.copyOf(elevatedIds);
Review Comment:
Fixed in b2d239f651e407f7dea149e8d84f7db6424e4105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1143435765
##########
solr/core/src/java/org/apache/solr/api/Api.java:
##########
@@ -44,9 +43,7 @@ public Map<String, JsonSchemaValidator> getCommandSchema() {
if (commandSchema == null) {
ValidatingJsonMap commands = getSpec().getMap("commands", null);
commandSchema =
- commands != null
- ? ImmutableMap.copyOf(ApiBag.getParsedSchema(commands))
- : ImmutableMap.of();
+ commands != null ? Map.copyOf(ApiBag.getParsedSchema(commands)) : Map.of();
Review Comment:
Map.of and Map.copyOf return immutable maps already. https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Map.html under `Unmodifiable Maps
##########
solr/core/src/java/org/apache/solr/api/Api.java:
##########
@@ -44,9 +43,7 @@ public Map<String, JsonSchemaValidator> getCommandSchema() {
if (commandSchema == null) {
ValidatingJsonMap commands = getSpec().getMap("commands", null);
commandSchema =
- commands != null
- ? ImmutableMap.copyOf(ApiBag.getParsedSchema(commands))
- : ImmutableMap.of();
+ commands != null ? Map.copyOf(ApiBag.getParsedSchema(commands)) : Map.of();
Review Comment:
Map.of and Map.copyOf return immutable maps already. https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Map.html under "Unmodifiable Maps"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1139196654
##########
solr/modules/gcs-repository/build.gradle:
##########
@@ -39,6 +39,5 @@ dependencies {
testImplementation 'com.carrotsearch.randomizedtesting:randomizedtesting-runner'
testImplementation 'junit:junit'
- testImplementation 'com.google.guava:guava'
Review Comment:
yay
##########
solr/solrj/build.gradle:
##########
@@ -78,6 +78,5 @@ dependencies {
exclude group: "org.apache.logging.log4j", module: "log4j-api"
})
testImplementation 'org.apache.commons:commons-lang3'
- testImplementation 'com.google.guava:guava'
Review Comment:
yay
##########
solr/modules/analytics/build.gradle:
##########
@@ -28,7 +28,6 @@ dependencies {
implementation 'com.fasterxml.jackson.core:jackson-annotations'
implementation 'com.fasterxml.jackson.core:jackson-core'
implementation 'com.fasterxml.jackson.core:jackson-databind'
- implementation 'com.google.guava:guava'
Review Comment:
yay
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144014874
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -2192,6 +2191,9 @@ public SolrCore getCore(String name) {
* @see SolrCore#close()
*/
public SolrCore getCore(String name, UUID id) {
+ if (name == null) {
+ return null;
+ }
Review Comment:
Line 2211 - getCoreInitFailures can't have null keys and so looking up name is null caused issue. This just shortcircuits since null key would never match anyway. The Map.of is more strict than the existing ImmutableMap around null keys. This fixed the issue and returns the same null we would have gotten anyway (desc is null on line 2207 and then line 2221 returns null)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] mkhludnev commented on a diff in pull request #1466: WIP Remove Guava usages
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1140611348
##########
solr/core/src/java/org/apache/solr/core/RequestParams.java:
##########
@@ -222,13 +222,15 @@ public static class ParamSet implements MapSerializable {
this.defaults = defaults;
this.invariants = invariants;
this.appends = appends;
- ImmutableMap.Builder<String, VersionedParams> builder =
- ImmutableMap.<String, VersionedParams>builder()
- .put(PluginInfo.DEFAULTS, new VersionedParams(defaults, this));
- if (appends != null) builder.put(PluginInfo.APPENDS, new VersionedParams(appends, this));
- if (invariants != null)
+ Map<String, VersionedParams> builder = new HashMap<>();
+ builder.put(PluginInfo.DEFAULTS, new VersionedParams(defaults, this));
+ if (appends != null) {
+ builder.put(PluginInfo.APPENDS, new VersionedParams(appends, this));
+ }
+ if (invariants != null) {
builder.put(PluginInfo.INVARIANTS, new VersionedParams(invariants, this));
- paramsMap = builder.build();
+ }
+ paramsMap = Map.copyOf(builder);
Review Comment:
Well, it seems like it was immutable. But it turns to be mutable now. is it ok?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1142606285
##########
solr/modules/s3-repository/src/java/org/apache/solr/s3/S3StorageClient.java:
##########
@@ -422,6 +423,7 @@ private Collection<String> deleteObjects(Collection<String> paths) throws S3Exce
* @param batchSize number of deletes to send to S3 at a time
*/
@VisibleForTesting
+ @SuppressForbidden(reason = "Lists.partition")
Review Comment:
don't see a simple way to do this with java streams.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144811614
##########
solr/core/src/java/org/apache/solr/cloud/SizeLimitedDistributedMap.java:
##########
@@ -77,7 +77,7 @@ protected boolean lessThan(Long a, Long b) {
}
};
- Map<String, Long> childToModificationZxid = Maps.newHashMapWithExpectedSize(children.size());
+ Map<String, Long> childToModificationZxid = new HashMap<>(children.size());
Review Comment:
The conversation/review here discusses this yesterday between Kevin and I.
I'd like to propose that we have a separate dedicated work item that has the goal of *nobody* calling the integer single-arg constructor to HashMap or HashSet, as it's a trap/gotcha (with probably minor down-side, granted). We can add a utility method for creating pre-sized maps or sets.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] bruno-roustant commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "bruno-roustant (via GitHub)" <gi...@apache.org>.
bruno-roustant commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144821047
##########
solr/core/src/java/org/apache/solr/cloud/SizeLimitedDistributedMap.java:
##########
@@ -77,7 +77,7 @@ protected boolean lessThan(Long a, Long b) {
}
};
- Map<String, Long> childToModificationZxid = Maps.newHashMapWithExpectedSize(children.size());
+ Map<String, Long> childToModificationZxid = new HashMap<>(children.size());
Review Comment:
Ah, I didn't realize it is Lucene 9.5. I like David's idea to have a separate PR for that. However we will lose track of the spots where to use this construction with this PR. Maybe let a comment/todo?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1466:
URL: https://github.com/apache/solr/pull/1466#issuecomment-1476588836
Things are looking better but have this reproducing failure:
```
./gradlew :solr:core:test --tests "org.apache.solr.cloud.NestedShardedAtomicUpdateTest.doNestedInplaceUpdateTest" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=E12B264EDC3501AD -Ptests.file.encoding=ISO-8859-1
```
haven't had a chance to figure out why yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1142606711
##########
solr/core/src/java/org/apache/solr/handler/designer/ManagedSchemaDiff.java:
##########
@@ -215,7 +216,7 @@ protected static Map<String, Object> diff(
* @param simpleOrderedMap2 Map to treat as "right" map
* @return List containing the left diff and right diff
*/
- @SuppressWarnings("unchecked")
Review Comment:
Handled in df954cc38766a2094d623ba816ac71bd0c55231e
##########
solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java:
##########
@@ -755,6 +756,7 @@ protected ZkStateReader zkStateReader() {
return cc.getZkController().getZkStateReader();
}
+ @SuppressForbidden(reason = "Sets.difference")
Review Comment:
handled in df954cc38766a2094d623ba816ac71bd0c55231e
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144122612
##########
solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java:
##########
@@ -885,7 +884,7 @@ private static SolrInputDocument toSolrInputDocument(SolrDocument doc, IndexSche
if (!fieldArrayListCreated && doc.getFieldValue(fname) instanceof Collection) {
// previous value was array so we must return as an array even if was a single value
// array
- out.setField(fname, Lists.newArrayList(val));
+ out.setField(fname, new ArrayList<>(List.of(val)));
Review Comment:
when I remove the `new ArrayList<>()` This is the error
```
2> 162085 ERROR (qtp339291692-2438) [n:127.0.0.1:51699_solr c:org.apache.solr.cloud.NestedShardedAtomicUpdateTest_collection s:shard1 r:core_node6 x:org.apache.solr.cloud.NestedShardedAtomicUpdateTest_collection_shard1_replica_n2] o.a.s.h.RequestHandlerBase java.lang.UnsupportedOperationException
2> => java.lang.UnsupportedOperationException
2> at java.base/java.util.ImmutableCollections.uoe(ImmutableCollections.java:142)
2> java.lang.UnsupportedOperationException: null
2> at java.util.ImmutableCollections.uoe(ImmutableCollections.java:142) ~[?:?]
2> at java.util.ImmutableCollections$AbstractImmutableCollection.add(ImmutableCollections.java:147) ~[?:?]
2> at org.apache.solr.common.SolrInputField.addValue(SolrInputField.java:95) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.common.SolrInputDocument.addField(SolrInputDocument.java:104) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.handler.component.RealTimeGetComponent.toSolrInputDocument(RealTimeGetComponent.java:892) ~[main/:?]
2> at org.apache.solr.handler.component.RealTimeGetComponent.toSolrInputDocument(RealTimeGetComponent.java:883) ~[main/:?]
2> at org.apache.solr.handler.component.RealTimeGetComponent.getInputDocument(RealTimeGetComponent.java:794) ~[main/:?]
2> at org.apache.solr.update.processor.DistributedUpdateProcessor.getUpdatedDocument(DistributedUpdateProcessor.java:789) ~[main/:?]
2> at org.apache.solr.update.processor.DistributedUpdateProcessor.doVersionAdd(DistributedUpdateProcessor.java:407) ~[main/:?]
2> at org.apache.solr.update.processor.DistributedUpdateProcessor.lambda$versionAdd$0(DistributedUpdateProcessor.java:357) ~[main/:?]
2> at org.apache.solr.update.VersionBucket.runWithLock(VersionBucket.java:51) ~[main/:?]
2> at org.apache.solr.update.processor.DistributedUpdateProcessor.versionAdd(DistributedUpdateProcessor.java:354) ~[main/:?]
2> at org.apache.solr.update.processor.DistributedUpdateProcessor.processAdd(DistributedUpdateProcessor.java:236) ~[main/:?]
2> at org.apache.solr.update.processor.DistributedZkUpdateProcessor.processAdd(DistributedZkUpdateProcessor.java:279) ~[main/:?]
2> at org.apache.solr.update.processor.LogUpdateProcessorFactory$LogUpdateProcessor.processAdd(LogUpdateProcessorFactory.java:111) ~[main/:?]
2> at org.apache.solr.handler.loader.JavabinLoader$1.update(JavabinLoader.java:123) ~[main/:?]
2> at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec$StreamingCodec.readOuterMostDocIterator(JavaBinUpdateRequestCodec.java:342) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec$StreamingCodec.readIterator(JavaBinUpdateRequestCodec.java:286) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.common.util.JavaBinCodec.readObject(JavaBinCodec.java:338) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.common.util.JavaBinCodec.readVal(JavaBinCodec.java:283) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec$StreamingCodec.readNamedList(JavaBinUpdateRequestCodec.java:236) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.common.util.JavaBinCodec.readObject(JavaBinCodec.java:303) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.common.util.JavaBinCodec.readVal(JavaBinCodec.java:283) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.common.util.JavaBinCodec.unmarshal(JavaBinCodec.java:193) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec.unmarshal(JavaBinUpdateRequestCodec.java:126) ~[solr-solrj-10.0.0-SNAPSHOT.jar:10.0.0-SNAPSHOT 735623be5eea82a70693aee39819f2baf151aa75 [snapshot build, details omitted]]
2> at org.apache.solr.handler.loader.JavabinLoader.parseAndLoadDocs(JavabinLoader.java:135) ~[main/:?]
2> at org.apache.solr.handler.loader.JavabinLoader.load(JavabinLoader.java:74) ~[main/:?]
2> at org.apache.solr.handler.UpdateRequestHandler$1.load(UpdateRequestHandler.java:101) ~[main/:?]
2> at org.apache.solr.handler.ContentStreamHandlerBase.handleRequestBody(ContentStreamHandlerBase.java:84) ~[main/:?]
2> at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:224) [main/:?]
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1466:
URL: https://github.com/apache/solr/pull/1466#issuecomment-1478627949
@dsmiley thanks for the thorough review. I addressed most of them or left comments. The only one that is outstanding I think is - https://github.com/apache/solr/pull/1466#discussion_r1144008874 which is around `new ArrayList<>(List.of(val)));`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144816158
##########
solr/core/src/java/org/apache/solr/cloud/SizeLimitedDistributedMap.java:
##########
@@ -77,7 +77,7 @@ protected boolean lessThan(Long a, Long b) {
}
};
- Map<String, Long> childToModificationZxid = Maps.newHashMapWithExpectedSize(children.size());
+ Map<String, Long> childToModificationZxid = new HashMap<>(children.size());
Review Comment:
Lucenes CollectionUtil is only available in 9.5 - https://github.com/apache/lucene/commit/e0b8c88ecc7d453608111da2ea1abc01d3631c13 so sadly can't use it yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144829541
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1265,15 +1260,13 @@ protected Elevation mergeWith(Elevation elevation) {
return this;
}
Set<BytesRef> elevatedIds =
- ImmutableSet.<BytesRef>builder()
- .addAll(this.elevatedIds)
- .addAll(elevation.elevatedIds)
- .build();
+ Stream.concat(this.elevatedIds.stream(), elevation.elevatedIds.stream())
+ .collect(Collectors.toCollection(LinkedHashSet::new));
boolean overlappingElevatedIds =
elevatedIds.size() != (this.elevatedIds.size() + elevation.elevatedIds.size());
BooleanQuery.Builder includeQueryBuilder = new BooleanQuery.Builder();
Set<BooleanClause> clauseSet =
- (overlappingElevatedIds ? Sets.newHashSetWithExpectedSize(elevatedIds.size()) : null);
+ (overlappingElevatedIds ? new HashSet<>(elevatedIds.size()) : null);
Review Comment:
Addressed in 30e502199996d815c2acdade8f0f3be58afc0618
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1144836611
##########
solr/core/src/java/org/apache/solr/cloud/SizeLimitedDistributedMap.java:
##########
@@ -77,7 +77,7 @@ protected boolean lessThan(Long a, Long b) {
}
};
- Map<String, Long> childToModificationZxid = Maps.newHashMapWithExpectedSize(children.size());
+ Map<String, Long> childToModificationZxid = new HashMap<>(children.size());
Review Comment:
https://issues.apache.org/jira/browse/SOLR-16715
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1466: SOLR-16713: Replace Guava usages with pure Java
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1143669893
##########
solr/core/src/java/org/apache/solr/core/backup/BackupManager.java:
##########
@@ -254,7 +254,10 @@ public void uploadConfigDir(
String sourceConfigName, String targetConfigName, ConfigSetService configSetService)
throws IOException {
URI source = repository.resolveDirectory(getZkStateDir(), CONFIG_STATE_DIR, sourceConfigName);
- Preconditions.checkState(repository.exists(source), "Path %s does not exist", source);
+ if (!repository.exists(source)) {
+ throw new IllegalArgumentException(
+ String.format(Locale.ROOT, "Path %s does not exist", source));
Review Comment:
Fine, but just want to say I think String.format is over-used, such as here. Good 'ol String concatenation is simple.
##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -2937,7 +2939,12 @@ public static void preDecorateResponse(SolrQueryRequest req, SolrQueryResponse r
@Override
public Iterator<String> getParameterNamesIterator() {
- return Iterators.filter(params.getParameterNamesIterator(), lpSet::contains);
+ return StreamSupport.stream(
+ Spliterators.spliteratorUnknownSize(
+ params.getParameterNamesIterator(), Spliterator.ORDERED),
+ false)
Review Comment:
It [was a deliberate decision](https://stackoverflow.com/a/23177907/92186) of Java architects to make it hard to go from an Iterator to a Stream. In light of this, maybe we should avoid returning Iterators from our APIs, I suppose. Just thinking out loud; your change here is fine.
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1265,15 +1261,13 @@ protected Elevation mergeWith(Elevation elevation) {
return this;
}
Set<BytesRef> elevatedIds =
- ImmutableSet.<BytesRef>builder()
- .addAll(this.elevatedIds)
- .addAll(elevation.elevatedIds)
- .build();
+ Stream.concat(this.elevatedIds.stream(), elevation.elevatedIds.stream())
+ .collect(Collectors.toCollection(LinkedHashSet::new));
boolean overlappingElevatedIds =
elevatedIds.size() != (this.elevatedIds.size() + elevation.elevatedIds.size());
BooleanQuery.Builder includeQueryBuilder = new BooleanQuery.Builder();
Set<BooleanClause> clauseSet =
- (overlappingElevatedIds ? Sets.newHashSetWithExpectedSize(elevatedIds.size()) : null);
+ (overlappingElevatedIds ? new HashSet<>(elevatedIds.size()) : null);
Review Comment:
This is a Java gotcha! [Java 19 has an substitute](https://bugs.openjdk.org/browse/JDK-8186958) but obviously we're not there yet. Maybe we continue to use Guava for this or have our own utility? Or just use 2x the size?
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1299,14 +1291,16 @@ protected Elevation mergeWith(Elevation elevation) {
excludedIds.size() != (this.excludedIds.size() + elevation.excludedIds.size());
if (overlappingExcludedIds) {
excludeQueries =
- ImmutableSet.<TermQuery>builder()
- .add(this.excludeQueries)
- .add(elevation.excludeQueries)
- .build()
- .toArray(new TermQuery[0]);
+ Stream.concat(
+ Arrays.stream(this.excludeQueries), Arrays.stream(elevation.excludeQueries))
+ .collect(Collectors.toUnmodifiableSet())
Review Comment:
Use Stream.distinct().toArray(...)
##########
solr/modules/analytics/src/test/org/apache/solr/analytics/legacy/LegacyAbstractAnalyticsTest.java:
##########
@@ -227,18 +229,25 @@ public <T extends Comparable<T>> Long calculateMissing(ArrayList<T> list, String
}
public static SolrQueryRequest request(String... args) {
- return SolrTestCaseJ4.req(ObjectArrays.concat(BASEPARMS, args, String.class));
+ return SolrTestCaseJ4.req(
+ Stream.concat(Arrays.stream(BASEPARMS), Arrays.stream(args))
+ .collect(Collectors.toUnmodifiableList())
+ .toArray(String[]::new));
Review Comment:
don't collect then toArray; just go straight to toArray
##########
solr/modules/analytics/src/test/org/apache/solr/analytics/legacy/LegacyAbstractAnalyticsTest.java:
##########
@@ -227,18 +229,25 @@ public <T extends Comparable<T>> Long calculateMissing(ArrayList<T> list, String
}
public static SolrQueryRequest request(String... args) {
- return SolrTestCaseJ4.req(ObjectArrays.concat(BASEPARMS, args, String.class));
+ return SolrTestCaseJ4.req(
+ Stream.concat(Arrays.stream(BASEPARMS), Arrays.stream(args))
+ .collect(Collectors.toUnmodifiableList())
+ .toArray(String[]::new));
}
public static SolrQueryRequest request(String[] args, String... additional) {
- return SolrTestCaseJ4.req(ObjectArrays.concat(BASEPARMS, args, String.class), additional);
+ return SolrTestCaseJ4.req(
+ Stream.concat(Arrays.stream(BASEPARMS), Arrays.stream(args))
+ .collect(Collectors.toUnmodifiableList())
+ .toArray(String[]::new),
Review Comment:
don't collect then toArray; just go straight to toArray
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1234,12 +1230,12 @@ public Elevation(Set<BytesRef> elevatedIds, Set<BytesRef> excludedIds, String qu
this.excludedIds = Collections.emptySet();
excludeQueries = null;
} else {
- this.excludedIds = ImmutableSet.copyOf(excludedIds);
- List<TermQuery> excludeQueriesBuilder = new ArrayList<>(excludedIds.size());
- for (BytesRef excludedId : excludedIds) {
- excludeQueriesBuilder.add(new TermQuery(new Term(queryFieldName, excludedId)));
- }
- excludeQueries = excludeQueriesBuilder.toArray(new TermQuery[0]);
+ this.excludedIds = Collections.unmodifiableSet(new LinkedHashSet<>(excludedIds));
+ this.excludeQueries =
+ this.excludedIds.stream()
+ .map(excludedId -> new TermQuery(new Term(queryFieldName, excludedId)))
+ .collect(Collectors.toUnmodifiableList())
Review Comment:
Stream.toArray exists; remove the intermediate List
##########
solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java:
##########
@@ -885,7 +884,7 @@ private static SolrInputDocument toSolrInputDocument(SolrDocument doc, IndexSche
if (!fieldArrayListCreated && doc.getFieldValue(fname) instanceof Collection) {
// previous value was array so we must return as an array even if was a single value
// array
- out.setField(fname, Lists.newArrayList(val));
+ out.setField(fname, new ArrayList<>(List.of(val)));
Review Comment:
Why the extra wrapping using List.of ?
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1299,14 +1291,16 @@ protected Elevation mergeWith(Elevation elevation) {
excludedIds.size() != (this.excludedIds.size() + elevation.excludedIds.size());
if (overlappingExcludedIds) {
excludeQueries =
- ImmutableSet.<TermQuery>builder()
- .add(this.excludeQueries)
- .add(elevation.excludeQueries)
- .build()
- .toArray(new TermQuery[0]);
+ Stream.concat(
+ Arrays.stream(this.excludeQueries), Arrays.stream(elevation.excludeQueries))
+ .collect(Collectors.toUnmodifiableSet())
+ .toArray(TermQuery[]::new);
} else {
excludeQueries =
- ObjectArrays.concat(this.excludeQueries, elevation.excludeQueries, TermQuery.class);
+ Stream.concat(
+ Arrays.stream(this.excludeQueries), Arrays.stream(elevation.excludeQueries))
+ .collect(Collectors.toUnmodifiableSet())
Review Comment:
Use Stream.distinct().toArray(...)
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -2192,6 +2191,9 @@ public SolrCore getCore(String name) {
* @see SolrCore#close()
*/
public SolrCore getCore(String name, UUID id) {
+ if (name == null) {
+ return null;
+ }
Review Comment:
Unrelated?
##########
solr/core/src/test/org/apache/solr/cloud/NestedShardedAtomicUpdateTest.java:
##########
@@ -64,7 +72,10 @@ public static void beforeClass() throws Exception {
@AfterClass
public static void afterClass() throws Exception {
- IOUtils.close(clients);
+ if (clients != null) {
+ IOUtils.close(clients);
+ clients = null;
+ }
Review Comment:
Did this change prove to be necessary or impactful?
##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -1558,7 +1556,7 @@ Solution is q(k) = 1/2 (k^2+k+2)
public Builder<E, M> addSubset(Collection<E> subset, M matchValue) {
if (!subset.isEmpty()) {
TrieSubsetMatcher.Node<E, M> node = root;
- for (E e : ImmutableSortedSet.copyOf(subset)) {
+ for (E e : Collections.unmodifiableSortedSet(new TreeSet<>(subset))) {
Review Comment:
Why wrap with unmodifiable?
Come to think of it, can't you do subset.sort().forEach(...) ? No copying is needed either; I looked.
CC @bruno-roustant
##########
solr/modules/analytics/src/test/org/apache/solr/analytics/legacy/facet/LegacyAbstractAnalyticsFacetTest.java:
##########
@@ -306,7 +307,10 @@ public <T extends Comparable<T>> Long calculateMissing(ArrayList<T> list, String
}
public static SolrQueryRequest request(String... args) {
- return SolrTestCaseJ4.req(ObjectArrays.concat(BASEPARMS, args, String.class));
+ return SolrTestCaseJ4.req(
+ Stream.concat(Arrays.stream(BASEPARMS), Arrays.stream(args))
+ .collect(Collectors.toUnmodifiableList())
+ .toArray(String[]::new));
Review Comment:
don't collect then toArray; just go straight to toArray
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1466:
URL: https://github.com/apache/solr/pull/1466#discussion_r1142293958
##########
solr/core/src/java/org/apache/solr/handler/designer/ManagedSchemaDiff.java:
##########
@@ -215,7 +216,7 @@ protected static Map<String, Object> diff(
* @param simpleOrderedMap2 Map to treat as "right" map
* @return List containing the left diff and right diff
*/
- @SuppressWarnings("unchecked")
Review Comment:
See if there is a java streams way to do this
##########
solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java:
##########
@@ -755,6 +756,7 @@ protected ZkStateReader zkStateReader() {
return cc.getZkController().getZkStateReader();
}
+ @SuppressForbidden(reason = "Sets.difference")
Review Comment:
See if there is a java streams way to do this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #1466: WIP Remove Guava usages
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1466:
URL: https://github.com/apache/solr/pull/1466#issuecomment-1476782916
`NestedShadedAtomicUpdateTest` fixed with 184449a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org