[
https://issues.apache.org/jira/browse/SHINDIG-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12634213#action_12634213
]
Cassie Doll commented on SHINDIG-601:
-------------------------------------
A couple of comments on this patch -
- watch your spacing. if statements should look like this "if (contentType !=
null) {" space before open paren, no space after open paren. space between
close paren and bracket
- make sure you aren't using any tab chars
- unfortunately, your patch just got out of sync because of another patch which
added a BeanAtomConverter... so now atom and xml are separate
- this method has a test in DataServiceServletTest. make sure you are running
it with mvn test and add new test cases to it.
- the "CONTENT_TYPE" string and all other strings should be pulled into
constants at the top of the file (next to FORMAT_PARAM and the others)
And one last overall comment. I don't think Chris's original description on
this bug was clear enough. Once we add this content type detection code in we
really need to use two separate converters. One to parse the incoming post data
and one to format our output. So then if you posted xml with format=json things
would work as they should. (Seems silly, but it shouldn't be hard to do it
right I suppose)
This shouldn't be too hard to change in the DataServiceServlet - are you up for
it?
Sorry for the large amount of comments and thanks for your help!
> Input format detection
> ----------------------
>
> Key: SHINDIG-601
> URL: https://issues.apache.org/jira/browse/SHINDIG-601
> Project: Shindig
> Issue Type: Bug
> Components: RESTful API (Java), RESTful API (PHP)
> Reporter: Chris Chabot
> Priority: Blocker
> Attachments: fix-601-bug.patch, inputContentType.patch
>
>
> Currently PHP uses the content_type header to detect the input format. On the
> other hand Java uses the format query param (?format=foo) for the input
> format selection.
> Most logical solution seems to be that we both use :
> if ( content type is set)
> use content_type
> else if (format query param is set)
> use query param
> else
> use json
> I *think* that will be what developers would expect :)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
