vgritsenko 2002/06/14 08:24:33 Modified: src/java/org/apache/cocoon/components/pipeline/impl CachingProcessingPipeline.java Log: Fix NPE in the pipeline: move calculation of validity up and out of if() Revision Changes Path 1.26 +58 -53 xml-cocoon2/src/java/org/apache/cocoon/components/pipeline/impl/CachingProcessingPipeline.java Index: CachingProcessingPipeline.java =================================================================== RCS file: /home/cvs/xml-cocoon2/src/java/org/apache/cocoon/components/pipeline/impl/CachingProcessingPipeline.java,v retrieving revision 1.25 retrieving revision 1.26 diff -u -r1.25 -r1.26 --- CachingProcessingPipeline.java 14 Jun 2002 07:09:12 -0000 1.25 +++ CachingProcessingPipeline.java 14 Jun 2002 15:24:33 -0000 1.26 @@ -50,7 +50,6 @@ */ package org.apache.cocoon.components.pipeline.impl; -import org.apache.avalon.excalibur.pool.Recyclable; import org.apache.avalon.framework.activity.Disposable; import org.apache.avalon.framework.component.ComponentException; import org.apache.avalon.framework.component.ComponentManager; @@ -70,22 +69,17 @@ import org.apache.cocoon.components.sax.XMLDeserializer; import org.apache.cocoon.components.sax.XMLSerializer; import org.apache.cocoon.components.sax.XMLTeePipe; -import org.apache.cocoon.components.saxconnector.SAXConnector; import org.apache.cocoon.environment.Environment; import org.apache.cocoon.transformation.Transformer; import org.apache.cocoon.xml.XMLConsumer; import org.apache.cocoon.xml.XMLProducer; import org.apache.excalibur.source.SourceValidity; -import org.xml.sax.SAXException; import java.io.ByteArrayOutputStream; -import java.io.IOException; import java.io.OutputStream; import java.io.Serializable; import java.net.SocketException; import java.util.ArrayList; -import java.util.HashMap; import java.util.Iterator; -import java.util.Map; /** * The CachingProcessingPipeline @@ -292,6 +286,10 @@ throws ProcessingException { super.setupPipeline( environment ); + // + // FIXME (VG): This method is 280 lines long. It *must* be refactored... + // + PipelineCacheKey cachedPipelineKey = null; PipelineCacheKey processingPipelineKey = null; @@ -311,7 +309,7 @@ // or Cacheable (deprecated). The returned keys are chained together // to build a unique key of the request - // is the generator cacheable? + // Is the generator cacheable? if (super.generator instanceof CacheableProcessingComponent) { key = ((CacheableProcessingComponent)super.generator).generateKey(); generatorIsCacheableProcessingComponent = true; @@ -326,18 +324,15 @@ key) ); - // now testing transformers + // Now testing transformers final int transformerSize = super.transformers.size(); - boolean continueTest = true; - - while (this.firstNotCacheableTransformerIndex < transformerSize - && continueTest) { + while (firstNotCacheableTransformerIndex < transformerSize) { final Transformer trans = - (Transformer)super.transformers.get(this.firstNotCacheableTransformerIndex); + (Transformer)super.transformers.get(firstNotCacheableTransformerIndex); key = null; if (trans instanceof CacheableProcessingComponent) { key = ((CacheableProcessingComponent)trans).generateKey(); - transformerIsCacheableProcessingComponent[this.firstNotCacheableTransformerIndex] = true; + transformerIsCacheableProcessingComponent[firstNotCacheableTransformerIndex] = true; } else if (trans instanceof Cacheable) { key = new Long(((Cacheable)trans).generateKey()); } @@ -348,10 +343,11 @@ ); this.firstNotCacheableTransformerIndex++; } else { - continueTest = false; + break; } } - // all transformers are cacheable => pipeline is cacheable, test serializer + + // All transformers are cacheable => pipeline is cacheable, test serializer if (this.firstNotCacheableTransformerIndex == transformerSize && super.serializer == this.lastConsumer) { @@ -373,7 +369,7 @@ } } - // now, we have a key representing all cacheable components up to the + // Now, we have a key representing all cacheable components up to the // first non cacheable if (processingPipelineKey != null) { cachedPipelineKey = processingPipelineKey.copy(); @@ -386,7 +382,7 @@ finished = true; CachedResponse response = this.cache.get( cachedPipelineKey ); - // now test validity + // Now test validity of the response if (response != null) { boolean responseIsValid = true; boolean responseIsUsable = true; @@ -397,34 +393,39 @@ SourceValidity[] validities = response.getValidityObjects(); int i = 0; while (responseIsValid && i < validities.length) { - int valid = validities[i].isValid(); - boolean isValid = false; + // Obtain new validity of the component SourceValidity validity = null; - if ( valid == 0) { - if (i == 0) { - // test generator - if (generatorIsCacheableProcessingComponent) { - validity = ((CacheableProcessingComponent)super.generator).generateValidity(); - } else { - validity = CacheValidityToSourceValidity.createValidity(((Cacheable)super.generator).generateValidity()); - } - } else if (i <= this.firstProcessedTransformerIndex) { - // test transformer - final Transformer trans = - (Transformer)super.transformers.get(i-1); - if (transformerIsCacheableProcessingComponent[i-1]) { - validity = ((CacheableProcessingComponent)trans).generateValidity(); - } else { - validity = CacheValidityToSourceValidity.createValidity(((Cacheable)trans).generateValidity()); - } + if (i == 0) { + // test generator + if (generatorIsCacheableProcessingComponent) { + validity = ((CacheableProcessingComponent)super.generator).generateValidity(); } else { - // test serializer - if (serializerIsCacheableProcessingComponent) { - validity = ((CacheableProcessingComponent)super.serializer).generateValidity(); - } else { - validity = CacheValidityToSourceValidity.createValidity(((Cacheable)super.serializer).generateValidity()); - } + validity = CacheValidityToSourceValidity.createValidity(((Cacheable)super.generator).generateValidity()); + } + } else if (i <= this.firstProcessedTransformerIndex) { + // test transformer + final Transformer trans = (Transformer)super.transformers.get(i-1); + if (transformerIsCacheableProcessingComponent[i-1]) { + validity = ((CacheableProcessingComponent)trans).generateValidity(); + } else { + validity = CacheValidityToSourceValidity.createValidity(((Cacheable)trans).generateValidity()); } + } else { + // test serializer + if (serializerIsCacheableProcessingComponent) { + validity = ((CacheableProcessingComponent)super.serializer).generateValidity(); + } else { + validity = CacheValidityToSourceValidity.createValidity(((Cacheable)super.serializer).generateValidity()); + } + } + + int valid = -1; + // It can be null. After update. + if (validities[i] != null) { + valid = validities[i].isValid(); + } + boolean isValid = false; + if ( valid == 0 ) { if (validity != null) { isValid = validities[i].isValid( validity ); } @@ -433,7 +434,7 @@ } if ( !isValid ) { responseIsValid = false; - // update validity + // Update validity. Validity can be null. validities[i] = validity; if (validity == null) responseIsUsable = false; } else { @@ -456,8 +457,8 @@ this.cachedResponse = null; if (!responseIsUsable) { - // we could compare, because we got no - // validity object, so shorten pipeline key + // We couldn't compare, because we got no validity + // object, so shorten pipeline key and try again if (i > 0) { int deleteCount = validities.length - i; if (i > 0 && i <= firstNotCacheableTransformerIndex + 1) { @@ -475,7 +476,7 @@ // the entry is invalid, remove it this.cache.remove( cachedPipelineKey ); } - // try a shorter key + // Try a shorter key if (i > 0) { cachedPipelineKey.removeLastKey(); this.firstProcessedTransformerIndex--; @@ -500,21 +501,25 @@ } } + // FIXME (VG): This part below incorrectly caches only part of pipeline when + // it was detected change in one of the pipeline components. It should cache + // whole pipeline (if it is cacheable), even if current response was created + // from partial cached response. + // now generate validity objects for the new response if (processingPipelineKey != null) { if (cachedPipelineKey == null || cachedPipelineKey.size() < processingPipelineKey.size()) { this.pipelineCacheKey = processingPipelineKey; this.pipelineValidityObjects = new SourceValidity[processingPipelineKey.size()]; - int start = 0; + int i = 0; if (cachedPipelineKey != null) { - start = cachedPipelineKey.size(); - for(int i=0; i<start;i++) { - this.pipelineValidityObjects[i] = cachedValidityObjects[i]; + i = cachedPipelineKey.size(); + for(int j = 0; j < i; j++) { + this.pipelineValidityObjects[j] = cachedValidityObjects[j]; } } int len = this.pipelineValidityObjects.length; - int i = start; while (i < len) { final SourceValidity validity; if (i == 0) {
---------------------------------------------------------------------- In case of troubles, e-mail: [EMAIL PROTECTED] To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]