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 |
+--------------------------------------------------+
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.
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).
Vadim
