You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2019/12/25 09:47:51 UTC
[GitHub] [lucene-solr] a-siuniaev opened a new pull request #1122:
SOLR-12490 Added parsing of json.queries for referring in JSON facets
a-siuniaev opened a new pull request #1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
URL: https://github.com/apache/lucene-solr/pull/1122
# Description
Added DSL support for queries - to have the ability to refer from domain.filter. It supports both single and multi-value query parameters. Support forexclusion is not yet implemented.
# Solution
Intoduced json.queries field, which is translated with query DSL (see [comment](https://issues.apache.org/jira/browse/SOLR-12490?focusedCommentId=16514420&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16514420)).
# Checklist
- [+] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [+] I have created a Jira issue and added the issue ID to my pull request title.
- [+] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [+] I have developed this patch against the `master` branch.
- [+] I have run `ant precommit` and the appropriate test suite.
- [+] I have added tests for my changes.
- [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] a-siuniaev commented on a change in pull request
#1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
Posted by GitBox <gi...@apache.org>.
a-siuniaev commented on a change in pull request #1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
URL: https://github.com/apache/lucene-solr/pull/1122#discussion_r361398218
##########
File path: solr/core/src/java/org/apache/solr/request/json/RequestUtil.java
##########
@@ -214,37 +216,22 @@ public static void processParams(SolrRequestHandler handler, SolrQueryRequest re
out = "rows";
} else if (SORT.equals(key)) {
out = SORT;
+ } else if ("queries".equals(key)) {
+ for (Map.Entry<String, Object> subEntry : ((Map<String, Object>) entry.getValue()).entrySet()) {
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mkhludnev commented on a change in pull request
#1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
Posted by GitBox <gi...@apache.org>.
mkhludnev commented on a change in pull request #1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
URL: https://github.com/apache/lucene-solr/pull/1122#discussion_r361401011
##########
File path: solr/core/src/java/org/apache/solr/request/json/RequestUtil.java
##########
@@ -214,48 +215,60 @@ public static void processParams(SolrRequestHandler handler, SolrQueryRequest re
out = "rows";
} else if (SORT.equals(key)) {
out = SORT;
+ } else if ("queries".equals(key)) {
+ Object queriesJsonObj = entry.getValue();
+ if (queriesJsonObj instanceof Map) {
+ for (Map.Entry<String, Object> queryJsonProperty : ((Map<String, Object>) queriesJsonObj).entrySet()) {
+ out = queryJsonProperty.getKey();
+ arr = true;
+ isQuery = true;
+ convertJsonPropertyToLocalParams(newMap, jsonQueryConverter, queryJsonProperty, out, isQuery, arr);
+ }
+ continue;
+ } else {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Expected Map for 'queries', received " + queriesJsonObj.getClass().getSimpleName() + "=" + queriesJsonObj);
Review comment:
not Map. but Object for json.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mkhludnev commented on a change in pull request
#1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
Posted by GitBox <gi...@apache.org>.
mkhludnev commented on a change in pull request #1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
URL: https://github.com/apache/lucene-solr/pull/1122#discussion_r361335761
##########
File path: solr/core/src/java/org/apache/solr/request/json/RequestUtil.java
##########
@@ -214,37 +216,22 @@ public static void processParams(SolrRequestHandler handler, SolrQueryRequest re
out = "rows";
} else if (SORT.equals(key)) {
out = SORT;
+ } else if ("queries".equals(key)) {
+ for (Map.Entry<String, Object> subEntry : ((Map<String, Object>) entry.getValue()).entrySet()) {
+ out = subEntry.getKey();
+ arr = true;
+ isQuery = true;
+ processJsonEntry(newMap, jsonQueryConverter, subEntry, out, isQuery, arr);
+ }
+ continue;
} else if ("params".equals(key) || "facet".equals(key) ) {
// handled elsewhere
continue;
} else {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unknown top-level key in JSON request : " + key);
}
- Object val = entry.getValue();
-
- if (arr) {
- String[] existing = newMap.get(out);
- List lst = val instanceof List ? (List)val : null;
- int existingSize = existing==null ? 0 : existing.length;
- int jsonSize = lst==null ? 1 : lst.size();
- String[] newval = new String[ existingSize + jsonSize ];
- for (int i=0; i<existingSize; i++) {
- newval[i] = existing[i];
- }
- if (lst != null) {
- for (int i = 0; i < jsonSize; i++) {
- Object v = lst.get(i);
- newval[existingSize + i] = isQuery ? jsonQueryConverter.toLocalParams(v, newMap) : v.toString();
- }
- } else {
- newval[newval.length-1] = isQuery ? jsonQueryConverter.toLocalParams(val, newMap) : val.toString();
- }
- newMap.put(out, newval);
- } else {
- newMap.put(out, new String[]{isQuery ? jsonQueryConverter.toLocalParams(val, newMap) : val.toString()});
- }
-
+ processJsonEntry(newMap, jsonQueryConverter, entry, out, isQuery, arr);
Review comment:
`process` is a meaningless verb, but I saw enclosing method. ok. From JSON's POV it's a `property` not entry.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mkhludnev commented on a change in pull request
#1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
Posted by GitBox <gi...@apache.org>.
mkhludnev commented on a change in pull request #1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
URL: https://github.com/apache/lucene-solr/pull/1122#discussion_r361335287
##########
File path: solr/core/src/java/org/apache/solr/request/json/RequestUtil.java
##########
@@ -191,7 +191,9 @@ public static void processParams(SolrRequestHandler handler, SolrQueryRequest re
}
// implement compat for existing components...
- JsonQueryConverter jsonQueryConverter = new JsonQueryConverter();
+ JsonQueryConverter jsonQueryConverter = (JsonQueryConverter) req.getContext()
Review comment:
I think it's necessary only for the earlier "dynamic" approach. I suppose it's not necessary in the eager one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mkhludnev commented on a change in pull request
#1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
Posted by GitBox <gi...@apache.org>.
mkhludnev commented on a change in pull request #1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
URL: https://github.com/apache/lucene-solr/pull/1122#discussion_r361400807
##########
File path: solr/core/src/java/org/apache/solr/request/json/JsonQueryConverter.java
##########
@@ -28,6 +28,10 @@
* @lucene.internal
*/
class JsonQueryConverter {
+ public static final String paramsPrefix = "_tt";
+
+ public static final Object contextKey = JsonQueryConverter.class.getSimpleName();
Review comment:
Now, It's redundant, I suppose.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] a-siuniaev commented on a change in pull request
#1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
Posted by GitBox <gi...@apache.org>.
a-siuniaev commented on a change in pull request #1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
URL: https://github.com/apache/lucene-solr/pull/1122#discussion_r361397810
##########
File path: solr/core/src/java/org/apache/solr/request/json/RequestUtil.java
##########
@@ -191,7 +191,9 @@ public static void processParams(SolrRequestHandler handler, SolrQueryRequest re
}
// implement compat for existing components...
- JsonQueryConverter jsonQueryConverter = new JsonQueryConverter();
+ JsonQueryConverter jsonQueryConverter = (JsonQueryConverter) req.getContext()
Review comment:
True. Reversed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] a-siuniaev commented on a change in pull request
#1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
Posted by GitBox <gi...@apache.org>.
a-siuniaev commented on a change in pull request #1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
URL: https://github.com/apache/lucene-solr/pull/1122#discussion_r361398204
##########
File path: solr/core/src/java/org/apache/solr/request/json/RequestUtil.java
##########
@@ -214,37 +216,22 @@ public static void processParams(SolrRequestHandler handler, SolrQueryRequest re
out = "rows";
} else if (SORT.equals(key)) {
out = SORT;
+ } else if ("queries".equals(key)) {
+ for (Map.Entry<String, Object> subEntry : ((Map<String, Object>) entry.getValue()).entrySet()) {
+ out = subEntry.getKey();
+ arr = true;
+ isQuery = true;
+ processJsonEntry(newMap, jsonQueryConverter, subEntry, out, isQuery, arr);
+ }
+ continue;
} else if ("params".equals(key) || "facet".equals(key) ) {
// handled elsewhere
continue;
} else {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unknown top-level key in JSON request : " + key);
}
- Object val = entry.getValue();
-
- if (arr) {
- String[] existing = newMap.get(out);
- List lst = val instanceof List ? (List)val : null;
- int existingSize = existing==null ? 0 : existing.length;
- int jsonSize = lst==null ? 1 : lst.size();
- String[] newval = new String[ existingSize + jsonSize ];
- for (int i=0; i<existingSize; i++) {
- newval[i] = existing[i];
- }
- if (lst != null) {
- for (int i = 0; i < jsonSize; i++) {
- Object v = lst.get(i);
- newval[existingSize + i] = isQuery ? jsonQueryConverter.toLocalParams(v, newMap) : v.toString();
- }
- } else {
- newval[newval.length-1] = isQuery ? jsonQueryConverter.toLocalParams(val, newMap) : val.toString();
- }
- newMap.put(out, newval);
- } else {
- newMap.put(out, new String[]{isQuery ? jsonQueryConverter.toLocalParams(val, newMap) : val.toString()});
- }
-
+ processJsonEntry(newMap, jsonQueryConverter, entry, out, isQuery, arr);
Review comment:
Tried my best to refactor the variables' names.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mkhludnev commented on a change in pull request
#1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
Posted by GitBox <gi...@apache.org>.
mkhludnev commented on a change in pull request #1122: SOLR-12490 Added parsing of json.queries for referring in JSON facets
URL: https://github.com/apache/lucene-solr/pull/1122#discussion_r361335644
##########
File path: solr/core/src/java/org/apache/solr/request/json/RequestUtil.java
##########
@@ -214,37 +216,22 @@ public static void processParams(SolrRequestHandler handler, SolrQueryRequest re
out = "rows";
} else if (SORT.equals(key)) {
out = SORT;
+ } else if ("queries".equals(key)) {
+ for (Map.Entry<String, Object> subEntry : ((Map<String, Object>) entry.getValue()).entrySet()) {
Review comment:
Pls check before cast, throw exception with meaningful message and assert 400 http code in test.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org