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 <aw...@google.com> on 2008/08/27 17:30:53 UTC
Service API code cleanup
Since we're trying to clean up the codebase to freeze it: I'd like to
clean up a few items in the Service APIs.
(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.
(2) ResponseError should go somewhere other than the top package - I
think it belongs in opensocial.model.
(3) Mutators should be able to return Void instead of an untyped Object, so:
Future<ResponseItem> createActivity(...);
becomes;
Future<Void> createActivity(...) throws SocialApiException;
-- Adam Winer
Re: Service API code cleanup
Posted by Ian Boston <ie...@tfd.co.uk>.
I think there are some minor refactors that need to be done to the
area still,
eg the SocialApiException was switched to extend RuntimeException,
but this doesn't make it clear to the implementors of the SPI that
they can use the exception as the error pathway, and slightly more
critical, the implementors of the SeviceAPI are not aware that they
must catch and process the SocialApiException.
but.... this needs a small bit of discussion.
Ian
On 2 Sep 2008, at 17:30, Cassie wrote:
> (btw Louis - this was already checked in by Ian as a patch from Adam)
>
> On Tue, Sep 2, 2008 at 9:24 AM, Louis Ryan <lr...@google.com> wrote:
>
>> +1
>>
>> On Wed, Aug 27, 2008 at 8:52 AM, Adam Winer <aw...@google.com>
>> wrote:
>>
>>> Either at the handler layer or the servlet layer (I think the latter
>>> is easier), but yes.
>>>
>>> On Wed, Aug 27, 2008 at 8:41 AM, Ian Boston <ie...@tfd.co.uk> wrote:
>>>> Is the intention to catch the SocialApiException somewhere near the
>>> protocol
>>>> handler and convert to a suitable error response ?
>>>>
>>>> If so, IMHO +1
>>>>
>>>> Ian
>>>>
>>>> On 27 Aug 2008, at 16:30, Adam Winer wrote:
>>>>
>>>>> Since we're trying to clean up the codebase to freeze it: I'd
>>>>> like to
>>>>> clean up a few items in the Service APIs.
>>>>>
>>>>> (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.
>>>>>
>>>>> (2) ResponseError should go somewhere other than the top
>>>>> package - I
>>>>> think it belongs in opensocial.model.
>>>>>
>>>>> (3) Mutators should be able to return Void instead of an untyped
>> Object,
>>>>> so:
>>>>>
>>>>> Future<ResponseItem> createActivity(...);
>>>>>
>>>>> becomes;
>>>>>
>>>>> Future<Void> createActivity(...) throws SocialApiException;
>>>>>
>>>>> -- Adam Winer
>>>>
>>>>
>>>
>>
Re: Service API code cleanup
Posted by Cassie <do...@apache.org>.
(btw Louis - this was already checked in by Ian as a patch from Adam)
On Tue, Sep 2, 2008 at 9:24 AM, Louis Ryan <lr...@google.com> wrote:
> +1
>
> On Wed, Aug 27, 2008 at 8:52 AM, Adam Winer <aw...@google.com> wrote:
>
> > Either at the handler layer or the servlet layer (I think the latter
> > is easier), but yes.
> >
> > On Wed, Aug 27, 2008 at 8:41 AM, Ian Boston <ie...@tfd.co.uk> wrote:
> > > Is the intention to catch the SocialApiException somewhere near the
> > protocol
> > > handler and convert to a suitable error response ?
> > >
> > > If so, IMHO +1
> > >
> > > Ian
> > >
> > > On 27 Aug 2008, at 16:30, Adam Winer wrote:
> > >
> > >> Since we're trying to clean up the codebase to freeze it: I'd like to
> > >> clean up a few items in the Service APIs.
> > >>
> > >> (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.
> > >>
> > >> (2) ResponseError should go somewhere other than the top package - I
> > >> think it belongs in opensocial.model.
> > >>
> > >> (3) Mutators should be able to return Void instead of an untyped
> Object,
> > >> so:
> > >>
> > >> Future<ResponseItem> createActivity(...);
> > >>
> > >> becomes;
> > >>
> > >> Future<Void> createActivity(...) throws SocialApiException;
> > >>
> > >> -- Adam Winer
> > >
> > >
> >
>
Re: Service API code cleanup
Posted by Louis Ryan <lr...@google.com>.
+1
On Wed, Aug 27, 2008 at 8:52 AM, Adam Winer <aw...@google.com> wrote:
> Either at the handler layer or the servlet layer (I think the latter
> is easier), but yes.
>
> On Wed, Aug 27, 2008 at 8:41 AM, Ian Boston <ie...@tfd.co.uk> wrote:
> > Is the intention to catch the SocialApiException somewhere near the
> protocol
> > handler and convert to a suitable error response ?
> >
> > If so, IMHO +1
> >
> > Ian
> >
> > On 27 Aug 2008, at 16:30, Adam Winer wrote:
> >
> >> Since we're trying to clean up the codebase to freeze it: I'd like to
> >> clean up a few items in the Service APIs.
> >>
> >> (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.
> >>
> >> (2) ResponseError should go somewhere other than the top package - I
> >> think it belongs in opensocial.model.
> >>
> >> (3) Mutators should be able to return Void instead of an untyped Object,
> >> so:
> >>
> >> Future<ResponseItem> createActivity(...);
> >>
> >> becomes;
> >>
> >> Future<Void> createActivity(...) throws SocialApiException;
> >>
> >> -- Adam Winer
> >
> >
>
Re: Service API code cleanup
Posted by Adam Winer <aw...@google.com>.
Either at the handler layer or the servlet layer (I think the latter
is easier), but yes.
On Wed, Aug 27, 2008 at 8:41 AM, Ian Boston <ie...@tfd.co.uk> wrote:
> Is the intention to catch the SocialApiException somewhere near the protocol
> handler and convert to a suitable error response ?
>
> If so, IMHO +1
>
> Ian
>
> On 27 Aug 2008, at 16:30, Adam Winer wrote:
>
>> Since we're trying to clean up the codebase to freeze it: I'd like to
>> clean up a few items in the Service APIs.
>>
>> (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.
>>
>> (2) ResponseError should go somewhere other than the top package - I
>> think it belongs in opensocial.model.
>>
>> (3) Mutators should be able to return Void instead of an untyped Object,
>> so:
>>
>> Future<ResponseItem> createActivity(...);
>>
>> becomes;
>>
>> Future<Void> createActivity(...) throws SocialApiException;
>>
>> -- Adam Winer
>
>
Re: Service API code cleanup
Posted by Ian Boston <ie...@tfd.co.uk>.
Is the intention to catch the SocialApiException somewhere near the
protocol handler and convert to a suitable error response ?
If so, IMHO +1
Ian
On 27 Aug 2008, at 16:30, Adam Winer wrote:
> Since we're trying to clean up the codebase to freeze it: I'd like to
> clean up a few items in the Service APIs.
>
> (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.
>
> (2) ResponseError should go somewhere other than the top package - I
> think it belongs in opensocial.model.
>
> (3) Mutators should be able to return Void instead of an untyped
> Object, so:
>
> Future<ResponseItem> createActivity(...);
>
> becomes;
>
> Future<Void> createActivity(...) throws SocialApiException;
>
> -- Adam Winer