cpoerschke commented on a change in pull request #207:
URL: https://github.com/apache/solr/pull/207#discussion_r667381791
##########
File path: solr/core/src/java/org/apache/solr/search/CacheConfig.java
##########
@@ -116,12 +116,10 @@ public static CacheConfig getConfig(SolrConfig
solrConfig, String xpath) {
}
- @SuppressWarnings({"unchecked"})
public static CacheConfig getConfig(SolrConfig solrConfig, String nodeName,
Map<String,String> attrs, String xpath) {
CacheConfig config = new CacheConfig();
config.nodeName = nodeName;
- @SuppressWarnings({"rawtypes"})
- Map attrsCopy = new LinkedHashMap<>(attrs.size());
+ Map<String,String> attrsCopy = new LinkedHashMap<>(attrs.size());
for (Map.Entry<String, String> e : attrs.entrySet()) {
attrsCopy.put(e.getKey(), String.valueOf(e.getValue()));
Review comment:
```suggestion
attrsCopy.put(e.getKey(), e.getValue());
```
##########
File path:
solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
##########
@@ -2004,9 +2003,8 @@ public SortedDocValues getSorted(FieldInfo ignored)
throws IOException {
* If it is, then "this" will be added to the readerContext
* using the "CSCORE" key, and true will be returned. If not returns
false.
*/
- @SuppressWarnings({"unchecked"})
public boolean setupIfNeeded(final GroupHeadSelector groupHeadSelector,
- @SuppressWarnings({"rawtypes"})final Map
readerContext) {
+ final Map<? super String, ? super
CollapseScore> readerContext) {
Review comment:
Not sure about this based on the `private Map<Object, Object> rcontext`
being passed to the method, though maybe I'm confusing the `PECS` thing again.
```suggestion
final Map<Object, Object> readerContext) {
```
##########
File path: solr/core/src/java/org/apache/solr/search/Grouping.java
##########
@@ -642,16 +639,14 @@ protected DocList
getDocList(@SuppressWarnings({"rawtypes"})GroupDocs groups) {
return docs;
}
- @SuppressWarnings({"unchecked"})
- protected void addDocList(@SuppressWarnings({"rawtypes"})NamedList rsp
- , @SuppressWarnings({"rawtypes"})GroupDocs groups) {
+ protected void addDocList(NamedList<? super DocList> rsp, GroupDocs<?>
groups) {
Review comment:
note to self: understand more if/why/how this works i.e. why not instead
```suggestion
protected void addDocList(NamedList<?> rsp, GroupDocs<?> groups) {
```
or
```suggestion
protected void addDocList(NamedList<Object> rsp, GroupDocs<?> groups) {
```
##########
File path:
solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
##########
@@ -176,13 +176,14 @@ private void validateConfig(PayloadObj<PluginMeta>
payload, PluginMeta info) thr
}
}
- @SuppressWarnings({"rawtypes", "unchecked"})
private void persistPlugins(Function<Map<String,Object>, Map<String,Object>>
modifier) throws IOException {
try {
zkClientSupplier.get().atomicUpdate(ZkStateReader.CLUSTER_PROPS, bytes
-> {
- Map rawJson = bytes == null ? new LinkedHashMap<>() :
- (Map) Utils.fromJSON(bytes);
- Map pluginsModified = modifier.apply((Map)
rawJson.computeIfAbsent(PLUGIN, o -> new LinkedHashMap<>()));
+ @SuppressWarnings("unchecked")
+ Map<String, Object> rawJson = bytes == null ? new LinkedHashMap<>() :
+ (Map<String, Object>) Utils.fromJSON(bytes);
+ @SuppressWarnings("unchecked")
+ Map<?, ?> pluginsModified = modifier.apply((Map<String, Object>)
rawJson.computeIfAbsent(PLUGIN, o -> new LinkedHashMap<>()));
Review comment:
```suggestion
Map<String, Object> pluginsModified = modifier.apply((Map<String,
Object>) rawJson.computeIfAbsent(PLUGIN, o -> new LinkedHashMap<>()));
```
##########
File path: solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
##########
@@ -281,14 +278,13 @@ private void handleGET() {
}
}
- @SuppressWarnings({"unchecked"})
private Map<String, Object> getConfigDetails(String componentType,
SolrQueryRequest req) {
String componentName = componentType == null ? null :
req.getParams().get("componentName");
boolean showParams = req.getParams().getBool("expandParams", false);
Map<String, Object> map = this.req.getCore().getSolrConfig().toMap(new
LinkedHashMap<>());
if (componentType != null &&
!SolrRequestHandler.TYPE.equals(componentType)) return map;
- @SuppressWarnings({"rawtypes"})
- Map reqHandlers = (Map) map.get(SolrRequestHandler.TYPE);
+ @SuppressWarnings({"unchecked"})
+ Map<String, Object> reqHandlers = (Map<String, Object>)
map.get(SolrRequestHandler.TYPE);
if (reqHandlers == null) map.put(SolrRequestHandler.TYPE, reqHandlers =
new LinkedHashMap<>());
Review comment:
```suggestion
Map<String, Object> reqHandlers = (Map<String, Object>)
map.computeIfAbsent(SolrRequestHandler.TYPE, k -> new LinkedHashMap<>());
```
##########
File path:
solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/SearchGroupsResultTransformer.java
##########
@@ -48,10 +48,9 @@ public SearchGroupsResultTransformer(SolrIndexSearcher
searcher) {
}
@Override
- @SuppressWarnings({"rawtypes"})
- public NamedList transform(List<Command> data) throws IOException {
- final NamedList<NamedList> result = new NamedList<>(data.size());
- for (Command command : data) {
+ public NamedList<NamedList<Object>> transform(List<Command<?>> data) throws
IOException {
+ final NamedList<NamedList<Object>> result = new NamedList<>(data.size());
+ for (Command<?> command : data) {
final NamedList<Object> commandResult = new NamedList<>(2);
if (SearchGroupsFieldCommand.class.isInstance(command)) {
Review comment:
Maybe use `instanceof` here like in the `TopGroupsResultTransformer`
class.
```suggestion
if (command instanceof SearchGroupsFieldCommand) {
```
##########
File path:
solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/SearchGroupsResultTransformer.java
##########
@@ -36,7 +36,7 @@
* Implementation for transforming {@link SearchGroup} into a {@link
NamedList} structure and visa versa.
*/
@SuppressWarnings({"rawtypes"})
Review comment:
Can this be removed or relocated now?
```suggestion
```
##########
File path:
solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/TopGroupsResultTransformer.java
##########
@@ -55,7 +55,7 @@
* vice versa.
*/
@SuppressWarnings({"rawtypes"})
Review comment:
Can this be removed now or relocated to reduced scope?
```suggestion
```
##########
File path: solr/core/src/java/org/apache/solr/search/facet/UnInvertedField.java
##########
@@ -682,15 +682,16 @@ protected static void rethrowAsSolrException(String
field, Throwable e) {
}
// Returns null if not already populated
- @SuppressWarnings({"rawtypes", "unchecked"})
public static UnInvertedField checkUnInvertedField(String field,
SolrIndexSearcher searcher) throws IOException {
- SolrCache cache = searcher.getFieldValueCache();
+ SolrCache<String, UnInvertedField> cache = searcher.getFieldValueCache();
if (cache == null) {
return null;
}
Object uif = cache.get(field); // cache is already synchronized, so no
extra sync needed
// placeholder is an implementation detail, keep it hidden and return null
if that is what we got
return uif==uifPlaceholder || !(uif instanceof UnInvertedField)? null :
(UnInvertedField) uif;
+ // TODO: SolrCache is not used safely in other places, but this might be
simpligfied to:
+ // return uif==uifPlaceholder ? null : uif;
Review comment:
Could you share more re: what other places do not safely use SolrCache?
I agree with the "this might be simplified" bit, from looking at the code here
only.
```suggestion
UnInvertedField uif = cache.get(field); // cache is already
synchronized, so no extra sync needed
// placeholder is an implementation detail, keep it hidden and return
null if that is what we got
return uif==uifPlaceholder || uif==null ? null : uif;
```
And I'd like to note that the `|| uif==null` is redundant but without it the
"return null if that is what we got" part of the comment would be less clear,
though we could somehow change the comment too of course.
--
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]
