rseitz commented on code in PR #1154:
URL: https://github.com/apache/solr/pull/1154#discussion_r1016028039


##########
solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java:
##########
@@ -553,6 +560,82 @@ private void setQuery(ResponseBuilder rb, Elevation 
elevation) {
     }
   }
 
+  /**
+   * Updates any filters that have been tagged for exclusion. Filters can be 
tagged for exclusion
+   * via the syntax fq={!tag=t1}field1:value1&elevate.excludeTags=t1 This 
method modifies each
+   * "excluded" filter so that it becomes a boolean OR of the original filter 
with an "include
+   * query" that matches the elevated documents by their IDs. To be clear, the 
"excluded" filters
+   * are not ignored entirely, but rather broadened so that the elevated 
documents are allowed
+   * through.
+   */
+  private void setFilters(ResponseBuilder rb, Elevation elevation) {
+    SolrParams params = rb.req.getParams();
+
+    String tagStr = params.get(QueryElevationParams.ELEVATE_EXCLUDE_TAGS);
+    if (StringUtils.isEmpty(tagStr)) {
+      // the parameter that specifies tags for exclusion was not provided or 
had no value
+      return;
+    }
+
+    List<String> excludeTags = StrUtils.splitSmart(tagStr, ',');
+    excludeTags.removeIf(s -> StringUtils.isBlank(s));
+    if (excludeTags.isEmpty()) {
+      // the parameter that specifies tags for exclusion was provided but the 
tag names were blank
+      return;
+    }
+
+    List<Query> filters = rb.getFilters();
+    if (filters == null || filters.isEmpty()) {
+      // no filters were provided
+      return;
+    }
+
+    Map<?, ?> tagMap = (Map<?, ?>) rb.req.getContext().get("tags");
+    if (tagMap == null) {
+      // no filters were tagged
+      return;
+    }
+
+    // TODO: this code is copied from FacetProcessor#handleFilterExclusions()
+    // duplication could be avoided by placing this code in a common utility 
method, perhaps in
+    // QueryUtils

Review Comment:
   Yes, thx for the encouragement to do it now :) Two questions came up in 
implementing the `ResponseBuilder` approach. 1) I noticed that 
`FacetProcessor#handleFilterExclusions()` has a comment about wanting to 
_remove_ the `ResponseBuilder` dependency there -- how significant is this? 2) 
I noticed that the facet processor code gets the "tagMap" via the 
`SolrQueryRequest` it finds in its `FacetContext`. Can I assume the 
`SolrQueryRequest` inside the `ResponseBuilder` (obtained via 
`SolrRequestInfo.getRequestInfo().getResponseBuilder()`) would be the same as 
the one in the FacetContext, or at least that its request context would have 
the same tags inside?  To make things clearer, I sketched out what the refactor 
would look like with RB as the destination, as well as two other possible 
destinations: QueryUtils and SolrQueryRequest itself.  Let me know if RB still 
seems best to you or if either of the others looks reasonable. See: 
https://github.com/rseitz/solr/commit/a0675c51eed9f29b35
 d0a1a2fe626dcb16812077



-- 
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]

Reply via email to