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]