Refactoring AbstractProvider to make it open for extension

[Sergio Bossa <se...@gmail.com>]

Hi all,

I'm working on a large project which will extensively use Apache Abdera.
We will have to support the OpenSearch (OS from now on) specification,
so while trying to enhance Abdera support for OS, I saw that the
AbstractProvider class, which is the core of the whole server
infrastructure, is poorly implemented as a bunch of "if" statements
depending on the TargetType for actual request processing.
This makes very hard to add request processing features to the
AbstractProvider, because it forces you to modify the class itself by
adding more and more "if" statements.
That said, I've refactored the AbstractProvider class, removed all
"if" statements and added the concept of "RequestProcessor", a
strategy-like interface to implement for request processing logic: by
doing so, adding request processing logic is just a matter of
implementing an interface and registering it under a given TargetType.
I've tested my work against all Abdera unit tests and everything works fine.

What do you think about that?
If you think it could be useful, I'd be more than happy to contribute it.

Cheers,

Sergio B.

-- 
Sergio Bossa
Software Passionate, Java Technologies Specialist and Open Source Enthusiast.
Blog : http://sbtourist.blogspot.com
Sourcesense - making sense of Open Source : http://www.sourcesense.com
Pro-netics s.p.a. : http://www.pronetics.it

Re: Refactoring AbstractProvider to make it open for extension

[Sergio Bossa <se...@gmail.com>]

Hi guys,

here's the Jira issue with the diff file:
https://issues.apache.org/jira/browse/ABDERA-185.
Just to make it clear: I don't think Abdera should be a general
purpose web content provider, and my patch doesn't go in this
direction.
My patch just enables you to easily provide additional request
processing logic depending on the target type: that's all, and that's
very useful for edge cases like OpenSearch (as said by Dan) which are
IMHO strictly related to Atom.
Other than that, having a decoupled strategy/plugin-like code is IMHO
better than having a bunch of hard-coded "if" statements, improving
code modularity and manageability.

Cheers,

Sergio B.

-- 
Sergio Bossa
Software Passionate, Java Technologies Specialist and Open Source Enthusiast.
Blog : http://sbtourist.blogspot.com
Sourcesense - making sense of Open Source : http://www.sourcesense.com
Pro-netics s.p.a. : http://www.pronetics.it

Re: Refactoring AbstractProvider to make it open for extension

[Dan Diephouse <da...@mulesource.com>]

On Fri, Jul 4, 2008 at 5:03 PM, David Calavera <da...@gmail.com>
wrote:

> well, I don't agree that provider should server non atom documents


This is actually one of the issues I see with Abdera long term. Abdera
should not be a new HTTP server/client framework. We should provide AtomPub
services over an existing one... Nothing has really emerged at a higher
level (other than servlets) as a dominant player though. Restlet is a
candidate but it has its weakness.

In the mean time I think we do have to serve non atom documents as there are
just too many edge cases like OpenSearch.

Dan

-- 
Dan Diephouse
http://mulesource.com | http://netzooid.com/blog

Re: Refactoring AbstractProvider to make it open for extension

[David Calavera <da...@gmail.com>]

well, I don't agree that provider should server non atom documents but it
actually sound interesting and I'd like to see your work.

just add the new classes to subversion and create the patch later:

svn add ...
svn diff ... > ...


On Fri, Jul 4, 2008 at 10:52 PM, Sergio Bossa <se...@gmail.com>
wrote:

> On Fri, Jul 4, 2008 at 8:28 PM, Dan Diephouse
> <da...@mulesource.com> wrote:
>
> > Could you file a JIRA with a patch? It sounds interesting and I do see
> the
> > issues that you mention.
>
> Thanks for your interest.
> Given that I added several files to the codebase, how do you want me
> to file this kind of patch?
>
> --
> Sergio Bossa
> Software Passionate, Java Technologies Specialist and Open Source
> Enthusiast.
> Blog : http://sbtourist.blogspot.com
> Sourcesense - making sense of Open Source : http://www.sourcesense.com
> Pro-netics s.p.a. : http://www.pronetics.it
>



