You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Pier Fumagalli <pi...@betaversion.org> on 2004/09/30 17:32:10 UTC
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/components/
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.PARENT_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.getValidityForEventPipeline())
;
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);
+ }
+ }
}
Re: SitemapSource problems
Posted by Pier Fumagalli <pi...@betaversion.org>.
Oh, yeah... I was using getPath to debug the thing... It's a leftover.
I'll fix-and-post it.
Pier
On 1 Oct 2004, at 12:14, Carsten Ziegeler wrote:
> 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:pier@betaversion.org]
>> Sent: Thursday, September 30, 2004 5:32 PM
>> To: dev@cocoon.apache.org
>> 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);
>> + }
>> + }
>> }
>>
>
>
RE: SitemapSource problems
Posted by Carsten Ziegeler <cz...@apache.org>.
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:pier@betaversion.org]
> Sent: Thursday, September 30, 2004 5:32 PM
> To: dev@cocoon.apache.org
> 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);
> + }
> + }
> }
>