Solr
  1. Solr
  2. SOLR-1123

Change the JSONResponseWriter content type

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.5, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None

      Description

      Currently the jSON content type is not used. Instead the palin/text content type is used. The reason for this as I understand is to enable viewing the json response as as text in the browser. While this is valid argument, I do believe that there should at least be an option to configure this writer to use the JSON content type. According to RFC4627 the json content type needs to be application/json (and not text/x-json). The reason this can be very helpful is that today you have plugins for browsers (e.g. JSONView) that can render any page with application/json content type in a user friendly manner (just like xml is supported).

      1. JSON_contentType_incl_tests.patch
        3 kB
        Uri Boness
      2. SOLR-1123.patch
        2 kB
        Chris Male
      3. SOLR-1123.patch
        2 kB
        Chris Male

        Activity

        Hide
        Uri Boness added a comment -

        This patch is a simple implementation for this functionality. The writer can be configured with a userJsonContentType boolean parameter that when set to true the content type for the output will be "application/json" instead of "text/plain". For backward compatibility reasons, when this parameter is absent, the "text/plain" content type will be used.

        Show
        Uri Boness added a comment - This patch is a simple implementation for this functionality. The writer can be configured with a userJsonContentType boolean parameter that when set to true the content type for the output will be "application/json" instead of "text/plain". For backward compatibility reasons, when this parameter is absent, the "text/plain" content type will be used.
        Hide
        Shalin Shekhar Mangar added a comment -

        Marking for 1.5

        I'm -0 on this. I'm not sure if this inconvenience is big enough to warrant adding a request parameter. I'll let others decide this one.

        Show
        Shalin Shekhar Mangar added a comment - Marking for 1.5 I'm -0 on this. I'm not sure if this inconvenience is big enough to warrant adding a request parameter. I'll let others decide this one.
        Hide
        Uri Boness added a comment -

        Indeed this is just for convenience and should not be in a high priority, but I definitely see it as a nice to have one. Just to clarify, the suggestion is not to have another request parameter (that would probably be too much as you mentioned) but instead add a configuration parameter in solrconfig. So you'll be able to define the json response writer as follows:

        <queryResponseWriter name="json" class="org.apache.solr.request.JSONResponseWriter">
            <bool name="useJsonContentType">true</bool>
        </queryResponseWriter>
        
        Show
        Uri Boness added a comment - Indeed this is just for convenience and should not be in a high priority, but I definitely see it as a nice to have one. Just to clarify, the suggestion is not to have another request parameter (that would probably be too much as you mentioned) but instead add a configuration parameter in solrconfig. So you'll be able to define the json response writer as follows: <queryResponseWriter name= "json" class= "org.apache.solr.request.JSONResponseWriter" > <bool name= "useJsonContentType" > true </bool> </queryResponseWriter>
        Hide
        Yonik Seeley added a comment -

        Is there perhaps a more general feature we could turn this into? An expert level ability or parameter to set a custom content-type?

        Show
        Yonik Seeley added a comment - Is there perhaps a more general feature we could turn this into? An expert level ability or parameter to set a custom content-type?
        Hide
        Uri Boness added a comment -

        I think that would be the best option. The problem right now is in the current class hierarchy of the response writers. Basically, I think the QueryResponseWriter interface should change to:

        public interface QueryResponseWriter extends NamedListInitializedPlugin {
         
          public void write(OutputStream out, SolrQueryRequest request, SolrQueryResponse response) throws IOException;
        
          public String getContentType(SolrQueryRequest request, SolrQueryResponse response);
        
        }
        

        Note: this interface will play nicer with the binary response writer

        Then we can have an AbstractTextResponseWriter which will serve as a parent for all non-binary response writers:

        public abstract class AbstractTextResponseWriter extends NamedListInitializedPlugin {
        
          public final static String CONTENT_TYPE_PARAM = "contentType";
          public static String DEFAULT_CONTENT_TYPE="text/plain; charset=UTF-8";
          
          private final String contentType;
          
          protected AbstractTextResponseWriter() {
            this(DEFAULT_CONTENT_TYPE);
          }
        
          protected AbstractTextResponseWriter(String defaultContentType) {
            this.contentType = defaultContentType;
          }
        
          public void init(NamedList args) {
            String configuredContentType = (String) args.get(CONTENT_TYPE_PARAM);
            if (configuredContentType != null) {
              contentType = configuredContentType;;
            }
          }
        
          public String getContentType(SolrQueryRequest request, SolrQueryResponse response) {
            return contentType;
          }
         
          public final void write(OutputStream out, SolrQueryRequest request, SolrQueryResponse response) throws IOException {
            OutputStreamWriter writer = new OutputStreamWriter(out, "UTF-8");
            write(writer, request, response);
          }
        
          protected abstract void write(Writer writer, SolrQueryRequest request, SolrQueryResponse response) throws IOException;
        
        }
        

        This will make it easy for every response writer to define its default content type, yet it will still allow to override this default using the "contentType" parameter in solrconfig. (I assume here that there's no need to customize the content type for the binary response writer as it's internal and specific for the current implementation).

        Show
        Uri Boness added a comment - I think that would be the best option. The problem right now is in the current class hierarchy of the response writers. Basically, I think the QueryResponseWriter interface should change to: public interface QueryResponseWriter extends NamedListInitializedPlugin { public void write(OutputStream out, SolrQueryRequest request, SolrQueryResponse response) throws IOException; public String getContentType(SolrQueryRequest request, SolrQueryResponse response); } Note: this interface will play nicer with the binary response writer Then we can have an AbstractTextResponseWriter which will serve as a parent for all non-binary response writers: public abstract class AbstractTextResponseWriter extends NamedListInitializedPlugin { public final static String CONTENT_TYPE_PARAM = "contentType" ; public static String DEFAULT_CONTENT_TYPE= "text/plain; charset=UTF-8" ; private final String contentType; protected AbstractTextResponseWriter() { this (DEFAULT_CONTENT_TYPE); } protected AbstractTextResponseWriter( String defaultContentType) { this .contentType = defaultContentType; } public void init(NamedList args) { String configuredContentType = ( String ) args.get(CONTENT_TYPE_PARAM); if (configuredContentType != null ) { contentType = configuredContentType;; } } public String getContentType(SolrQueryRequest request, SolrQueryResponse response) { return contentType; } public final void write(OutputStream out, SolrQueryRequest request, SolrQueryResponse response) throws IOException { OutputStreamWriter writer = new OutputStreamWriter(out, "UTF-8" ); write(writer, request, response); } protected abstract void write(Writer writer, SolrQueryRequest request, SolrQueryResponse response) throws IOException; } This will make it easy for every response writer to define its default content type, yet it will still allow to override this default using the "contentType" parameter in solrconfig. (I assume here that there's no need to customize the content type for the binary response writer as it's internal and specific for the current implementation).
        Hide
        Shalin Shekhar Mangar added a comment - - edited

        Or we could just add a request parameter and SolrDispatchFilter can set the value of the param as the content type.

        Show
        Shalin Shekhar Mangar added a comment - - edited Or we could just add a request parameter and SolrDispatchFilter can set the value of the param as the content type.
        Hide
        Uri Boness added a comment -

        Yeah, that's also an option. The only drawbacks like we mentioned above is that you'll need to add yet another request parameter. And I also thought it was a good opportunity to "fix"/"clean" the class hierarchy of the response writer. But indeed this is a quicker fix to the problem.

        Show
        Uri Boness added a comment - Yeah, that's also an option. The only drawbacks like we mentioned above is that you'll need to add yet another request parameter. And I also thought it was a good opportunity to "fix"/"clean" the class hierarchy of the response writer. But indeed this is a quicker fix to the problem.
        Hide
        Noble Paul added a comment -

        Should we make responsewriters use the GenericTextResponseWriter SOLR-1516?

        Show
        Noble Paul added a comment - Should we make responsewriters use the GenericTextResponseWriter SOLR-1516 ?
        Hide
        Yonik Seeley added a comment -

        I never got a chance to check out SOLR-1516, but my understanding was that it was a response writer that made it easy for custom response writers to inherit from? I'm not sure we should introduce that as a dependency for the standard response writers.

        Show
        Yonik Seeley added a comment - I never got a chance to check out SOLR-1516 , but my understanding was that it was a response writer that made it easy for custom response writers to inherit from? I'm not sure we should introduce that as a dependency for the standard response writers.
        Hide
        Noble Paul added a comment -

        SOLR-1516 has the potential to simplify our existing responsewriters .

        Show
        Noble Paul added a comment - SOLR-1516 has the potential to simplify our existing responsewriters .
        Hide
        Uri Boness added a comment -

        I think the main issue with the inheritance right now is that the QueryResponseWriter interface is dealing with a Writer rather than with an OutputStream. This accounts for the hacky GenericBinaryResponseWriter.

        Looking at SOLR-1516 I'm a bit confused. I always had the impression that the main idea behind the response writers is that all they need to know is how to marshal a NamedList (so they don't need explicit knowledge of documents, highlighting, etc...). But now the GenericTextResponseWriter knows about documents (via the SingleResponseWriter). But perhaps I just go it wrong.

        Show
        Uri Boness added a comment - I think the main issue with the inheritance right now is that the QueryResponseWriter interface is dealing with a Writer rather than with an OutputStream. This accounts for the hacky GenericBinaryResponseWriter. Looking at SOLR-1516 I'm a bit confused. I always had the impression that the main idea behind the response writers is that all they need to know is how to marshal a NamedList (so they don't need explicit knowledge of documents, highlighting, etc...). But now the GenericTextResponseWriter knows about documents (via the SingleResponseWriter). But perhaps I just go it wrong.
        Hide
        Chris A. Mattmann added a comment -

        I think the main issue with the inheritance right now is that the QueryResponseWriter interface is dealing with a Writer rather than with an OutputStream. This accounts for the hacky GenericBinaryResponseWriter.

        I'm taking a look at this.

        Looking at SOLR-1516 I'm a bit confused. I always had the impression that the main idea behind the response writers is that all they need to know is how to marshal a NamedList (so they don't need explicit knowledge of documents, highlighting, etc...). But now the GenericTextResponseWriter knows about documents (via the SingleResponseWriter). But perhaps I just go it wrong.

        Well if that's the main idea behind ResponseWriters as you put it, then as I put it in SOLR-1516, it's pretty confusing. Users (who understand Lucene and SOLR) know that if they query they get back o.a.lucene.Documents or o.a.solr.SolrDocumentList, etc. The whole NamedList structure is pretty confusing to me (and to others as I've noted on other issues and on the mailing list). SOLR-1516 was an attempt to give those people writing ResponseWriters (now) the ability to deal with results of queries, aka Documents (and not all the other NamedList typeless bag of objects where you have to do instanceof everwhere to unmarshall it). Clearly not all ResponseWriters will be able to take advantage of this (e.g., those that need the specified output structures provided by other components, e.g., Facets, etc.), which is why my original patch called the two response writers I added Document*ResponseWriter b/c that's what they dealt with.

        Cheers,
        Chris

        Show
        Chris A. Mattmann added a comment - I think the main issue with the inheritance right now is that the QueryResponseWriter interface is dealing with a Writer rather than with an OutputStream. This accounts for the hacky GenericBinaryResponseWriter. I'm taking a look at this. Looking at SOLR-1516 I'm a bit confused. I always had the impression that the main idea behind the response writers is that all they need to know is how to marshal a NamedList (so they don't need explicit knowledge of documents, highlighting, etc...). But now the GenericTextResponseWriter knows about documents (via the SingleResponseWriter). But perhaps I just go it wrong. Well if that's the main idea behind ResponseWriters as you put it, then as I put it in SOLR-1516 , it's pretty confusing. Users (who understand Lucene and SOLR) know that if they query they get back o.a.lucene.Documents or o.a.solr.SolrDocumentList, etc. The whole NamedList structure is pretty confusing to me (and to others as I've noted on other issues and on the mailing list). SOLR-1516 was an attempt to give those people writing ResponseWriters (now) the ability to deal with results of queries, aka Documents (and not all the other NamedList typeless bag of objects where you have to do instanceof everwhere to unmarshall it). Clearly not all ResponseWriters will be able to take advantage of this (e.g., those that need the specified output structures provided by other components, e.g., Facets, etc.), which is why my original patch called the two response writers I added Document*ResponseWriter b/c that's what they dealt with. Cheers, Chris
        Hide
        Noble Paul added a comment -

        I always had the impression that the main idea behind the response writers is that all they need to know is how to marshal a NamedList ...

        That is the problem. the NamedList is a weird datastructure for those who are not so used to Solr. You don't know what is included in that unless you do an instanceof. Most of the users are happy to write out the documents . understanding a SolrDocument is far easier than figuring outhow to handle a DocList .So it is an attempt to cater to those needs .

        If you know how to handle the NamedList beast then you can do that also ( but only if you wish to).

        Show
        Noble Paul added a comment - I always had the impression that the main idea behind the response writers is that all they need to know is how to marshal a NamedList ... That is the problem. the NamedList is a weird datastructure for those who are not so used to Solr. You don't know what is included in that unless you do an instanceof. Most of the users are happy to write out the documents . understanding a SolrDocument is far easier than figuring outhow to handle a DocList .So it is an attempt to cater to those needs . If you know how to handle the NamedList beast then you can do that also ( but only if you wish to).
        Hide
        Lance Norskog added a comment -

        It should be possible to pass an arbitrary set of parameters to a ResponseWriter. I added this feature to XSLTResponseWriter in a patch; it made it possible to genericize XSL scripts.

        SOLR-1354

        Since 'tr' chooses the XSLT script, I made 'tr.param=x' pass 'param=x' into the XSL interpreter. 'tr.mime-type=application/json' would be used by the JSON interpreter.

        I did not have to change anything outside of XSLTResponseWriter.

        Show
        Lance Norskog added a comment - It should be possible to pass an arbitrary set of parameters to a ResponseWriter. I added this feature to XSLTResponseWriter in a patch; it made it possible to genericize XSL scripts. SOLR-1354 Since 'tr' chooses the XSLT script, I made 'tr.param=x' pass 'param=x' into the XSL interpreter. 'tr.mime-type=application/json' would be used by the JSON interpreter. I did not have to change anything outside of XSLTResponseWriter.
        Hide
        Lance Norskog added a comment -

        Thinking about it again, 'wt' selects the writer, so parameters to the JSON writer should be 'wt.param=value'. 'tr' is the transformer script, and 'tr.param=value' should pass data through to the transformer code. So, to handle this problem I would use 'wt=json&wt.mime-type=application/json'.

        Show
        Lance Norskog added a comment - Thinking about it again, 'wt' selects the writer, so parameters to the JSON writer should be 'wt.param=value'. 'tr' is the transformer script, and 'tr.param=value' should pass data through to the transformer code. So, to handle this problem I would use 'wt=json&wt.mime-type=application/json'.
        Hide
        Hoss Man added a comment -

        Bulk updating 240 Solr issues to set the Fix Version to "next" per the process outlined in this email...

        http://mail-archives.apache.org/mod_mbox/lucene-dev/201005.mbox/%3Calpine.DEB.1.10.1005251052040.24672@radix.cryptio.net%3E

        Selection criteria was "Unresolved" with a Fix Version of 1.5, 1.6, 3.1, or 4.0. email notifications were suppressed.

        A unique token for finding these 240 issues in the future: hossversioncleanup20100527

        Show
        Hoss Man added a comment - Bulk updating 240 Solr issues to set the Fix Version to "next" per the process outlined in this email... http://mail-archives.apache.org/mod_mbox/lucene-dev/201005.mbox/%3Calpine.DEB.1.10.1005251052040.24672@radix.cryptio.net%3E Selection criteria was "Unresolved" with a Fix Version of 1.5, 1.6, 3.1, or 4.0. email notifications were suppressed. A unique token for finding these 240 issues in the future: hossversioncleanup20100527
        Hide
        Lorrin Nelson added a comment -

        I propose treating this as two issues:

        A short-term bug fix to properly return JSON data with the standard application/json ContentType

        A low-priority feature request to allow customizing the JSON writer to return non-standard ContentType

        Show
        Lorrin Nelson added a comment - I propose treating this as two issues: A short-term bug fix to properly return JSON data with the standard application/json ContentType A low-priority feature request to allow customizing the JSON writer to return non-standard ContentType
        Hide
        Chris Tucker added a comment -

        I'd like to +1 the short-term fix. The incorrect content type makes it difficult to filter/transform the response in a servlet filter or Jetty handler: one has to inspect the wt parameter on the request to establish (guess?) that JSON has been requested and is being sent back.

        Show
        Chris Tucker added a comment - I'd like to +1 the short-term fix. The incorrect content type makes it difficult to filter/transform the response in a servlet filter or Jetty handler: one has to inspect the wt parameter on the request to establish (guess?) that JSON has been requested and is being sent back.
        Hide
        Ingo Renner added a comment -

        +1 for the short term fix

        Show
        Ingo Renner added a comment - +1 for the short term fix
        Hide
        Robert Muir added a comment -

        Bulk move 3.2 -> 3.3

        Show
        Robert Muir added a comment - Bulk move 3.2 -> 3.3
        Hide
        Neil Hooey added a comment -

        Can we revive this issue?

        Does anyone have a patch for the short-term fix?

        Show
        Neil Hooey added a comment - Can we revive this issue? Does anyone have a patch for the short-term fix?
        Hide
        Robert Muir added a comment -

        3.4 -> 3.5

        Show
        Robert Muir added a comment - 3.4 -> 3.5
        Hide
        Ralph added a comment -

        I agree with Neil, can someone post about a solution. I tried some of the options listed here and none seem to be active.

        Thanks,
        Ralph

        Show
        Ralph added a comment - I agree with Neil, can someone post about a solution. I tried some of the options listed here and none seem to be active. Thanks, Ralph
        Hide
        Chris Male added a comment -

        I had assumed this had been dealt with. It seems we should just break backwards compat on trunk and change the JSONResponseWriter to return application/json. Why continue to return text?

        Show
        Chris Male added a comment - I had assumed this had been dealt with. It seems we should just break backwards compat on trunk and change the JSONResponseWriter to return application/json. Why continue to return text?
        Hide
        Yonik Seeley added a comment -

        Why continue to return text?

        This was discussed in a different issue (it has come up a number of times before). At the time, returning application/json as the content-type would cause all of the major browsers to fail to display the JSON and some would prompt to download it and some would prompt to search for an application that could handle it. Using "text" was much more user friendly. Clients that are talking to solr and send "wt=json" or "wt=xml" know exactly what they are going to get back anyway.

        Show
        Yonik Seeley added a comment - Why continue to return text? This was discussed in a different issue (it has come up a number of times before). At the time, returning application/json as the content-type would cause all of the major browsers to fail to display the JSON and some would prompt to download it and some would prompt to search for an application that could handle it. Using "text" was much more user friendly. Clients that are talking to solr and send "wt=json" or "wt=xml" know exactly what they are going to get back anyway.
        Hide
        Yonik Seeley added a comment -

        Why continue to return text?

        This was discussed in a different issue (it has come up a number of times before). At the time, returning application/json as the content-type would cause all of the major browsers to fail to display the JSON and some would prompt to download it and some would prompt to search for an application that could handle it. Using "text" was much more user friendly. Clients that are talking to solr and send "wt=json" or "wt=xml" know exactly what they are going to get back anyway.

        Show
        Yonik Seeley added a comment - Why continue to return text? This was discussed in a different issue (it has come up a number of times before). At the time, returning application/json as the content-type would cause all of the major browsers to fail to display the JSON and some would prompt to download it and some would prompt to search for an application that could handle it. Using "text" was much more user friendly. Clients that are talking to solr and send "wt=json" or "wt=xml" know exactly what they are going to get back anyway.
        Hide
        Chris Male added a comment -

        Okay, what about today when there are plenty of extensions for the major browsers to correctly display JSON? Do we really need to support users do a manual Solr request through their browser and returning JSON? If they want that, they can use a text protocol like XML.

        Show
        Chris Male added a comment - Okay, what about today when there are plenty of extensions for the major browsers to correctly display JSON? Do we really need to support users do a manual Solr request through their browser and returning JSON? If they want that, they can use a text protocol like XML.
        Hide
        Ryo Hideno added a comment -

        +1

        Show
        Ryo Hideno added a comment - +1
        Hide
        Yonik Seeley added a comment -

        Okay, what about today when there are plenty of extensions for the major browsers to correctly display JSON?

        Requiring the user to install an extension to display a response isn't that friendly either.
        We should think about the number of users who would be inconvenienced (and in what manner) using one default vs the other.
        The most flexible approach would allow the user to set the content-type, but the default should remain what gives the best user experience.

        Show
        Yonik Seeley added a comment - Okay, what about today when there are plenty of extensions for the major browsers to correctly display JSON? Requiring the user to install an extension to display a response isn't that friendly either. We should think about the number of users who would be inconvenienced (and in what manner) using one default vs the other. The most flexible approach would allow the user to set the content-type, but the default should remain what gives the best user experience.
        Hide
        Chris Tucker added a comment -

        Is there really such a large use case for users who don't have a JSON plugin installed but do need to view JSON results in the browser? An XML response will almost certainly be rendered more readably for those users (in which case the JSON point is moot), and if the user does intend to view JSON frequently they simply install a JSON plugin: reading JSON as a big lump of text without the aid of a plugin is not a manageable solution for most people I know. Intentionally returning a known-wrong content type to work around a client issue for casual users while causing a much bigger problem (thwarting well-behaved clients, JSON plugins, and filters) for users who actually need to work with this seems like a poor choice. I'd argue the default should be changed to application/json immediately, and a separate (lower-priority) ticket raised to address allowing the user to override the content-type in the response if they so desire.

        Show
        Chris Tucker added a comment - Is there really such a large use case for users who don't have a JSON plugin installed but do need to view JSON results in the browser? An XML response will almost certainly be rendered more readably for those users (in which case the JSON point is moot), and if the user does intend to view JSON frequently they simply install a JSON plugin: reading JSON as a big lump of text without the aid of a plugin is not a manageable solution for most people I know. Intentionally returning a known-wrong content type to work around a client issue for casual users while causing a much bigger problem (thwarting well-behaved clients, JSON plugins, and filters) for users who actually need to work with this seems like a poor choice. I'd argue the default should be changed to application/json immediately, and a separate (lower-priority) ticket raised to address allowing the user to override the content-type in the response if they so desire.
        Hide
        Chris Male added a comment - - edited

        I agree with Chris wholeheartedly.

        It doesn't seem to be user-friendly to return a content-type that isn't inline with the RFC standard and I can't help but feel that the number of users who will be negatively impacted by this change will be small. Those users can then either use XML or some other text format, or install a JSON plugin for their browser.

        There's a reason this keeps coming up in discussion.

        Chris, can you put together a patch which changes the content-type?

        Show
        Chris Male added a comment - - edited I agree with Chris wholeheartedly. It doesn't seem to be user-friendly to return a content-type that isn't inline with the RFC standard and I can't help but feel that the number of users who will be negatively impacted by this change will be small. Those users can then either use XML or some other text format, or install a JSON plugin for their browser. There's a reason this keeps coming up in discussion. Chris, can you put together a patch which changes the content-type?
        Hide
        Yonik Seeley added a comment -

        Is there really such a large use case for users who don't have a JSON plugin installed but do need to view JSON results in the browser?

        JSON has been all the rage for the past number of years - tons of people have been programmed to say "yuck XML, yay JSON, YAML, etc".
        In my experience, very few people have JSON plugins installed, and the out-of-the-box experience will be horrible for those people (JSON is even used in our beginner tutorial).

        Show
        Yonik Seeley added a comment - Is there really such a large use case for users who don't have a JSON plugin installed but do need to view JSON results in the browser? JSON has been all the rage for the past number of years - tons of people have been programmed to say "yuck XML, yay JSON, YAML, etc". In my experience, very few people have JSON plugins installed, and the out-of-the-box experience will be horrible for those people (JSON is even used in our beginner tutorial).
        Hide
        Chris Tucker added a comment -

        JSON is certainly popular, and I agree that many users have a visceral reaction against XML. However, we're talking here about maintaining current behavior to support one very specific set of users: those who (1) are not committed enough to JSON to know how to view it (a plugin, Firebug, or what have you, which they will need for every other case where they get JSON from any other service out there) but (2) are committed enough to it to not be able/willing to use the (arguably much better) XML visualization tools built into every browser. This just seems like a very small population of users. Meanwhile, the current behavior thwarts many serious users who do want to use a JSON plugin, makes it impossible to write content-type aware servlet filters (which was my use case way back when), and violates the expectation that a response will have the RFC defined content type associated with it. IMHO the cost vastly outweighs the benefit here: if need be, put a note on the wiki in the tutorial where JSON is mentioned (which, as far as I can tell, is simply to say "hey, you can get the results in JSON, too!") pointing people to a browser plugin and be done with it.

        Show
        Chris Tucker added a comment - JSON is certainly popular, and I agree that many users have a visceral reaction against XML. However, we're talking here about maintaining current behavior to support one very specific set of users: those who (1) are not committed enough to JSON to know how to view it (a plugin, Firebug, or what have you, which they will need for every other case where they get JSON from any other service out there) but (2) are committed enough to it to not be able/willing to use the (arguably much better) XML visualization tools built into every browser. This just seems like a very small population of users. Meanwhile, the current behavior thwarts many serious users who do want to use a JSON plugin, makes it impossible to write content-type aware servlet filters (which was my use case way back when), and violates the expectation that a response will have the RFC defined content type associated with it. IMHO the cost vastly outweighs the benefit here: if need be, put a note on the wiki in the tutorial where JSON is mentioned (which, as far as I can tell, is simply to say "hey, you can get the results in JSON, too!") pointing people to a browser plugin and be done with it.
        Hide
        Uwe Schindler added a comment -

        +1 to change the content type to the official one. I was about to do this together with another ResponseWriter change once ago... the wrong type is a problem for all users actually using browsers with plugins. If Solr would send xml without application/xml I would complain, too, as e.g. Internet Explorer has the best xml viewer on earth (sorry, FF and Chrome have broken namespace prefix support).

        Show
        Uwe Schindler added a comment - +1 to change the content type to the official one. I was about to do this together with another ResponseWriter change once ago... the wrong type is a problem for all users actually using browsers with plugins. If Solr would send xml without application/xml I would complain, too, as e.g. Internet Explorer has the best xml viewer on earth (sorry, FF and Chrome have broken namespace prefix support).
        Hide
        Yonik Seeley added a comment -

        The downside for the small minority of users likely to have a browser JSON plugin installed is that they will still see a text JSON response. This is a much more graceful fallback, and we can add an optional parameter to change the content-type. It seems pretty clear to me that the default content-type should remain as is for the best user experience for the majority of new users. I originally coded the content-type as something with x-json in it (Solr's JSON support pre-dates the RFC), but sending a query to solr and being prompted to download the reply or search the internet for a plugin is horrible and will definitely turn people off.

        And don't get me started on the overreaching JSON RFC wrt specifying encodings.

        Show
        Yonik Seeley added a comment - The downside for the small minority of users likely to have a browser JSON plugin installed is that they will still see a text JSON response. This is a much more graceful fallback, and we can add an optional parameter to change the content-type. It seems pretty clear to me that the default content-type should remain as is for the best user experience for the majority of new users. I originally coded the content-type as something with x-json in it (Solr's JSON support pre-dates the RFC), but sending a query to solr and being prompted to download the reply or search the internet for a plugin is horrible and will definitely turn people off. And don't get me started on the overreaching JSON RFC wrt specifying encodings.
        Hide
        Hoss Man added a comment -

        Personally i'm a fan of the proposal that was earlier suggested to rethink the response writer API so we can add a generalized SolrParam for overriding the default Content-Type of any response writer, and then letting the example specify "text/plain" as a default to make the tutorial easy to read.

        but independent of that (since it's a much bigger issue), why don't we move forward by:

        • changing the default mime type for the writer to be "application/json"
        • add an init param on the writer to override the mime type used (much like the patch does, but i'm not a fan of the proposed param name nor of making it a simple boolean)
        • update the example solrconfig.xml to explicitly use this init param to set the type to "text/plain" with a comment like so...
        <queryResponseWriter name="json" class="solr.JSONResponseWriter">
          <!-- For the purposes of the tutorial, JSON response are written as 
               plain text so that it's easy to read in *any* browser.  
               If you are building applications that consume JSON, just remove
               this override to get the default "application/json" mime type.
            -->
          <str name="content.type">text/plain</str>
        </queryResponseWriter>
        

        ...which seems like a good compromise all the way around: The default mime type of the writer starts matching the expectations of advanced users, the example configs make the tutorial output easy to read for novices in any browser, and there is clear indication in those example configs how people can change the content type used (crazy people can even use "text/json-x" if they really want to)

        objections?

        (If/when we adding a more general request time mime type over ride param, this init param can still be used as a fallback, so it doesn't really paint us into a corner or anything)

        Show
        Hoss Man added a comment - Personally i'm a fan of the proposal that was earlier suggested to rethink the response writer API so we can add a generalized SolrParam for overriding the default Content-Type of any response writer, and then letting the example specify "text/plain" as a default to make the tutorial easy to read. but independent of that (since it's a much bigger issue), why don't we move forward by: changing the default mime type for the writer to be "application/json" add an init param on the writer to override the mime type used (much like the patch does, but i'm not a fan of the proposed param name nor of making it a simple boolean) update the example solrconfig.xml to explicitly use this init param to set the type to "text/plain" with a comment like so... <queryResponseWriter name= "json" class= "solr.JSONResponseWriter" > <!-- For the purposes of the tutorial, JSON response are written as plain text so that it's easy to read in *any* browser. If you are building applications that consume JSON, just remove this override to get the default "application/json" mime type. --> <str name= "content.type" >text/plain</str> </queryResponseWriter> ...which seems like a good compromise all the way around: The default mime type of the writer starts matching the expectations of advanced users, the example configs make the tutorial output easy to read for novices in any browser, and there is clear indication in those example configs how people can change the content type used (crazy people can even use "text/json-x" if they really want to) objections? (If/when we adding a more general request time mime type over ride param, this init param can still be used as a fallback, so it doesn't really paint us into a corner or anything)
        Hide
        Chris Tucker added a comment -

        Sounds awesome.

        Show
        Chris Tucker added a comment - Sounds awesome.
        Hide
        Chris Male added a comment -

        Patch which implements what Hossman has suggested. I've verified that the content-type changes based on the configured parameter.

        Show
        Chris Male added a comment - Patch which implements what Hossman has suggested. I've verified that the content-type changes based on the configured parameter.
        Hide
        Uwe Schindler added a comment -

        +1

        Show
        Uwe Schindler added a comment - +1
        Hide
        Chris Male added a comment -

        As there doesn't seem to be any objections, I plan to commit this shortly.

        Show
        Chris Male added a comment - As there doesn't seem to be any objections, I plan to commit this shortly.
        Hide
        Yonik Seeley added a comment -

        Since this is literally the Content-Type header, should we use a parameter name like "content-type" or "Content-Type" rather than content.type? Or are there other content.foo params that would make scoping them all under "content." make more sense?

        +      If you are building applications that consume JSON, just remove
        +      this override to get the default "application/json" mime type.
        

        This part makes it sound like you need to remove it... but most programmatic clients won't care (or will actually work better with text since the charset is defined). Let's change that part to something like:

        If your browser or application expects a MIME type of "application/json",
        just remove this override.
        
        Show
        Yonik Seeley added a comment - Since this is literally the Content-Type header, should we use a parameter name like "content-type" or "Content-Type" rather than content.type? Or are there other content.foo params that would make scoping them all under "content." make more sense? + If you are building applications that consume JSON, just remove + this override to get the default "application/json" mime type. This part makes it sound like you need to remove it... but most programmatic clients won't care (or will actually work better with text since the charset is defined). Let's change that part to something like: If your browser or application expects a MIME type of "application/json" , just remove this override.
        Hide
        Chris Male added a comment -

        Since this is literally the Content-Type header, should we use a parameter name like "content-type" or "Content-Type" rather than content.type? Or are there other content.foo params that would make scoping them all under "content." make more sense?

        Yup 'content-type' makes the most sense.

        This part makes it sound like you need to remove it... but most programmatic clients won't care (or will actually work better with text since the charset is defined).

        The reason this issue keeps getting brought up is that clients do care. But I will make the change you suggest since it does make it clearer.

        Show
        Chris Male added a comment - Since this is literally the Content-Type header, should we use a parameter name like "content-type" or "Content-Type" rather than content.type? Or are there other content.foo params that would make scoping them all under "content." make more sense? Yup 'content-type' makes the most sense. This part makes it sound like you need to remove it... but most programmatic clients won't care (or will actually work better with text since the charset is defined). The reason this issue keeps getting brought up is that clients do care. But I will make the change you suggest since it does make it clearer.
        Hide
        Chris Male added a comment -

        New patch changing the parameter name and improving the documentation.

        Show
        Chris Male added a comment - New patch changing the parameter name and improving the documentation.
        Hide
        Hoss Man added a comment -

        Chris: any reason not to commit and get this into 3.5?

        Show
        Hoss Man added a comment - Chris: any reason not to commit and get this into 3.5?
        Hide
        Chris Male added a comment - - edited

        Hey Hoss,

        I haven't had a chance to commit it to trunk yet either (it fell between the cracks), I'll commit it to both today.

        Show
        Chris Male added a comment - - edited Hey Hoss, I haven't had a chance to commit it to trunk yet either (it fell between the cracks), I'll commit it to both today.
        Hide
        Simon Willnauer added a comment -

        +1 I will roll a release candidate on monday.... I didn't finish it this week

        Show
        Simon Willnauer added a comment - +1 I will roll a release candidate on monday.... I didn't finish it this week
        Hide
        Chris Male added a comment -

        Committed to trunk in revision 1204327.

        Show
        Chris Male added a comment - Committed to trunk in revision 1204327.
        Hide
        Chris Male added a comment -

        Committed to 3x in revision 1204329.

        Show
        Chris Male added a comment - Committed to 3x in revision 1204329.
        Hide
        Chris Male added a comment -

        Fixed.

        Show
        Chris Male added a comment - Fixed.
        Hide
        Mark Miller added a comment -

        FWIW: we should probably add Uri to changes as well, as he filed the issue.

        Show
        Mark Miller added a comment - FWIW: we should probably add Uri to changes as well, as he filed the issue.
        Hide
        Chris Male added a comment -

        Good call Mark, done in both trunk and 3x.

        Show
        Chris Male added a comment - Good call Mark, done in both trunk and 3x.
        Hide
        Uwe Schindler added a comment -

        Bulk close after 3.5 is released

        Show
        Uwe Schindler added a comment - Bulk close after 3.5 is released

          People

          • Assignee:
            Chris Male
            Reporter:
            Uri Boness
          • Votes:
            13 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development