You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by hs...@apache.org on 2011/03/24 08:05:14 UTC
svn commit: r1084860 - in /shindig/trunk/java:
common/src/main/java/org/apache/shindig/protocol/
common/src/test/java/org/apache/shindig/protocol/
social-api/src/test/java/org/apache/shindig/social/dataservice/integration/
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));
}