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]

Reply via email to