You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-dev@xmlgraphics.apache.org by Glen Mazza <gr...@yahoo.com> on 2004/10/11 01:46:54 UTC
Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java
Jeremias,
The reason for getFOEventHandlerOverride()/getRendererOverride() in
FOUserAgent is to better black box the FOP system. This gives us a ton
of freedom internally of where we implement FOEventHandlers and
Renderers inside FOP. This has been a perfect example: So far over the
past few months, we've been setting the renderer in apps.Driver(), then
in fo.FOTreeControl, then in area.AreaTreeHandler, and then in
RenderPagesModel, and now RendererFactory. But note that during all
these movements, FOUserAgent.getRendererOverride() never had to change!
I don't know if this is what is meant by Avalon's Inversion of Control,
or if this is a separate concept, but it is providing us wonderful
flexibility.
If, instead, we provided a user interface that had the outside user
*directly* call RendererFactory.setRendererOverride(), we would be
forever stuck with having this code in a RendererFactory class. Good
for you, but some preferred it in AreaTreeModel, and some wanted it in
apps.Document or apps.Driver. Point is, future committers, because of
backwards compatibility issues, would be forever frozen with a single
design.
Thanks,
Glen
jeremias@apache.org schrieb:
>jeremias 2004/10/10 05:24:03
>
> Modified: src/java/org/apache/fop/fo FOTreeBuilder.java
> src/java/org/apache/fop/area RenderPagesModel.java
> src/java/org/apache/fop/apps FOUserAgent.java
> Added: src/java/org/apache/fop/render RendererFactory.java
> Log:
> Centralized Renderer and FOEventHandler creation in the RenderFactory class.
> Provide a similar mechanism as for the Renderers to override the FOEventHandler being used to be able to plug in custom FOEventHandlers. (I don't particularly like this approach but so be it for the moment.)
> Javadocs updates in FOUserAgent.
>
> Revision Changes Path
> 1.1 xml-fop/src/java/org/apache/fop/render/RendererFactory.java
>
> Index: RendererFactory.java
> ===================================================================
> /*
> * Copyright 2004 The Apache Software Foundation.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> * You may obtain a copy of the License at
> *
> * http://www.apache.org/licenses/LICENSE-2.0
> *
> * Unless required by applicable law or agreed to in writing, software
> * distributed under the License is distributed on an "AS IS" BASIS,
> * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> * See the License for the specific language governing permissions and
> * limitations under the License.
> */
>
> /* $Id: RendererFactory.java,v 1.1 2004/10/10 12:24:02 jeremias Exp $ */
>
> package org.apache.fop.render;
>
> import java.io.OutputStream;
>
> //Avalon
> import org.apache.avalon.framework.configuration.Configuration;
> import org.apache.avalon.framework.configuration.ConfigurationException;
> import org.apache.avalon.framework.container.ContainerUtil;
>
> //FOP
> import org.apache.fop.apps.FOPException;
> import org.apache.fop.apps.FOUserAgent;
> import org.apache.fop.area.AreaTreeHandler;
> import org.apache.fop.fo.Constants;
> import org.apache.fop.fo.FOEventHandler;
> import org.apache.fop.render.mif.MIFHandler;
> import org.apache.fop.render.rtf.RTFHandler;
>
> /**
> * Factory for FOEventHandlers and Renderers.
> */
> public class RendererFactory {
>
> /**
> * Creates a Renderer object based on render-type desired
> * @param renderType the type of renderer to use
> * @return the new Renderer instance
> * @throws IllegalArgumentException if an unsupported renderer type was requested
> */
> private static Renderer newInstance(int renderType) throws IllegalArgumentException {
>
> switch (renderType) {
> case Constants.RENDER_PDF:
> return new org.apache.fop.render.pdf.PDFRenderer();
> case Constants.RENDER_AWT:
> return new org.apache.fop.render.awt.AWTRenderer();
> case Constants.RENDER_PRINT:
> return new org.apache.fop.render.awt.AWTPrintRenderer();
> case Constants.RENDER_PCL:
> return new org.apache.fop.render.pcl.PCLRenderer();
> case Constants.RENDER_PS:
> return new org.apache.fop.render.ps.PSRenderer();
> case Constants.RENDER_TXT:
> return new org.apache.fop.render.txt.TXTRenderer();
> case Constants.RENDER_XML:
> return new org.apache.fop.render.xml.XMLRenderer();
> case Constants.RENDER_SVG:
> return new org.apache.fop.render.svg.SVGRenderer();
> default:
> throw new IllegalArgumentException("Invalid renderer type "
> + renderType);
> }
> }
>
> /**
> * Creates a Renderer object based on render-type desired
> * @param userAgent the user agent for access to configuration
> * @param renderType the type of renderer to use
> * @return the new Renderer instance
> * @throws FOPException if the renderer cannot be properly constructed
> */
> public static Renderer createRenderer(FOUserAgent userAgent, int renderType)
> throws FOPException {
> if (userAgent.getRendererOverride() != null) {
> return userAgent.getRendererOverride();
> } else {
> Renderer rend = newInstance(renderType);
> rend.setUserAgent(userAgent);
> String mimeType = rend.getMimeType();
> Configuration userRendererConfig = null;
> if (mimeType != null) {
> userRendererConfig
> = userAgent.getUserRendererConfig(mimeType);
> }
> if (userRendererConfig != null) {
> try {
> ContainerUtil.configure(rend, userRendererConfig);
> } catch (ConfigurationException e) {
> throw new FOPException(e);
> }
> }
> return rend;
> }
> }
>
>
> /**
> * Creates FOEventHandler instances based on the desired output.
> * @param userAgent the user agent for access to configuration
> * @param renderType the type of renderer to use
> * @param out the OutputStream where the output is written to (if applicable)
> * @return the newly constructed FOEventHandler
> * @throws FOPException if the FOEventHandler cannot be properly constructed
> */
> public static FOEventHandler createFOEventHandler(FOUserAgent userAgent,
> int renderType, OutputStream out) throws FOPException {
>
> if (userAgent.getFOEventHandlerOverride() != null) {
> return userAgent.getFOEventHandlerOverride();
> } else {
> if (renderType != Constants.RENDER_PRINT
> && renderType != Constants.RENDER_AWT) {
> if (out == null) {
> throw new IllegalStateException(
> "OutputStream has not been set");
> }
> }
>
> if (renderType == Constants.RENDER_MIF) {
> return new MIFHandler(userAgent, out);
> } else if (renderType == Constants.RENDER_RTF) {
> return new RTFHandler(userAgent, out);
> } else {
> if (renderType < Constants.RENDER_MIN_CONST
> || renderType > Constants.RENDER_MAX_CONST) {
> throw new IllegalArgumentException(
> "Invalid render ID#" + renderType);
> }
>
> return new AreaTreeHandler(userAgent, renderType, out);
> }
> }
> }
> }
>
>
>
> 1.54 +5 -25 xml-fop/src/java/org/apache/fop/fo/FOTreeBuilder.java
>
> Index: FOTreeBuilder.java
> ===================================================================
> RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/fo/FOTreeBuilder.java,v
> retrieving revision 1.53
> retrieving revision 1.54
> diff -u -r1.53 -r1.54
> --- FOTreeBuilder.java 24 Sep 2004 10:31:22 -0000 1.53
> +++ FOTreeBuilder.java 10 Oct 2004 12:24:02 -0000 1.54
> @@ -35,9 +35,7 @@
> import org.apache.commons.logging.LogFactory;
> import org.apache.fop.apps.FOPException;
> import org.apache.fop.apps.FOUserAgent;
> -import org.apache.fop.area.AreaTreeHandler;
> -import org.apache.fop.render.mif.MIFHandler;
> -import org.apache.fop.render.rtf.RTFHandler;
> +import org.apache.fop.render.RendererFactory;
> import org.apache.fop.fo.ElementMapping.Maker;
> import org.apache.fop.fo.extensions.ExtensionElementMapping;
> import org.apache.fop.fo.pagination.Root;
> @@ -95,32 +93,14 @@
> * @param renderType output type as defined in Constants class
> * @param foUserAgent in effect for this process
> * @param stream OutputStream to direct results
> + * @throws FOPException if the FOTreeBuilder cannot be properly created
> */
> public FOTreeBuilder(int renderType, FOUserAgent foUserAgent,
> OutputStream stream) throws FOPException {
>
> - if (renderType != Constants.RENDER_PRINT &&
> - renderType != Constants.RENDER_AWT) {
> - if (stream == null) {
> - throw new IllegalStateException(
> - "OutputStream has not been set");
> - }
> - }
> -
> - if (renderType == Constants.RENDER_MIF) {
> - foEventHandler = new MIFHandler(foUserAgent, stream);
> - } else if (renderType == Constants.RENDER_RTF) {
> - foEventHandler = new RTFHandler(foUserAgent, stream);
> - } else {
> - if (renderType < Constants.RENDER_MIN_CONST
> - || renderType > Constants.RENDER_MAX_CONST) {
> - throw new IllegalArgumentException(
> - "Invalid render ID#" + renderType);
> - }
> -
> - foEventHandler = new AreaTreeHandler(foUserAgent, renderType,
> - stream);
> - }
> + //This creates either an AreaTreeHandler and ultimately a Renderer, or
> + //one of the RTF-, MIF- etc. Handlers.
> + foEventHandler = RendererFactory.createFOEventHandler(foUserAgent, renderType, stream);
>
> // Add standard element mappings
> setupDefaultMappings();
>
>
>
> 1.6 +4 -57 xml-fop/src/java/org/apache/fop/area/RenderPagesModel.java
>
> Index: RenderPagesModel.java
> ===================================================================
> RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/area/RenderPagesModel.java,v
> retrieving revision 1.5
> retrieving revision 1.6
> diff -u -r1.5 -r1.6
> --- RenderPagesModel.java 25 Sep 2004 21:55:36 -0000 1.5
> +++ RenderPagesModel.java 10 Oct 2004 12:24:03 -0000 1.6
> @@ -27,17 +27,12 @@
> // XML
> import org.xml.sax.SAXException;
>
> -// avalon configuration
> -import org.apache.avalon.framework.configuration.Configuration;
> -import org.apache.avalon.framework.configuration.ConfigurationException;
> -
> // FOP
> import org.apache.fop.apps.FOPException;
> import org.apache.fop.apps.FOUserAgent;
> -import org.apache.fop.fo.Constants;
> import org.apache.fop.fonts.FontInfo;
> import org.apache.fop.render.Renderer;
> -import org.apache.fop.render.AbstractRenderer;
> +import org.apache.fop.render.RendererFactory;
>
> /**
> * This uses the store pages model to store the pages
> @@ -67,30 +62,12 @@
> * RENDER_PS, etc.)
> * @param fontInfo FontInfo object
> * @param stream OutputStream
> + * @throws FOPException if the renderer cannot be properly initialized
> */
> public RenderPagesModel (FOUserAgent userAgent, int renderType,
> FontInfo fontInfo, OutputStream stream) throws FOPException {
>
> - if (userAgent.getRendererOverride() != null) {
> - renderer = userAgent.getRendererOverride();
> - } else {
> - AbstractRenderer rend = createRenderer(renderType);
> - rend.setUserAgent(userAgent);
> - String mimeType = rend.getMimeType();
> - Configuration userRendererConfig = null;
> - if (mimeType != null) {
> - userRendererConfig
> - = userAgent.getUserRendererConfig(mimeType);
> - }
> - if (userRendererConfig != null) {
> - try {
> - rend.configure(userRendererConfig);
> - } catch (ConfigurationException e) {
> - throw new FOPException(e);
> - }
> - }
> - renderer = rend;
> - }
> + renderer = RendererFactory.createRenderer(userAgent, renderType);
>
> try {
> renderer.setupFontInfo(fontInfo);
> @@ -106,37 +83,6 @@
> }
>
> /**
> - * Creates an AbstractRenderer object based on render-type desired
> - * @param renderType the type of renderer to use
> - * @return AbstractRenderer the new Renderer instance
> - * @throws IllegalArgumentException if an unsupported renderer type was requested
> - */
> - private AbstractRenderer createRenderer(int renderType) throws IllegalArgumentException {
> -
> - switch (renderType) {
> - case Constants.RENDER_PDF:
> - return new org.apache.fop.render.pdf.PDFRenderer();
> - case Constants.RENDER_AWT:
> - return new org.apache.fop.render.awt.AWTRenderer();
> - case Constants.RENDER_PRINT:
> - return new org.apache.fop.render.awt.AWTPrintRenderer();
> - case Constants.RENDER_PCL:
> - return new org.apache.fop.render.pcl.PCLRenderer();
> - case Constants.RENDER_PS:
> - return new org.apache.fop.render.ps.PSRenderer();
> - case Constants.RENDER_TXT:
> - return new org.apache.fop.render.txt.TXTRenderer();
> - case Constants.RENDER_XML:
> - return new org.apache.fop.render.xml.XMLRenderer();
> - case Constants.RENDER_SVG:
> - return new org.apache.fop.render.svg.SVGRenderer();
> - default:
> - throw new IllegalArgumentException("Invalid renderer type "
> - + renderType);
> - }
> - }
> -
> - /**
> * Start a new page sequence.
> * This tells the renderer that a new page sequence has
> * started with the given title.
> @@ -260,6 +206,7 @@
>
> /**
> * End the document. Render any end document extensions.
> + * @see org.apache.fop.area.AreaTreeModel#endDocument()
> */
> public void endDocument() throws SAXException {
> // render any pages that had unresolved ids
>
>
>
> 1.18 +35 -9 xml-fop/src/java/org/apache/fop/apps/FOUserAgent.java
>
> Index: FOUserAgent.java
> ===================================================================
> RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/apps/FOUserAgent.java,v
> retrieving revision 1.17
> retrieving revision 1.18
> diff -u -r1.17 -r1.18
> --- FOUserAgent.java 24 Sep 2004 10:31:22 -0000 1.17
> +++ FOUserAgent.java 10 Oct 2004 12:24:03 -0000 1.18
> @@ -36,6 +36,7 @@
>
> // FOP
> import org.apache.fop.fo.ElementMapping;
> +import org.apache.fop.fo.FOEventHandler;
> import org.apache.fop.pdf.PDFEncryptionParams;
> import org.apache.fop.render.Renderer;
>
> @@ -65,12 +66,14 @@
> public Map defaults = new java.util.HashMap();
> /** Map containing XML handlers for various document types */
> public Map handlers = new java.util.HashMap();
> +
> private String baseURL;
> private PDFEncryptionParams pdfEncryptionParams;
> private float px2mm = (25.4f / 72); //dpi (=25.4/dpi)
> private HashMap rendererOptions = new java.util.HashMap();
> private InputHandler inputHandler = null;
> private Renderer rendererOverride = null;
> + private FOEventHandler foEventHandlerOverride = null;
> /* user configuration */
> private Configuration userConfig = null;
> private Log log = LogFactory.getLog("FOP");
> @@ -112,7 +115,7 @@
>
> /**
> * Add the element mapping with the given class name.
> - * @param mappingClassName the class name representing the element mapping.
> + * @param elementMapping the class name representing the element mapping.
> */
> public void addElementMapping(ElementMapping elementMapping) {
> if (additionalElementMappings == null) {
> @@ -130,22 +133,40 @@
> }
>
> /**
> - * Sets the producer of the document.
> - * @param producer source of document
> + * Sets an explicit renderer to use which overrides the one defined by the
> + * render type setting.
> + * @param renderer the Renderer instance to use
> */
> public void setRendererOverride(Renderer renderer) {
> this.rendererOverride = renderer;
> }
>
> /**
> - * Returns the producer of the document
> - * @return producer name
> + * Returns the overriding Renderer instance, if any.
> + * @return the overriding Renderer or null
> */
> public Renderer getRendererOverride() {
> return rendererOverride;
> }
>
> /**
> + * Sets an explicit FOEventHandler instance which overrides the one
> + * defined by the render type setting.
> + * @param handler the FOEventHandler instance
> + */
> + public void setFOEventHandlerOverride(FOEventHandler handler) {
> + this.foEventHandlerOverride = handler;
> + }
> +
> + /**
> + * Returns the overriding FOEventHandler instance, if any.
> + * @return the overriding FOEventHandler or null
> + */
> + public FOEventHandler getFOEventHandlerOverride() {
> + return this.foEventHandlerOverride;
> + }
> +
> + /**
> * Sets the producer of the document.
> * @param producer source of document
> */
> @@ -179,7 +200,7 @@
>
> /**
> * Sets the creation date of the document.
> - * @param creation date of document
> + * @param creationDate date of document
> */
> public void setCreationDate(Date creationDate) {
> this.creationDate = creationDate;
> @@ -203,7 +224,7 @@
>
> /**
> * Set the user configuration.
> - * @param user configuration
> + * @param userConfig configuration
> */
> public void setUserConfig(Configuration userConfig) {
> this.userConfig = userConfig;
> @@ -217,6 +238,11 @@
> return userConfig;
> }
>
> + /**
> + * Returns the configuration subtree for a specific renderer.
> + * @param mimeType MIME type of the renderer
> + * @return the requested configuration subtree, null if there's no configuration
> + */
> public Configuration getUserRendererConfig (String mimeType) {
>
> if (userConfig == null || mimeType == null) {
> @@ -238,7 +264,7 @@
> // silently pass over configurations without mime type
> }
> }
> - log.debug((userRendererConfig==null ? "No u" : "U")
> + log.debug((userRendererConfig == null ? "No u" : "U")
> + "ser configuration found for MIME type " + mimeType);
> return userRendererConfig;
> }
>
>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: fop-cvs-unsubscribe@xml.apache.org
>For additional commands, e-mail: fop-cvs-help@xml.apache.org
>
>
>
>
Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java
Posted by Jeremias Maerki <de...@greenmail.ch>.
Sorry for the delay.
On 12.10.2004 01:20:54 Glen Mazza wrote:
> --- Jeremias Maerki <de...@greenmail.ch> wrote:
>
> > Glen,
> >
> > I don't
> > particularly like selecting renderers by integer
> > constant.
>
> FOP has been using integers for six years now, and as
> I explained earlier [1], MIME types don't make a very
> good primary key for renderers, because not all
> renderers have a MIME type, also multiple renderers
> can share the same MIME type (e.g. our PDF renderer
> and Finn's version of it.). In some cases, we would
> may indeed need a RENDER_PDF_V14 and an
> RENDER_PDF_V15, for example.
I would disagree with that. There are ways to do all that pretty cleanly.
The point is that with your approach you always (!) have to work in Java
code to use a non-standard renderer. It can't be plugged in from the
command-line for example.
> Again, integer constants also work well in arrays,
> they are ultra-fast, and, as a bonus, "new
> Fop(RENDER_PFD)" is a ultra-quick-to-catch
> compile-time error, while new FOP("application/pfd")
> would turn into a pesky run-time error (and even a ML
> question). The integer constants are also much easier
> to remember than MIME types: RENDER_ followed by the
> desired render type.
Performance is really unimportant when we're talking about choosing the
renderer. The compile-time checking argument is valid, however.
> [1]
> http://marc.theaimsgroup.com/?l=fop-dev&m=109261374110951&w=2
>
> > I like
> > pluggability.
>
> We sufficiently have that for our next release, where
> I define "sufficiently" as "as much as we have already
> in 0.20.5, and basically what AH/RX offers". We have
> already learned that adding too many hooks,
> interfaces, visitor patterns, *before* the
> layout/renderer work is done, results in FOP never
> getting finished because the code becomes too hard to
> work through. So let's get the code working first
> (moving the renderers over, whitespace handling, PDF
> bookmarks, layout), *then* put it into Mandarin with
> the advanced API many desire.
As you like. But this will stay on my personal todo list.
> I share your impatience with the next release not
> coming out yet, but we have to keep the code simple to
> get more people to look at and help in the code.
I agree, but IMHO it's confusing that we're choosing standard renderers
in one place (Fop.java) but provide a possibility to set non-standard
renderer in another (FOUserAgent.java).
> > I'd prefer to register all our
> > renderers in a central
> > registry. Integrators could plug in their renderers
> > using the service
> > interface just as the FOP extension can.
>
> That's beautiful post-0.30/1.0 talk that should be
> added to our roadmap. But, in the meantime, we
> already have a perfectly satisfactory
> FOUserAgent.setRendererOverride() that will satisfy
> the users that currently have renderer overrides. I
> would prefer a minimal API with as much black-boxed as
> possible to allow future implementors as much
> architectural freedom as possible.
>
> Besides, what you prefer now is not what the team
> preferred six months ago, or a year ago, or 18 months
> ago, etc., etc. Our API/internal architecture desires
> keep changing.
Are they? Is that how you justify ignoring API discussions held earlier?
> > You could
> > even override
> > renderers (for "application/postscript" for example)
>
> We already have that with
> setRendererOverride(Renderer). You can't use MIME
> type for reasons given above.
I can't override a Renderer without changing Java code.
> > if you have a
> > renderer with some custom extensions. IMO
> > FOUserAgent is already
> > overloaded with too many different things.
>
> That is because we tend to, whenever any user might
> want something, give him/her an API method for it.
> The only way we can satisfy these many needs without
> forever locking the FOP architecture (each of these
> switches internally modifying the behavior of one
> internal class or another) is to add the method to
> FOUserAgent, and have internal classes read it, rather
> than the other way around. Furthermore, we have one
> class--FOUserAgent--for these parameters to make the
> embedded/servlet API easier for non-advanced users.
> (BTW, FOUserAgent should be reduced somewhat anyway,
> when we create fox:metadata or whatever.)
Means keep Fop.java as lean as possible but no problem with FOUserAgent
getting bloated. I don't get it.
> > I think
> > that the overrides
> > don't belong there, or rather they are probably
> > unnecessary given a
> > better renderer selection mechanism.
> >
>
> Oh, I think they do, it is sufficient for our next
> release. Nothing is easier, nothing is simpler for
> embedded/servlet users. Take a look at the 1.0
> embedded examples.
My desires for the API don't complicate things for the embedded examples.
> > What triggered the creation of the RendererFactory
> > was not IoC but the
> > other Avalon concept: Separation of concerns. I
> > think that handling
> > renderer and FOEventHandler creation in a separate
> > class rather than in
> > the quite crowded FOTreeBuilder and RenderPagesModel
> > improves code
> > readability.
>
> FOTreeBuilder and RenderPagesModel IMO aren't that
> crowded, they are the size they need to be for the
> business logic they are responsible for. Still, I've
> bent over backwards to remove as many classes and
> unneeded methods to simplify the code as much as
> possible. Furthermore, bouncing around classes IMO
> raises stress rather than lowering it.
Ah, and you didn't do that lately? I believe that my change has
simplyfied FOTreeBuilder and RenderPagesModel quite a bit.
> As for Avalon SoC, the concerns *were* properly
> separated: the FOTreeBuilder chose the FOEventHandler
> based on the desired render type, and RenderPagesModel
> chooses its renderer also on the desired render type.
> It is business logic purely the departments of the
> FOTreeBuilder and RenderPagesModel.
>
> Still, I'm not objecting to this change as long as
> it's not the external API. (Indeed, I support it,
> because it makes you happy, and when you're happy
> you're more productive.)
>
> >
> > Don't get me wrong. I can live with the current
> > situation as long as I
> > can plug in my custom FOEventHandler. You don't
> > object to that, do you?
>
> Not at all, because it does not directly touch
> internal code. Future committers can move
> FOEventHandler handling anywhere they like without the
> FOUserAgent method changing.
>
>
> > By the way, my change didn't change the end-user
> > API, so I don't see any
> > reason why RendererFactory is a bad idea. Do I miss
> > your point?
> >
>
> Only if you were planning on exposing RendererFactory
> to the external API.
I don't.
> It is the fact that the end-user
> API didn't need to change for your internal change, is
> why my design of FOUserAgent is solid. (Think of a
> database view--where the view accesses several
> underlying tables. By having applications query the
> view instead of the tables, the DBA/Data Modeler is
> able to make table changes without concern about the
> application needing updating--only the view definition
> changes. This is what I would like our API to be.)
Ok, and I have a few different ideas about it.
> Future committers must have the ability to get rid of
> RendererFactory for something they deem better, just
> like you changed my design for something you thought
> was better. Our external API should mask our internal
> workings--which, along with keeping things simple, are
> my main concerns here.
And with my change I tried to make things even simpler.
Jeremias Maerki
Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java
Posted by Glen Mazza <gr...@yahoo.com>.
--- Jeremias Maerki <de...@greenmail.ch> wrote:
> Glen,
>
> I don't
> particularly like selecting renderers by integer
> constant.
FOP has been using integers for six years now, and as
I explained earlier [1], MIME types don't make a very
good primary key for renderers, because not all
renderers have a MIME type, also multiple renderers
can share the same MIME type (e.g. our PDF renderer
and Finn's version of it.). In some cases, we would
may indeed need a RENDER_PDF_V14 and an
RENDER_PDF_V15, for example.
Again, integer constants also work well in arrays,
they are ultra-fast, and, as a bonus, "new
Fop(RENDER_PFD)" is a ultra-quick-to-catch
compile-time error, while new FOP("application/pfd")
would turn into a pesky run-time error (and even a ML
question). The integer constants are also much easier
to remember than MIME types: RENDER_ followed by the
desired render type.
[1]
http://marc.theaimsgroup.com/?l=fop-dev&m=109261374110951&w=2
> I like
> pluggability.
We sufficiently have that for our next release, where
I define "sufficiently" as "as much as we have already
in 0.20.5, and basically what AH/RX offers". We have
already learned that adding too many hooks,
interfaces, visitor patterns, *before* the
layout/renderer work is done, results in FOP never
getting finished because the code becomes too hard to
work through. So let's get the code working first
(moving the renderers over, whitespace handling, PDF
bookmarks, layout), *then* put it into Mandarin with
the advanced API many desire.
I share your impatience with the next release not
coming out yet, but we have to keep the code simple to
get more people to look at and help in the code.
> I'd prefer to register all our
> renderers in a central
> registry. Integrators could plug in their renderers
> using the service
> interface just as the FOP extension can.
That's beautiful post-0.30/1.0 talk that should be
added to our roadmap. But, in the meantime, we
already have a perfectly satisfactory
FOUserAgent.setRendererOverride() that will satisfy
the users that currently have renderer overrides. I
would prefer a minimal API with as much black-boxed as
possible to allow future implementors as much
architectural freedom as possible.
Besides, what you prefer now is not what the team
preferred six months ago, or a year ago, or 18 months
ago, etc., etc. Our API/internal architecture desires
keep changing.
> You could
> even override
> renderers (for "application/postscript" for example)
We already have that with
setRendererOverride(Renderer). You can't use MIME
type for reasons given above.
> if you have a
> renderer with some custom extensions. IMO
> FOUserAgent is already
> overloaded with too many different things.
That is because we tend to, whenever any user might
want something, give him/her an API method for it.
The only way we can satisfy these many needs without
forever locking the FOP architecture (each of these
switches internally modifying the behavior of one
internal class or another) is to add the method to
FOUserAgent, and have internal classes read it, rather
than the other way around. Furthermore, we have one
class--FOUserAgent--for these parameters to make the
embedded/servlet API easier for non-advanced users.
(BTW, FOUserAgent should be reduced somewhat anyway,
when we create fox:metadata or whatever.)
> I think
> that the overrides
> don't belong there, or rather they are probably
> unnecessary given a
> better renderer selection mechanism.
>
Oh, I think they do, it is sufficient for our next
release. Nothing is easier, nothing is simpler for
embedded/servlet users. Take a look at the 1.0
embedded examples.
> What triggered the creation of the RendererFactory
> was not IoC but the
> other Avalon concept: Separation of concerns. I
> think that handling
> renderer and FOEventHandler creation in a separate
> class rather than in
> the quite crowded FOTreeBuilder and RenderPagesModel
> improves code
> readability.
FOTreeBuilder and RenderPagesModel IMO aren't that
crowded, they are the size they need to be for the
business logic they are responsible for. Still, I've
bent over backwards to remove as many classes and
unneeded methods to simplify the code as much as
possible. Furthermore, bouncing around classes IMO
raises stress rather than lowering it.
As for Avalon SoC, the concerns *were* properly
separated: the FOTreeBuilder chose the FOEventHandler
based on the desired render type, and RenderPagesModel
chooses its renderer also on the desired render type.
It is business logic purely the departments of the
FOTreeBuilder and RenderPagesModel.
Still, I'm not objecting to this change as long as
it's not the external API. (Indeed, I support it,
because it makes you happy, and when you're happy
you're more productive.)
>
> Don't get me wrong. I can live with the current
> situation as long as I
> can plug in my custom FOEventHandler. You don't
> object to that, do you?
Not at all, because it does not directly touch
internal code. Future committers can move
FOEventHandler handling anywhere they like without the
FOUserAgent method changing.
> By the way, my change didn't change the end-user
> API, so I don't see any
> reason why RendererFactory is a bad idea. Do I miss
> your point?
>
Only if you were planning on exposing RendererFactory
to the external API. It is the fact that the end-user
API didn't need to change for your internal change, is
why my design of FOUserAgent is solid. (Think of a
database view--where the view accesses several
underlying tables. By having applications query the
view instead of the tables, the DBA/Data Modeler is
able to make table changes without concern about the
application needing updating--only the view definition
changes. This is what I would like our API to be.)
Future committers must have the ability to get rid of
RendererFactory for something they deem better, just
like you changed my design for something you thought
was better. Our external API should mask our internal
workings--which, along with keeping things simple, are
my main concerns here.
Sorry for the long (and perhaps repetitive) post.
Thanks,
Glen
Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java
Posted by Jeremias Maerki <de...@greenmail.ch>.
Glen,
I don't want a RendererFactory.setRendererOverride(). I don't
particularly like selecting renderers by integer constant. I like
pluggability. I'd prefer to register all our renderers in a central
registry. Integrators could plug in their renderers using the service
interface just as the FOP extension can. You could even override
renderers (for "application/postscript" for example) if you have a
renderer with some custom extensions. IMO FOUserAgent is already
overloaded with too many different things. I think that the overrides
don't belong there, or rather they are probably unnecessary given a
better renderer selection mechanism.
What triggered the creation of the RendererFactory was not IoC but the
other Avalon concept: Separation of concerns. I think that handling
renderer and FOEventHandler creation in a separate class rather than in
the quite crowded FOTreeBuilder and RenderPagesModel improves code
readability.
Don't get me wrong. I can live with the current situation as long as I
can plug in my custom FOEventHandler. You don't object to that, do you?
The RendererFactory simply came out of the fact that I need to plug in
my FOEventHandler and I found that the code was IMHO a bit unclean.
By the way, my change didn't change the end-user API, so I don't see any
reason why RendererFactory is a bad idea. Do I miss your point?
On 11.10.2004 01:46:54 Glen Mazza wrote:
> Jeremias,
>
> The reason for getFOEventHandlerOverride()/getRendererOverride() in
> FOUserAgent is to better black box the FOP system. This gives us a ton
> of freedom internally of where we implement FOEventHandlers and
> Renderers inside FOP. This has been a perfect example: So far over the
> past few months, we've been setting the renderer in apps.Driver(), then
> in fo.FOTreeControl, then in area.AreaTreeHandler, and then in
> RenderPagesModel, and now RendererFactory. But note that during all
> these movements, FOUserAgent.getRendererOverride() never had to change!
> I don't know if this is what is meant by Avalon's Inversion of Control,
> or if this is a separate concept, but it is providing us wonderful
> flexibility.
>
> If, instead, we provided a user interface that had the outside user
> *directly* call RendererFactory.setRendererOverride(), we would be
> forever stuck with having this code in a RendererFactory class. Good
> for you, but some preferred it in AreaTreeModel, and some wanted it in
> apps.Document or apps.Driver. Point is, future committers, because of
> backwards compatibility issues, would be forever frozen with a single
> design.
Jeremias Maerki