-- 
David Calavera
http://www.thinkincode.net

Re: Refactoring AbstractProvider to make it open for extension

[Sergio Bossa <se...@gmail.com>]

On Fri, Jul 4, 2008 at 8:28 PM, Dan Diephouse
<da...@mulesource.com> wrote:

> Could you file a JIRA with a patch? It sounds interesting and I do see the
> issues that you mention.

Thanks for your interest.
Given that I added several files to the codebase, how do you want me
to file this kind of patch?

-- 
Sergio Bossa
Software Passionate, Java Technologies Specialist and Open Source Enthusiast.
Blog : http://sbtourist.blogspot.com
Sourcesense - making sense of Open Source : http://www.sourcesense.com
Pro-netics s.p.a. : http://www.pronetics.it

Re: Refactoring AbstractProvider to make it open for extension

[Dan Diephouse <da...@mulesource.com>]

Could you file a JIRA with a patch? It sounds interesting and I do see the
issues that you mention.

On Fri, Jul 4, 2008 at 1:43 PM, Sergio Bossa <se...@gmail.com> wrote:

> Hi all,
>
> I'm working on a large project which will extensively use Apache Abdera.
> We will have to support the OpenSearch (OS from now on) specification,
> so while trying to enhance Abdera support for OS, I saw that the
> AbstractProvider class, which is the core of the whole server
> infrastructure, is poorly implemented as a bunch of "if" statements
> depending on the TargetType for actual request processing.
> This makes very hard to add request processing features to the
> AbstractProvider, because it forces you to modify the class itself by
> adding more and more "if" statements.
> That said, I've refactored the AbstractProvider class, removed all
> "if" statements and added the concept of "RequestProcessor", a
> strategy-like interface to implement for request processing logic: by
> doing so, adding request processing logic is just a matter of
> implementing an interface and registering it under a given TargetType.
> I've tested my work against all Abdera unit tests and everything works
> fine.
>
> What do you think about that?
> If you think it could be useful, I'd be more than happy to contribute it.
>
> Cheers,
>
> Sergio B.
>
> --
> Sergio Bossa
> Software Passionate, Java Technologies Specialist and Open Source
> Enthusiast.
> Blog : http://sbtourist.blogspot.com
> Sourcesense - making sense of Open Source : http://www.sourcesense.com
> Pro-netics s.p.a. : http://www.pronetics.it
>



-- 
Dan Diephouse
http://mulesource.com | http://netzooid.com/blog

Re: Refactoring AbstractProvider to make it open for extension

[David Primmer <da...@gmail.com>]

On Fri, Jul 4, 2008 at 1:51 PM, Sergio Bossa <se...@gmail.com> wrote:
> On Fri, Jul 4, 2008 at 8:53 PM, David Primmer <da...@gmail.com> wrote:
>
>> I'm not sure what things you want to do with the request processing
>> that force you to replace the 'if' statements that you mention.
>> [CUT]
>
> AbstractProvider uses "if" statements in order to decide what to do
> depending on the TargetType: this prevents you to extend provider
> features for supporting new target types, i.e. Open Search documents.
> I know you can support OpenSearch (or whatever else) with filters,
> resolvers, and alike, but I think it would be wrong: if it's the
> provider job to decide what to do depending on the TargetType, you
> shouldn't give this same responsibility to other classes.

Ah, I see what you mean. I guess I just never thought of this as a new
TargetType, but rather a new adapter. For example, I recently wanted
to add Json processing and avoid using the FOM, so I thought a
JSONCollection TargetType might work. But then I realized that I could
just re-use collection TargeType and make a JSON CollectionAdapter.
Having more easily extensible TargetType system would make abdera more
general purpose. I suppose it's useful for doing things that aren't
exactly resource oriented, like processing a call for a service doc.
Shindig could extend TargetType to include processing our Discovery
Doc which is a more dynamic Service Doc.

