The patch looks good to me, apart from one problem :) The validity objects have to be serializable, so the cache can write them to disk; As your new SitemapSourceValidity stores the SitemapSource object - a component - in an instance variable this is not serializable as components can't be serialized.
But as far as I see it, the getPath() method isn't used and you don't need the reference to the SitemapSource. So you can remove them, make the validity object serializable and everything should work. Carsten > -----Original Message----- > From: Pier Fumagalli [mailto:[EMAIL PROTECTED] > Sent: Thursday, September 30, 2004 5:32 PM > To: [EMAIL PROTECTED] > Subject: SitemapSource problems > > While playing around with the IncludeTransformer, to generate > an XSLT stylesheet from a bunch of included sources and so > on, I stepped on a "little" problem in the SitemapSource. > > Basically, the XSLTProcessor (I assume that the same behavior > might happen in other components as well), caches the source > and its validity as soon as it gets a hold on them, even > _before_ actually reading the XSLT from the source. > > Now, the problem is that within the SitemapSource, sometimes > (when, for example, one of the elements in a pipeline is an > IncludeTransformer), the "validity" instance gets reset, and > therefore (in the example of the XSLTProcessor), the validity > acquired before producing XML data from the content is a > different instance from the validity instance I acquire AFTER > producing the XML. > > The problem is that if I don't re-update the validity > instance after reding the data, I am going to get a wrong > validity, one that will never be valid, in some cases, as the > content associated with that validity has never been generated. > > Now, the problem lies in the fact that the "init()" and > "reset()/refresh()" methods change the instance of the > validity returned by "getValidity()". > > I patched this behavior by basically nesting the calculated > validity in an instance which will always be returned to the > caller, so, that from the outside, it doesn't matter whether > the source got internally refreshed, the instance they can > see is _always_ the same. > > The only time when the validity instance is (re)created is at > construction and in the "recycle()" method... > > Attached here is a patch which fixes this behavior, and > allows cacheability of (for example) XSLT templates generated > by pipelines using the IncludeTransformer. > > Given this is a kind of a "biggie", I'd like to have someone > to review it before applying it to the tree... I'd suspect > this might also fix some other problems somewhere else... > > Pier > > /src/java/org/apache/cocoon/components/source/impl/SitemapSource.java > SitemapSource.java > --- > /Users/pier/Workspace/cocoon-2.1/src/java/org/apache/cocoon/co > mponents/ > source/impl/SitemapSource.java Wed Sep 22 00:24:47 2004 > +++ SitemapSource.java Thu Sep 30 16:10:50 2004 > @@ -60,7 +60,7 @@ > implements Source, XMLizable { > > /** validities for the internal pipeline */ > - private SourceValidity sourceValidity; > + private SitemapSourceValidity validity; > > /** The system id */ > private final String systemId; > @@ -86,9 +86,6 @@ > /** The redirect <code>Source</code> */ > private Source redirectSource; > > - /** Redirect validity */ > - private SourceValidity redirectValidity; > - > /** The <code>SAXException</code> if unable to get resource */ > private SAXException exception; > > @@ -208,6 +205,9 @@ > > this.environment.getObjectModel().remove(ObjectModelHelper.PAR > ENT_CONTEX > T); > } > > + // create a new validity holder > + this.validity = new SitemapSourceValidity(this); > + > // initialize > this.init(); > } > @@ -300,10 +300,7 @@ > * <code>null</code> is returned. > */ > public SourceValidity getValidity() { > - if (this.redirectSource != null) { > - return this.redirectValidity; > - } > - return this.sourceValidity; > + return this.validity; > } > > /** > @@ -343,7 +340,7 @@ > > this.pipelineProcessor); > try { > > this.processingPipeline.prepareInternal(this.environment); > - this.sourceValidity = > this.processingPipeline.getValidityForEventPipeline(); > + > this.validity.set(this.processingPipeline.getValidityForEventP > ipeline()) > ; > this.mimeType = > this.environment.getContentType(); > > final String eventPipelineKey = > this.processingPipeline.getKeyForEventPipeline(); > @@ -371,7 +368,7 @@ > this.sourceResolver = (SourceResolver) > this.manager.lookup(SourceResolver.ROLE); > } > this.redirectSource = > this.sourceResolver.resolveURI(redirectURL); > - this.redirectValidity = > this.redirectSource.getValidity(); > + this.validity.set(this.redirectSource.getValidity()); > this.mimeType = this.redirectSource.getMimeType(); > } > } catch (SAXException e) { > @@ -450,8 +447,7 @@ > this.redirectSource = null; > } > > - this.sourceValidity = null; > - this.redirectValidity = null; > + this.validity.set(null); > > this.environment.reset(); > this.exception = null; > @@ -463,6 +459,7 @@ > * Recyclable > */ > public void recycle() { > + this.validity = new SitemapSourceValidity(this); > this.reset(); > if (this.sourceResolver != null) { > this.manager.release(this.sourceResolver); > @@ -497,4 +494,37 @@ > return java.util.Collections.EMPTY_LIST.iterator(); > } > > + /** > + * A simple SourceValidity protecting callers from resets. > + */ > + public static final class SitemapSourceValidity implements > SourceValidity { > + > + public SourceValidity nested_validity = null; > + public SitemapSource nested_source = null; > + > + private SitemapSourceValidity(SitemapSource source) { > + this.nested_source = source; > + } > + > + private void set(SourceValidity validity) { > + this.nested_validity = validity; > + } > + > + public int isValid() { > + return(this.nested_validity != null? > + this.nested_validity.isValid(): > + SourceValidity.INVALID); > + } > + > + public int isValid(SourceValidity validity) { > + return(this.nested_validity != null? > + this.nested_validity.isValid(validity): > + SourceValidity.INVALID); > + } > + > + public String getPath() { > + if (this.nested_source.systemId == null) > return("[UNKNOWN]"); > + return(this.nested_source.systemId); > + } > + } > } >
