cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Fagerstrom <dani...@nada.kth.se>
Subject Re: isolate the pipeline "component" from rest of cocoon
Date Tue, 24 Oct 2006 13:10:51 GMT
Vadim Gritsenko skrev:
> Daniel Fagerstrom wrote:
>>
>> If we want to factor out the pipeline functionality to an own block 
>> that doesn't depend on the rest of Cocoon it will certainly be a lot 
>> of work. But I think it would be worthwhile anyway as it would 
>> increase usability and make the architecture easier to follow and 
>> maintain.
>
> I'm not that sure that pipelines should be extracted first. One would 
> think that it is a sitemap engine that is logically sitting on top of 
> pipelines.
>
>    +-----------------------------+
>    | Env Impl + Sitemap Servlet  |
>    +-----------------------------+
>    | Sitemap Engine              |
>    +-----------------------------+--------------------+
>    | Environment API + Pipelines | Sitemap Components |
>    +-----------------------------+--------------------+
>    | Container + Core Components                      |
>    +--------------------------------------------------+
>
Agree about your view, not certain that there is much use in splitting 
up the sitemap stuff in sitemap engine and env impl + sitemap servlet 
though.

We need names for the blocks, what about cocoon-sitemap, 
cocoon-pipeline, cocoon-core (or cocoon-container) and 
cocoon-sitemap-components?

>> We should IMO strive towards a layered architecture where we have a 
>> sitemap (treeprocessor) block that depends on a pipeline block. Where 
>> the pipeline block contains the pipeline, pipeline components and the 
>> environment as well as numerous classes supporting them.
>
> Sounds good. Only point is that we should start with outermost layer 
> first - extracting CocoonServlet first, following by sitemap engine, 
> and only then pipeline block.
Yes. Although I think we could split out the two upper layer in your 
picture at once and split them in a servlet and engine part later if needed.

Next we need to plan the work so that it doesn't disturb the rest of the 
work to much, and so that it can be performed incrementally. I think 
that the Pipeline part is the largest part of Cocoon core so it would 
probably be simplest to move away the sitemap, container and sitemap 
component part from the core and keep the sitemap part.

The sitemap components should be the simplest part to move out.

Something that can be complicated and tedious to keep track on during 
the work is all the dependencies in the poms that depend on cocoon-core. 
One way would be to start with moving all code within cocoon-core to 
e.g. cocoon-pipeline and let cocoon-core depend on cocoon-pipeline, 
cocoon-sitemap, cocoon-sitemap, cocoon-container and 
cocoon-sitemap-components. Then we can narrow the dependencies when we 
have finished the work.

If some part of the work need larger or more complicated refactorings, 
we probably need to do such work in a branch.

At last we of course need to be a number of people that are prepared to 
work on it. I volunteer to do a part of the job.
>> Taking a look at the pipeline code there are some dependencies on 
>> that would need to be removed. The ProcessingPipeline interface 
>> depends on the class o.a.c.sitemap.SitemapErrorHandler that in turn 
>> depend on treeprocessor stuff. It would be better to create an 
>> interface e.g. PipelineErrorHandler that the ProcessingPipeline 
>> depends on.
> +1
>
>> There is also a Processor.InternalPipeline descriptor that is used 
>> for the error handling within the pipeline implementations. It would 
>> IMO be better to move this internal class from the Processor 
>> interface to the components.pipeline package.
> That's new for 2.2, I don't see it in 2.1. No objections for the move 
> here also.
>
>> Both these changes affects interfaces, but I would be surprised if 
>> anyone have implemented Processor or ProcessingPipeline outside our 
>> code base, so it shouldn't matter that much.
> Since InternalPipelineDescription is 2.2 only, it was not released yet.
> SitemapErrorHandler can implement PipelineErrorHandler without 
> affecting anybody.
>> The AbstractProcessingPipeline gets its SourceResolver through 
>> EnvironmentHelper.getCurrentProcessor().getSourceResolver(), we need 
>> to use dependency injection for it instead to make the implementation 
>> easier to reuse outside the treeprocessor.
>         // TODO (CZ) Get the processor set via IoC
>         this.processor = EnvironmentHelper.getCurrentProcessor();
>
> AbstractProcessingPipeline does not even use processor, only resolver. 
> (Did not check derived classes though).
OK, then we can start with this part of the work right away.

/Daniel


Mime
View raw message