Author: hsaputra
Date: Thu Mar 24 07:05:13 2011
New Revision: 1084860
URL: http://svn.apache.org/viewvc?rev=1084860&view=rev
Log:
SHINDIG-1515 | Why shindig uses "Content-Type" not "Accept" to determine the
return info type.
CR: http://codereview.appspot.com/4282058
The patch contains change to separate logic to get converter for request and
response.
For response, it will use"format" request parameter as defined in the
OpenSocial Core API server spec.
So with this change you can send POST request with Content-Type of XML but
request return of format JSON by specifiying format=json in the URL parameter.
Modified:
shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/DataServiceServlet.java
shindig/trunk/java/common/src/test/java/org/apache/shindig/protocol/DataServiceServletTest.java
shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulAtomActivityEntryTest.java
shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulXmlActivityEntryTest.java
Modified:
shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/DataServiceServlet.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/DataServiceServlet.java?rev=1084860&r1=1084859&r2=1084860&view=diff
==============================================================================
---
shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/DataServiceServlet.java
(original)
+++
shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/DataServiceServlet.java
Thu Mar 24 07:05:13 2011
@@ -17,7 +17,9 @@
*/
package org.apache.shindig.protocol;
+import org.apache.commons.lang.StringUtils;
import org.apache.shindig.auth.SecurityToken;
+import org.apache.shindig.common.Nullable;
import org.apache.shindig.common.servlet.HttpUtil;
import org.apache.shindig.protocol.conversion.BeanConverter;
@@ -110,9 +112,7 @@ public class DataServiceServlet extends
HttpUtil.setCORSheader(servletResponse,
containerConfig.<String>getList(token.getContainer(),
"gadgets.parentOrigins"));
- BeanConverter converter = getConverterForRequest(servletRequest);
-
- handleSingleRequest(servletRequest, servletResponse, token, converter);
+ handleSingleRequest(servletRequest, servletResponse, token);
}
@Override
@@ -149,12 +149,39 @@ public class DataServiceServlet extends
* Handler for non-batch requests.
*/
private void handleSingleRequest(HttpServletRequest servletRequest,
- HttpServletResponse servletResponse, SecurityToken token,
- BeanConverter converter) throws IOException {
+ HttpServletResponse servletResponse, SecurityToken token) throws
IOException {
// Always returns a non-null handler.
RestHandler handler = getRestHandler(servletRequest);
+ // Get Content-Type and format
+ String format = null;
+ String contentType = null;
+
+ try {
+ format = servletRequest.getParameter(FORMAT_PARAM);
+ } catch (Throwable t) {
+ // this happens while testing
+ if (LOG.isLoggable(Level.FINE)) {
+ LOG.fine("Unexpected error : format param is null " + t.toString());
+ }
+ }
+ try {
+ // TODO: First implementation causes bug when Content-Type is
application/atom+xml. Fix is applied.
+ contentType =
ContentTypes.extractMimePart(servletRequest.getContentType());
+ } catch (Throwable t) {
+ //this happens while testing
+ if (LOG.isLoggable(Level.FINE)) {
+ LOG.fine("Unexpected error : content type is null " + t.toString());
+ }
+ }
+
+ // Get BeanConverter for Request payload.
+ BeanConverter requestConverter = getConverterForRequest(contentType,
format);
+
+ // Get BeanConverter for Response body.
+ BeanConverter responseConverter = getConverterForFormat(format);
+
Reader bodyReader = null;
if (!servletRequest.getMethod().equals("GET") &&
!servletRequest.getMethod().equals("HEAD")) {
bodyReader = servletRequest.getReader();
@@ -163,11 +190,10 @@ public class DataServiceServlet extends
// Execute the request
@SuppressWarnings("unchecked")
Map<String, String[]> parameterMap = servletRequest.getParameterMap();
- Future<?> future = handler.execute(parameterMap, bodyReader, token,
converter);
-
+ Future<?> future = handler.execute(parameterMap, bodyReader, token,
requestConverter);
ResponseItem responseItem = getResponseItem(future);
- servletResponse.setContentType(converter.getContentType());
+ servletResponse.setContentType(responseConverter.getContentType());
if (responseItem.getErrorCode() >= 200 && responseItem.getErrorCode() <
400) {
PrintWriter writer = servletResponse.getWriter();
Object response = responseItem.getResponse();
@@ -178,11 +204,11 @@ public class DataServiceServlet extends
// JSONP style callbacks
String callback = (HttpUtil.isJSONP(servletRequest) &&
-
ContentTypes.OUTPUT_JSON_CONTENT_TYPE.equals(converter.getContentType())) ?
+
ContentTypes.OUTPUT_JSON_CONTENT_TYPE.equals(responseConverter.getContentType()))
?
servletRequest.getParameter("callback") : null;
if (callback != null) writer.write(callback + '(');
- writer.write(converter.convertToString(response));
+ writer.write(responseConverter.convertToString(response));
if (callback != null) writer.write(");\n");
} else {
sendError(servletResponse, responseItem);
@@ -203,47 +229,34 @@ public class DataServiceServlet extends
return dispatcher.getRestHandler(path, method.toUpperCase());
}
- public BeanConverter getConverterForRequest(HttpServletRequest
servletRequest) {
- String formatString = null;
- BeanConverter converter = jsonConverter; // default is jsonConverter
- String contentType = null;
-
- try {
- formatString = servletRequest.getParameter(FORMAT_PARAM);
- } catch (Throwable t) {
- // this happens while testing
- if (LOG.isLoggable(Level.FINE)) {
- LOG.fine("Unexpected error : format param is null " + t.toString());
- }
- }
- try {
- // TODO: First implementation causes bug when Content-Type is
application/atom+xml. Fix is applied.
- //contentType = servletRequest.getContentType();
- contentType =
ContentTypes.extractMimePart(servletRequest.getContentType());
- } catch (Throwable t) {
- //this happens while testing
- if (LOG.isLoggable(Level.FINE)) {
- LOG.fine("Unexpected error : content type is null " + t.toString());
- }
+ /*
+ * Return the right BeanConverter to convert the request payload.
+ */
+ BeanConverter getConverterForRequest(@Nullable String contentType, String
format) {
+ if (StringUtils.isNotBlank(contentType)) {
+ return getConverterForContentType(contentType);
+ } else {
+ return getConverterForFormat(format);
}
+ }
- if (contentType != null) {
- if (ContentTypes.ALLOWED_JSON_CONTENT_TYPES.contains(contentType)) {
- converter = jsonConverter;
- } else if
(ContentTypes.ALLOWED_ATOM_CONTENT_TYPES.contains(contentType)) {
- converter = atomConverter;
- } else if (ContentTypes.ALLOWED_XML_CONTENT_TYPES.contains(contentType))
{
- converter = xmlConverter;
- }
- } else if (formatString != null) {
- if (formatString.equals(ATOM_FORMAT)) {
- converter = atomConverter;
- } else if (formatString.equals(XML_FORMAT)) {
- converter = xmlConverter;
- } else {
- converter = jsonConverter;
- }
- }
- return converter;
+ /**
+ * Return BeanConverter based on content type.
+ * @param contentType the content type for the converter.
+ * @return BeanConverter based on the contentType input param. Will default
to JSON
+ */
+ protected BeanConverter getConverterForContentType(String contentType) {
+ return ContentTypes.ALLOWED_ATOM_CONTENT_TYPES.contains(contentType) ?
atomConverter :
+ ContentTypes.ALLOWED_XML_CONTENT_TYPES.contains(contentType) ?
xmlConverter : jsonConverter;
+ }
+
+ /**
+ * Return BeanConverter based on format request parameter.
+ * @param format the format for the converter.
+ * @return BeanConverter based on the format input param. Will default to
JSON
+ */
+ protected BeanConverter getConverterForFormat(String format) {
+ return ATOM_FORMAT.equals(format) ? atomConverter :
XML_FORMAT.equals(format) ? xmlConverter :
+ jsonConverter;
}
}
Modified:
shindig/trunk/java/common/src/test/java/org/apache/shindig/protocol/DataServiceServletTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/common/src/test/java/org/apache/shindig/protocol/DataServiceServletTest.java?rev=1084860&r1=1084859&r2=1084860&view=diff
==============================================================================
---
shindig/trunk/java/common/src/test/java/org/apache/shindig/protocol/DataServiceServletTest.java
(original)
+++
shindig/trunk/java/common/src/test/java/org/apache/shindig/protocol/DataServiceServletTest.java
Thu Mar 24 07:05:13 2011
@@ -162,16 +162,16 @@ public class DataServiceServletTest exte
}
@Test
- public void testGetConverterForRequest() throws Exception {
- assertConverter(atomConverter, "atom");
- assertConverter(xmlConverter, "xml");
- assertConverter(jsonConverter, "");
- assertConverter(jsonConverter, null);
- assertConverter(jsonConverter, "ahhhh!");
+ public void testGetConverterForFormat() throws Exception {
+ assertConverterForFormat(atomConverter, "atom");
+ assertConverterForFormat(xmlConverter, "xml");
+ assertConverterForFormat(jsonConverter, "");
+ assertConverterForFormat(jsonConverter, null);
+ assertConverterForFormat(jsonConverter, "ahhhh!");
}
@Test
- public void testGetConverterForRequestContentType() throws Exception {
+ public void testGetConverterForContentType() throws Exception {
assertConverterForContentType(atomConverter,
ContentTypes.OUTPUT_ATOM_CONTENT_TYPE);
assertConverterForContentType(xmlConverter,
ContentTypes.OUTPUT_XML_CONTENT_TYPE);
assertConverterForContentType(xmlConverter, "text/xml");
@@ -182,21 +182,11 @@ public class DataServiceServletTest exte
assertConverterForContentType(jsonConverter, "abcd!");
}
- private void assertConverter(BeanConverter converter, String format) {
- EasyMock.expect(req.getParameter(ApiServlet.FORMAT_PARAM))
- .andReturn(format);
- EasyMock.replay(req);
- assertEquals(converter, servlet.getConverterForRequest(req));
- EasyMock.verify(req);
- EasyMock.reset(req);
+ private void assertConverterForFormat(BeanConverter converter, String
format) {
+ assertEquals(converter, servlet.getConverterForFormat(format));
}
- private void assertConverterForContentType(BeanConverter converter,
- String contentType) {
- EasyMock.expect(req.getContentType()).andReturn(contentType);
- EasyMock.replay(req);
- assertEquals(converter, servlet.getConverterForRequest(req));
- EasyMock.verify(req);
- EasyMock.reset(req);
+ private void assertConverterForContentType(BeanConverter converter, String
contentType) {
+ assertEquals(converter, servlet.getConverterForContentType(contentType));
}
}
Modified:
shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulAtomActivityEntryTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulAtomActivityEntryTest.java?rev=1084860&r1=1084859&r2=1084860&view=diff
==============================================================================
---
shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulAtomActivityEntryTest.java
(original)
+++
shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulAtomActivityEntryTest.java
Thu Mar 24 07:05:13 2011
@@ -12,14 +12,14 @@ public class RestfulAtomActivityEntryTes
@Test
public void testGetActivityEntryAtomById() throws Exception {
- String resp = getResponse("/activitystreams/john.doe/@self/1/object1",
"GET", null, ContentTypes.OUTPUT_ATOM_CONTENT_TYPE);
+ String resp = getResponse("/activitystreams/john.doe/@self/1/object1",
"GET", "atom", ContentTypes.OUTPUT_ATOM_CONTENT_TYPE);
String expected = TestUtils.loadTestFixture(FIXTURE_LOC +
"ActivityEntryAtomId.xml");
assertTrue(TestUtils.xmlsEqual(expected, resp));
}
@Test
public void testGetActivityEntryAtomByIds() throws Exception {
- String resp =
getResponse("/activitystreams/john.doe/@self/1/object1,object2", "GET", null,
ContentTypes.OUTPUT_ATOM_CONTENT_TYPE);
+ String resp =
getResponse("/activitystreams/john.doe/@self/1/object1,object2", "GET", "atom",
ContentTypes.OUTPUT_ATOM_CONTENT_TYPE);
String expected = TestUtils.loadTestFixture(FIXTURE_LOC +
"ActivityEntryAtomIds.xml");
assertTrue(TestUtils.xmlsEqual(expected, resp));
}
Modified:
shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulXmlActivityEntryTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulXmlActivityEntryTest.java?rev=1084860&r1=1084859&r2=1084860&view=diff
==============================================================================
---
shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulXmlActivityEntryTest.java
(original)
+++
shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulXmlActivityEntryTest.java
Thu Mar 24 07:05:13 2011
@@ -12,14 +12,14 @@ public class RestfulXmlActivityEntryTest
@Test
public void testGetActivityEntryXmlById() throws Exception {
- String resp = getResponse("/activitystreams/john.doe/@self/1/object1",
"GET", null, ContentTypes.OUTPUT_XML_CONTENT_TYPE);
+ String resp = getResponse("/activitystreams/john.doe/@self/1/object1",
"GET", "xml", ContentTypes.OUTPUT_XML_CONTENT_TYPE);
String expected = TestUtils.loadTestFixture(FIXTURE_LOC +
"ActivityEntryXmlId.xml");
assertTrue(TestUtils.xmlsEqual(expected, resp));
}
@Test
public void testGetActivityEntryXmlByIds() throws Exception {
- String resp =
getResponse("/activitystreams/john.doe/@self/1/object1,object2", "GET", null,
ContentTypes.OUTPUT_XML_CONTENT_TYPE);
+ String resp =
getResponse("/activitystreams/john.doe/@self/1/object1,object2", "GET", "xml",
ContentTypes.OUTPUT_XML_CONTENT_TYPE);
String expected = TestUtils.loadTestFixture(FIXTURE_LOC +
"ActivityEntryXmlIds.xml");
assertTrue(TestUtils.xmlsEqual(expected, resp));
}
