dsmiley commented on code in PR #4910:
URL: https://github.com/apache/solr/pull/4910#discussion_r4028822401
##########
solr/core/src/java/org/apache/solr/handler/admin/api/UpdateAPI.java:
##########
@@ -65,4 +76,34 @@ public void updateJson(SolrQueryRequest req,
SolrQueryResponse rsp) throws Excep
public void updateJavabin(SolrQueryRequest req, SolrQueryResponse rsp)
throws Exception {
updateRequestHandler.handleRequest(req, rsp);
}
+
+ /**
+ * Whether this request should be routed to the NDJSON loader. The rewritten
path applies to every
+ * content stream of the request, so a request is only treated as NDJSON
when all of its streams
+ * are; anything else keeps the historic {@code /update/json/docs} behavior.
+ */
+ private static boolean isNdJson(SolrQueryRequest req) {
+ String assumed = req.getParams().get(UpdateParams.ASSUME_CONTENT_TYPE);
+ if (assumed != null) {
+ return isNdJsonContentType(assumed);
+ }
+ // Peeking is safe: content streams are always backed by a re-iterable List
Review Comment:
don't need to say that. An Iterable means it's re-iterable... otherwise we
would have returned an Iterator.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/UpdateAPI.java:
##########
@@ -65,4 +76,34 @@ public void updateJson(SolrQueryRequest req,
SolrQueryResponse rsp) throws Excep
public void updateJavabin(SolrQueryRequest req, SolrQueryResponse rsp)
throws Exception {
updateRequestHandler.handleRequest(req, rsp);
}
+
+ /**
+ * Whether this request should be routed to the NDJSON loader. The rewritten
path applies to every
+ * content stream of the request, so a request is only treated as NDJSON
when all of its streams
+ * are; anything else keeps the historic {@code /update/json/docs} behavior.
+ */
+ private static boolean isNdJson(SolrQueryRequest req) {
+ String assumed = req.getParams().get(UpdateParams.ASSUME_CONTENT_TYPE);
+ if (assumed != null) {
+ return isNdJsonContentType(assumed);
+ }
+ // Peeking is safe: content streams are always backed by a re-iterable List
+ Iterable<ContentStream> streams = req.getContentStreams();
+ if (streams == null) {
+ return false;
+ }
+ boolean any = false;
+ for (ContentStream stream : streams) {
+ if (!isNdJsonContentType(stream.getContentType())) {
+ return false;
+ }
+ any = true;
+ }
+ return any;
+ }
+
+ private static boolean isNdJsonContentType(String contentType) {
+ String base = UpdateRequestHandler.baseContentType(contentType);
+ return base != null && NDJsonLoader.CONTENT_TYPES.contains(base);
Review Comment:
null check isn't needed; this whole method could be one line
##########
solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc:
##########
@@ -46,6 +46,10 @@ If you use
xref:configuration-guide:index-segments-merging.adoc#customizing-merg
This requires a fresh Solr 10.1 index.
To retain compatibility with existing sorted indexes, deferring a reindex,
keep `luceneMatchVersion` at 10.3.1 or earlier in solrconfig.xml.
+=== JSON Lines on /update/json/docs is deprecated
+
+The `/update/json/docs` path, and the v2 `/update` and `/update/json` paths
that map to it, currently accept a payload of several whitespace spearated JSON
objects. In a future version, only valid JSON will be allowed for this endpoint
(either a single JSON object or an array of objects). Use the
`/update/json/ndjson` endpoint for JSON Lines (newline-delimited JSON) payloads.
Review Comment:
If we're serious, then use the DeprecationLog to log once
##########
solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-update-handlers.adoc:
##########
@@ -478,6 +479,44 @@ You can also specify `\_version_` with each "delete":
You can specify the version of deletes in the body of the update request as
well.
+=== Newline Delimited JSON
Review Comment:
I often forget about this doc... yet another thing to remember in addition
to the ~mandatory changelog
--
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]
