You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by "Adam Winer (JIRA)" <ji...@apache.org> on 2008/08/28 02:09:44 UTC
[jira] Updated: (SHINDIG-549) Use exceptions instead of
ResponseItem to signal service errors
[ https://issues.apache.org/jira/browse/SHINDIG-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adam Winer updated SHINDIG-549:
-------------------------------
Attachment: shindig-549.patch
Here's a first pass at implementing this, off of svn revision 689672. All Shindig tests pass, though I'd like a bit more manual testing.
> Use exceptions instead of ResponseItem to signal service errors
> ---------------------------------------------------------------
>
> Key: SHINDIG-549
> URL: https://issues.apache.org/jira/browse/SHINDIG-549
> Project: Shindig
> Issue Type: Improvement
> Components: RESTful API (Java)
> Reporter: Adam Winer
> Attachments: shindig-549.patch
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> As discussed on shindig-dev:
> (1) Get error code and message out of
> RestfulItem/RestfulCollection/DataCollection and into an exception,
> so:
> PersonService {
> Future<RestfulCollection<Person>> getPeople(...);
> Future<RestfulItem<Person>> getPerson(...);
> }
> becomes:
> PersonService {
> // Note: the Future may also throw a SocialApiException (wrapped in
> an EvaluationException)
> Future<RestfulCollection<Person>> getPeople(...) throws SocialApiException
> Future<Person> getPerson(...) throws SocialApiException
> }
> RestfulItem goes away entirely, and RestfulCollection doesn't extend
> ResponseItem. ResponseItem can entirely disappear from the SPI, and
> can move out of org.apache.shindig.social and into
> o.a.s.s.opensocial.service alongside of RequestItem.
> One advantage of this change is that it makes it easy to write generic
> preconditions across datatypes, and gets rid of early returns from
> service implementations, which are a bit of a code smell.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Updated: (SHINDIG-549) Use exceptions instead of ResponseItem to signal service errors
Posted by Adam Winer <aw...@google.com>.
I made one other change in the second patch, which was for
SocialSpiException to inherit from RuntimeException. When I tried
implementing against the API, that proved to be much easier than
exposing SocialSpiException everywhere, especially where my codebase
works with APIs like
the Function interface from Google Collections.
-- Adam
On Thu, Aug 28, 2008 at 2:20 PM, Ian Boston <ie...@tfd.co.uk> wrote:
> Ok, no worries svn/diff patches dont work well with moves,
> The new location is good.
> so the patch instructions are
> mv then patch
>
> One patch is just fine, I will try again.
> Thanks
> Ian
>
> On 28 Aug 2008, at 17:23, Adam Winer wrote:
>
>>>
>>> This looks like a move and then a diff on the result of the move ?
>>
>> Yes, that's what the patch is trying to do.
>>
>>> Should this be preceded by a mv
>>> java/social-api/src/main/java/org/apache/shindig/social/ResponseItem.java
>>>
>>> java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ResponseItem.java
>>> ?
>>
>> Or followed by it, since ResponseItem doesn't really belong in
>> opensocial/service until this patch is in. Would you rather we keep
>> ResponseItem where it is for this patch, and move it in a second
>> patch?
>>
>> -- Adam
>
>
Re: [jira] Updated: (SHINDIG-549) Use exceptions instead of ResponseItem to signal service errors
Posted by Ian Boston <ie...@tfd.co.uk>.
Ok, no worries svn/diff patches dont work well with moves,
The new location is good.
so the patch instructions are
mv then patch
One patch is just fine, I will try again.
Thanks
Ian
On 28 Aug 2008, at 17:23, Adam Winer wrote:
>>
>> This looks like a move and then a diff on the result of the move ?
>
> Yes, that's what the patch is trying to do.
>
>> Should this be preceded by a mv
>> java/social-api/src/main/java/org/apache/shindig/social/
>> ResponseItem.java
>> java/social-api/src/main/java/org/apache/shindig/social/opensocial/
>> service/ResponseItem.java
>> ?
>
> Or followed by it, since ResponseItem doesn't really belong in
> opensocial/service until this patch is in. Would you rather we keep
> ResponseItem where it is for this patch, and move it in a second
> patch?
>
> -- Adam
Re: [jira] Updated: (SHINDIG-549) Use exceptions instead of ResponseItem to signal service errors
Posted by Adam Winer <aw...@google.com>.
On Thu, Aug 28, 2008 at 1:41 AM, Ian Boston <ie...@tfd.co.uk> wrote:
> The concept looks good but there is something funny going on with
> ResponseItem in the patch
>
>
> Index:
> java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ResponseItem.java
> ===================================================================
> ---
> java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ResponseItem.java
> (revision 689519)
> +++
> java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ResponseItem.java
> (working copy)
> @@ -15,28 +15,31 @@
> * KIND, either express or implied. See the License for the
> * specific language governing permissions and limitations under the
> License.
> */
> -package org.apache.shindig.social;
> +package org.apache.shindig.social.opensocial.service;
>
> +import com.google.common.base.Objects;
> +
> +import org.apache.shindig.social.ResponseError;
> +
>
>
> This looks like a move and then a diff on the result of the move ?
Yes, that's what the patch is trying to do.
> Should this be preceded by a mv
> java/social-api/src/main/java/org/apache/shindig/social/ResponseItem.java
> java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ResponseItem.java
> ?
Or followed by it, since ResponseItem doesn't really belong in
opensocial/service until this patch is in. Would you rather we keep
ResponseItem where it is for this patch, and move it in a second
patch?
-- Adam
> On 28 Aug 2008, at 01:09, Adam Winer (JIRA) wrote:
>
>>
>> [
>> https://issues.apache.org/jira/browse/SHINDIG-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
>> ]
>>
>> Adam Winer updated SHINDIG-549:
>> -------------------------------
>>
>> Attachment: shindig-549.patch
>>
>> Here's a first pass at implementing this, off of svn revision 689672. All
>> Shindig tests pass, though I'd like a bit more manual testing.
>>
>>> Use exceptions instead of ResponseItem to signal service errors
>>> ---------------------------------------------------------------
>>>
>>> Key: SHINDIG-549
>>> URL: https://issues.apache.org/jira/browse/SHINDIG-549
>>> Project: Shindig
>>> Issue Type: Improvement
>>> Components: RESTful API (Java)
>>> Reporter: Adam Winer
>>> Attachments: shindig-549.patch
>>>
>>> Original Estimate: 24h
>>> Remaining Estimate: 24h
>>>
>>> As discussed on shindig-dev:
>>> (1) Get error code and message out of
>>> RestfulItem/RestfulCollection/DataCollection and into an exception,
>>> so:
>>> PersonService {
>>> Future<RestfulCollection<Person>> getPeople(...);
>>> Future<RestfulItem<Person>> getPerson(...);
>>> }
>>> becomes:
>>> PersonService {
>>> // Note: the Future may also throw a SocialApiException (wrapped in
>>> an EvaluationException)
>>> Future<RestfulCollection<Person>> getPeople(...) throws
>>> SocialApiException
>>> Future<Person> getPerson(...) throws SocialApiException
>>> }
>>> RestfulItem goes away entirely, and RestfulCollection doesn't extend
>>> ResponseItem. ResponseItem can entirely disappear from the SPI, and
>>> can move out of org.apache.shindig.social and into
>>> o.a.s.s.opensocial.service alongside of RequestItem.
>>> One advantage of this change is that it makes it easy to write generic
>>> preconditions across datatypes, and gets rid of early returns from
>>> service implementations, which are a bit of a code smell.
>>
>> --
>> This message is automatically generated by JIRA.
>> -
>> You can reply to this email to add a comment to the issue online.
>>
>
>
Re: [jira] Updated: (SHINDIG-549) Use exceptions instead of ResponseItem to signal service errors
Posted by Ian Boston <ie...@tfd.co.uk>.
The concept looks good but there is something funny going on with
ResponseItem in the patch
Index: java/social-api/src/main/java/org/apache/shindig/social/
opensocial/service/ResponseItem.java
===================================================================
--- java/social-api/src/main/java/org/apache/shindig/social/
opensocial/service/ResponseItem.java (revision 689519)
+++ java/social-api/src/main/java/org/apache/shindig/social/
opensocial/service/ResponseItem.java (working copy)
@@ -15,28 +15,31 @@
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under
the License.
*/
-package org.apache.shindig.social;
+package org.apache.shindig.social.opensocial.service;
+import com.google.common.base.Objects;
+
+import org.apache.shindig.social.ResponseError;
+
This looks like a move and then a diff on the result of the move ?
Should this be preceded by a mv java/social-api/src/main/java/org/
apache/shindig/social/ResponseItem.java java/social-api/src/main/java/
org/apache/shindig/social/opensocial/service/ResponseItem.java ?
Ian
On 28 Aug 2008, at 01:09, Adam Winer (JIRA) wrote:
>
> [ https://issues.apache.org/jira/browse/SHINDIG-549?
> page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>
> Adam Winer updated SHINDIG-549:
> -------------------------------
>
> Attachment: shindig-549.patch
>
> Here's a first pass at implementing this, off of svn revision
> 689672. All Shindig tests pass, though I'd like a bit more manual
> testing.
>
>> Use exceptions instead of ResponseItem to signal service errors
>> ---------------------------------------------------------------
>>
>> Key: SHINDIG-549
>> URL: https://issues.apache.org/jira/browse/
>> SHINDIG-549
>> Project: Shindig
>> Issue Type: Improvement
>> Components: RESTful API (Java)
>> Reporter: Adam Winer
>> Attachments: shindig-549.patch
>>
>> Original Estimate: 24h
>> Remaining Estimate: 24h
>>
>> As discussed on shindig-dev:
>> (1) Get error code and message out of
>> RestfulItem/RestfulCollection/DataCollection and into an exception,
>> so:
>> PersonService {
>> Future<RestfulCollection<Person>> getPeople(...);
>> Future<RestfulItem<Person>> getPerson(...);
>> }
>> becomes:
>> PersonService {
>> // Note: the Future may also throw a SocialApiException (wrapped in
>> an EvaluationException)
>> Future<RestfulCollection<Person>> getPeople(...) throws
>> SocialApiException
>> Future<Person> getPerson(...) throws SocialApiException
>> }
>> RestfulItem goes away entirely, and RestfulCollection doesn't extend
>> ResponseItem. ResponseItem can entirely disappear from the SPI, and
>> can move out of org.apache.shindig.social and into
>> o.a.s.s.opensocial.service alongside of RequestItem.
>> One advantage of this change is that it makes it easy to write
>> generic
>> preconditions across datatypes, and gets rid of early returns from
>> service implementations, which are a bit of a code smell.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>