On Wed, Feb 4, 2009 at 9:06 AM, Louis Ryan <[email protected]> wrote:
> Ben
>
> Glad you like it overall. This piece is definitely something of a work in
> progress but the intent is to support the things you want to do Some notes
> inline.
>
> On Wed, Feb 4, 2009 at 1:53 AM, Ben Smith <[email protected]> wrote:
>
>> Hey all (and particularly @lryan),
>>
>> I spent a lot of yesturday migrating our code to comply with the new
>> Shindig annotations based handler and handler registry. I really like this
>> way of organising things, but I did have quite a few problems while
>> itegrating which I hope you can help us with.
>>
>> *1. Registering a new Handler*
>>
>> Up until yesturday we've been including the SocialApiGuiceModule in our
>> web.xml and test-bootstraps. This is generally a good idea as we don't have
>> to manually keep the bindings up-to-date.
>>
>> Now that SocialApiGuiceModule registers a list of handlers:
>> bind(List.class).annotatedWith(Names.named("org.apache.shindig.handlers"))
>> .toInstance(Lists.immutableList(ActivityHandler.class,
>> AppDataHandler.class,
>> PersonHandler.class, SampleContainerHandler.class));
>>
>> And we extend the PersonHandler with, say, SuperAwesomeBBCPersonHandler, we
>> now have to copy and paste all of SocialApiGuiceModule, so we can change
>> that one registration. Even if we bind PersonHandler.class to
>> SuperAwesomeBBCPersonHandler.class, it's PersonHandler that gets registered
>> as a handler (unless I did something wrong when I tried this.. which is
>> likely).
>>
>> There's probably some way of achieving this that I missed yesturday, but as
>> it stands this will increase our maintenance / tracking overhead.
>
>
> Yup, definitely an issue and something Ill fix. If we were to move to Guice
> 2 this problem would disappear as you could simply override the binding.
> Unfortunately its not released yet so Ill need to resort to other means to
> make this easier
Could you change this from a List to a Set? On that ever-distant day
when Guice 2.0 is released, we could switch this to a Multibinding,
which would let you contribute new handlers to the base instead of
just overriding.
>
>>
>>
>> *2. Modularising the Actions*
>>
>> The major advantage to this system can be seen in the /@supportedFields as
>> you can modularise the actions and not have to stuff them all in to one long
>> function.
>>
>> The problem I am having is creating seperate functions with paths that only
>> change at the end.
>>
>> Say I've got an operation that creates a Person:
>> @Operation(httpMethods = "POST")
>> public Future<?> create(RequestItem request) throws SocialSpiException {
>>
>> And an operation that creates a person's Igloos:
>> @Operation(httpMethods = "POST", path = "/{userId}+/@self/igloos")
>> public Future<?> createIgloo(RequestItem request) throws SocialSpiException
>> {
>>
>> The paths aren't different enough for the DefaultHandlerRegistry and the
>> two functions conflict. Am I doing something wrong or does the Hash for Rest
>> operations in the DefaultHandlerRegistry need to create cleverer keys for
>> this to work?
>
>
> Yes, this is a known issue with the patch matching. It basically searches
> using a longest-prefix match and only matches constant parts of the path.
> Let me take a look at doing something a bit more refined.
>
>
>>
>>
>> Hope you can help me with this. Overall, nice work, it could be made just a
>> bit easier for extending parties.
>>
>> Cheers,
>> Ben Smith
>> BBC
>>
>