davep

>
> Obviously IMHO.
> Cheers,
>
> Sergio B.
>
> --
> Sergio Bossa
> Software Passionate, Java Technologies Specialist and Open Source Enthusiast.
> Blog : http://sbtourist.blogspot.com
> Sourcesense - making sense of Open Source : http://www.sourcesense.com
> Pro-netics s.p.a. : http://www.pronetics.it
>

Re: Refactoring AbstractProvider to make it open for extension

[Sergio Bossa <se...@gmail.com>]

On Fri, Jul 4, 2008 at 8:53 PM, David Primmer <da...@gmail.com> wrote:

> I'm not sure what things you want to do with the request processing
> that force you to replace the 'if' statements that you mention.
> [CUT]

AbstractProvider uses "if" statements in order to decide what to do
depending on the TargetType: this prevents you to extend provider
features for supporting new target types, i.e. Open Search documents.
I know you can support OpenSearch (or whatever else) with filters,
resolvers, and alike, but I think it would be wrong: if it's the
provider job to decide what to do depending on the TargetType, you
shouldn't give this same responsibility to other classes.

Obviously IMHO.
Cheers,

Sergio B.

-- 
Sergio Bossa
Software Passionate, Java Technologies Specialist and Open Source Enthusiast.
Blog : http://sbtourist.blogspot.com
Sourcesense - making sense of Open Source : http://www.sourcesense.com
Pro-netics s.p.a. : http://www.pronetics.it

Re: Refactoring AbstractProvider to make it open for extension

[David Primmer <da...@gmail.com>]

I'm not sure what things you want to do with the request processing
that force you to replace the 'if' statements that you mention. There
are other ways to do this in Abdera so maybe you're just focused in
the wrong place. If you want to manage how urls and params are
resolved, as I do, you could work with the RouteManager. Or you could
add a custom TargetResolver. There are also Abdera Filters, that allow
you to apply some logic to every request (I still am not sure at which
point filters are applied but I think it's after The RequestContext is
built and the first run of target resolution has happened).

My understanding of how Abdera should work is that you should put all
the logic you need into creating enough targets to cover all your main
use cases and then use logic in your adapters or filters to do request
handling that applies to all requests. This could be OpenSearch, but
I've not looked at it in detail yet.

I'm still interested in seeing your new class in any case.

davep

On Fri, Jul 4, 2008 at 10:43 AM, Sergio Bossa <se...@gmail.com> wrote:
> Hi all,
>
> I'm working on a large project which will extensively use Apache Abdera.
> We will have to support the OpenSearch (OS from now on) specification,
> so while trying to enhance Abdera support for OS, I saw that the
> AbstractProvider class, which is the core of the whole server
> infrastructure, is poorly implemented as a bunch of "if" statements
> depending on the TargetType for actual request processing.
> This makes very hard to add request processing features to the
> AbstractProvider, because it forces you to modify the class itself by
> adding more and more "if" statements.
> That said, I've refactored the AbstractProvider class, removed all
> "if" statements and added the concept of "RequestProcessor", a
> strategy-like interface to implement for request processing logic: by
> doing so, adding request processing logic is just a matter of
> implementing an interface and registering it under a given TargetType.
> I've tested my work against all Abdera unit tests and everything works fine.
>
> What do you think about that?
> If you think it could be useful, I'd be more than happy to contribute it.
>
> Cheers,
>
> Sergio B.
>
> --
> Sergio Bossa
> Software Passionate, Java Technologies Specialist and Open Source Enthusiast.
> Blog : http://sbtourist.blogspot.com
> Sourcesense - making sense of Open Source : http://www.sourcesense.com
> Pro-netics s.p.a. : http://www.pronetics.it
>