cpoerschke commented on a change in pull request #207:
URL: https://github.com/apache/solr/pull/207#discussion_r667522463
##########
File path:
solr/core/src/test/org/apache/solr/handler/component/DistributedFacetPivotLongTailTest.java
##########
@@ -81,41 +82,40 @@ private void sanityCheckIndividualShards() throws Exception
{
PivotField pivot = null;
List<PivotField> pivots = null;
- @SuppressWarnings({"unchecked", "rawtypes"})
- List<PivotField>[] shardPivots = new List[clients.size()];
+ List<List<PivotField>> shardPivots = new ArrayList<>(clients.size());
for (int i = 0; i < clients.size(); i++) {
- shardPivots[i] = clients.get(i).query( req
).getFacetPivot().get("foo_s,bar_s");
+ shardPivots.add(clients.get(i).query( req
).getFacetPivot().get("foo_s,bar_s"));
}
// top 5 same on all shards
for (int i = 0; i < 3; i++) {
- assertEquals(10, shardPivots[i].size());
+ assertEquals(10, shardPivots.get(i).size());
for (int j = 0; j < 5; j++) {
- pivot = shardPivots[i].get(j);
+ pivot = shardPivots.get(i).get(j);
assertEquals(pivot.toString(), "aaa"+j, pivot.getValue());
assertEquals(pivot.toString(), 100, pivot.getCount());
}
}
// top 6-10 same on shard0 & shard11
Review comment:
```suggestion
// top 6-10 same on shard0 & shard1
```
##########
File path: solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
##########
@@ -589,13 +580,11 @@ public void testFailures() throws Exception {
public static class CacheTest extends DumpRequestHandler {
@Override
- @SuppressWarnings({"unchecked"})
public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp)
throws IOException {
super.handleRequestBody(req, rsp);
String[] caches = req.getParams().getParams("cacheNames");
if(caches != null && caches.length>0){
- @SuppressWarnings({"rawtypes"})
- HashMap m = new HashMap();
+ HashMap<String, String> m = new HashMap<>();
Review comment:
The UI won't let me suggest it there but in the for loop that uses `m`
we could do
```
- @SuppressWarnings({"rawtypes"})
- SolrCache cache = req.getSearcher().getCache(c);
+ SolrCache<?,?> cache = req.getSearcher().getCache(c);
```
now I think whilst here rather than coming back for that later.
##########
File path: solr/core/src/test/org/apache/solr/SolrInfoBeanTest.java
##########
@@ -64,10 +62,11 @@ public void testCallMBeanInfo() throws Exception {
String registry = h.getCore().getCoreMetricManager().getRegistryName();
SolrMetricsContext solrMetricsContext = new
SolrMetricsContext(metricManager, registry, "foo");
String scope = TestUtil.randomSimpleString(random(), 2, 10);
- for(@SuppressWarnings({"rawtypes"})Class clazz : classes ) {
+ for(Class<?> clazz : classes ) {
if( SolrInfoBean.class.isAssignableFrom( clazz ) ) {
try {
SolrInfoBean info =
(SolrInfoBean)clazz.getConstructor().newInstance();
+ // TODO: The following line is always true?
Review comment:
> // TODO: The following line is always true?
Yes, it is now via the
https://github.com/apache/solr/commit/e58a90f18d9823404f3f4ff59bffedf31d093a0d#diff-dd8188c05fd076ad0993f85b2d05b98f1aa8fb65d41eebbcac120f741b3c7118
change but was not at the time of the `instanceof` addition in
https://github.com/apache/solr/commit/e30cc70fddcdd6fddb5eedf9f38e77fcb3f33bd1#diff-04b9e9519e437e30054389ba264496f668fc8861d8879ba5513eb01961f68605
i.e. could be simplified now then.
##########
File path: solr/core/src/test/org/apache/solr/pkg/TestPackages.java
##########
@@ -501,15 +501,15 @@ public void testPluginLoading() throws Exception {
Map.of(":files:" + FILE3 + ":name", "runtimelibs_v3.jar"),
false);
}
- @SuppressWarnings({"unchecked","rawtypes"})
- private void executeReq(String uri, JettySolrRunner jetty,
Utils.InputStreamConsumer parser, Map expected) throws Exception {
+ @SuppressWarnings("unchecked")
+ private void executeReq(String uri, JettySolrRunner jetty,
Utils.InputStreamConsumer<?> parser, Map<String, Object> expected) throws
Exception {
Review comment:
minor: maintain existing indentation
```suggestion
@SuppressWarnings("unchecked")
private void executeReq(String uri, JettySolrRunner jetty,
Utils.InputStreamConsumer<?> parser, Map<String, Object> expected) throws
Exception {
```
##########
File path:
solr/core/src/test/org/apache/solr/handler/TestSolrConfigHandlerConcurrent.java
##########
@@ -105,7 +99,8 @@ public void test() throws Exception {
private void invokeBulkCall(String cacheName, List<String> errs,
- @SuppressWarnings({"rawtypes"})Map val) throws
Exception {
+ // TODO this is unused - is that a bug?
Review comment:
> // TODO this is unused - is that a bug?
It seems to me that the test could also test that `val` content other than
size/initialSize/autowarmCount is preserved. So more of an omission or gap than
a bug, but confusing either way, so thanks for adding the TODO comment.
##########
File path: solr/solrj/src/java/org/apache/solr/common/SolrInputField.java
##########
@@ -167,27 +168,9 @@ public void setName(String name) {
@SuppressWarnings("unchecked")
public Iterator<Object> iterator(){
if( value instanceof Collection ) {
- return ((Collection)value).iterator();
+ return ((Collection<Object>)value).iterator();
}
- return new Iterator<Object>() {
- boolean nxt = (value!=null);
-
- @Override
- public boolean hasNext() {
- return nxt;
- }
-
- @Override
- public Object next() {
- nxt = false;
- return value;
- }
-
- @Override
- public void remove() {
- throw new UnsupportedOperationException();
- }
- };
+ return Collections.singleton(value).iterator();
Review comment:
assuming existing behaviour w.r.t. value being null is to be maintained
need this be something like this instead?
```suggestion
return value==null ? Collections.emptyIterator() :
Collections.singleton(value).iterator();
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
