DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT <http://issues.apache.org/bugzilla/show_bug.cgi?id=31012>. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=31012 CachingPipeline validity and new IncludeTransformer Summary: CachingPipeline validity and new IncludeTransformer Product: Cocoon 2 Version: 2.1.5 Platform: PC OS/Version: Linux Status: NEW Severity: Normal Priority: Other Component: general components AssignedTo: [EMAIL PROTECTED] ReportedBy: [EMAIL PROTECTED] CC: [EMAIL PROTECTED] This bug is related to an earlier closed bug #30356. We are now using Unico's new org.apache.cocoon.transformation.IncludeTransformer, which is working fantastically for us. In a nutshell, the key feature of this new class for us is that it 'takes on' its included source's validity as its own validity after processing the included source/pipeline. This means the validities of included content 'bubble up'. However, I believe this new Transformer has exposed a bug with the way the CachingProcessingPipeline returns its validity in its getValidityForEventPipeline method. The problem occurs when a pipeline that contains an IncludeTransformer step is invoked and a valid CachedResponse is found for it. (All this action happens primarily in the AbstractCachingProcessingPipeline class). The cached response is validated and returned fine, but the problem is later when getValidityForEventPipeline is called on the pipeline processing instance. This method fetches the validity objects from all of its cacheable components in the pipeline (which in this case have only just been initialised but not executed, because a cached response was found), chucks them all in an AggregatedValidity and sends this back representing the validity of the entire pipeline. This method is used by the SitemapSource class to set its own validity. The problem here is that the IncludeTransformer's validity doesn't get populated until it is executed, as naturally it doesn't know what the validity of the included source is until is resolves it. So calling IncludeTransformer.getValidity when it hasn't been executed (which is what getValidityForEventPipeline does when a valid CachedResponse is found) will always return an empty MultiSourceValidity, which is of course invalid. Therefore, the Source validity for this pipeline will always be invalid because the IncludeTransformer step validity is not correct. My fix: ======== The thing is, when you have a valid CachedResponse, you actually already have all the correct validities for the pipeline inside it. There is no need to re-fetch them by calling getValidity on each pipeline component. So, I altered getValidityForEventPipeline in AbstractCachingProcessingPipeline to use the validities that were inside the CachedResponse, if one exists and is valid. The method now looks like this: public SourceValidity getValidityForEventPipeline() { //***** This if block was added by OliverP ****** if (this.cachedResponse != null) { final AggregatedValidity validity = new AggregatedValidity(); for (int i=0; i < this.toCacheSourceValidities.length; i++) { validity.add(this.toCacheSourceValidities[i]); } if (this.getLogger().isDebugEnabled()) { this.getLogger().debug("OliverP: getValidityForEventPipeline: returning cached response validity: " + validity); } return validity; } else { int vals = 0; if ( null != this.toCacheKey && !this.cacheCompleteResponse && this.firstNotCacheableTransformerIndex == super.transformers.size()) { vals = this.toCacheKey.size(); } else if ( null != this.fromCacheKey && !this.completeResponseIsCached && this.firstProcessedTransformerIndex == super.transformers.size()) { vals = this.fromCacheKey.size(); } if ( vals > 0 ) { final AggregatedValidity validity = new AggregatedValidity(); for(int i=0; i < vals; i++) { validity.add(this.getValidityForInternalPipeline(i)); //validity.add(new DeferredPipelineValidity(this, i)); } return validity; } return null; } } The CachedResponse's validities have been stored in the toCacheSourceValidities member variable. I altered the validatePipeline method in AbstractCachingProcessingPipeline, near the end, to update this variable when it has been determined that the CachedResponse is valid - before we lose the handle on the CachedResponse object. This is the code block (about line #563) from the validatePipeline method: if (responseIsValid) { if (this.getLogger().isDebugEnabled()) { this.getLogger().debug("validatePipeline: using valid cached content for '" + environment.getURI() + "'."); } // we are valid, ok that's it this.cachedResponse = response.getResponse(); this.cachedLastModified = response.getLastModified(); //***** Added by OliverP ***** this.toCacheSourceValidities = fromCacheValidityObjects; } else { if (this.getLogger().isDebugEnabled()) { this.getLogger().debug("validatePipeline: cached content is invalid for '" + environment.getURI() + "'."); } // we are not valid! ...... After this, it worked great! (so far...) I haven't got subversion going yet, so I'm not able to submit a diff format to make these changes clearer. If anyone wants this, let me know and I'll try and whizz one up. Any comments? Any pitfalls with this fix? If this is a valid bug, it would be great if a fix could be added to the next release. Thanks in advance, Oliver Powell
