You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Henry Saputra <he...@gmail.com> on 2012/04/12 18:51:55 UTC

Accidental commit for svn commit: r1325372 - in /shindig/trunk: features/src/main/javascript/features/container.site.gadget/ features/src/main/javascript/features/shindig.uri/ features/src/test/javascript/features/container/ java/gadgets/src/main/jav

Uff super mea culpa from me =(
I accidentally checked in some changes from different reviews:

Checkin for this comment ("Throw SpecRetrievalFailedException
exception instead of generic GadgetException to avoid cache at the
AbstractSpecFactory level because it could be cache in the
RequestPipeline with some logic to set the right cache content) :
    shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java

https://reviews.apache.org/r/4703/   => Add rpctoken to common
container render flow
   shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
   shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js
   shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js

https://reviews.apache.org/r/4706/  => Refactor DefaultRequestPipeline
for easy of extension (need comments per Stanton comment, will add to
them in next commit. Sorry)
 shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java


Please let me know if anyone object to any of the patch then I could revert

- Henry

On Thu, Apr 12, 2012 at 9:46 AM,  <hs...@apache.org> wrote:
> Author: hsaputra
> Date: Thu Apr 12 16:46:04 2012
> New Revision: 1325372
>
> URL: http://svn.apache.org/viewvc?rev=1325372&view=rev
> Log:
> Throw SpecRetrievalFailedException exception instead of generic GadgetException to avoid cache at the AbstractSpecFactory level because it could be cache in the RequestPipeline with some logic to set the right cache content.
>
> Modified:
>    shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
>    shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js
>    shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js
>    shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java
>    shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
>
> Modified: shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
> URL: http://svn.apache.org/viewvc/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js?rev=1325372&r1=1325371&r2=1325372&view=diff
> ==============================================================================
> --- shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js (original)
> +++ shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js Thu Apr 12 16:46:04 2012
> @@ -261,6 +261,12 @@ osapi.container.GadgetHolder.prototype.g
>     uri.setFP('view-params', gadgetParamText);
>   }
>
> +  // add rpctoken fragment to support flash transport if not in the uri
> +  if(typeof(uri.getFP('rpctoken')) === 'undefined' ) {
> +    var rpcToken = (0x7FFFFFFF * Math.random()) | 0;
> +    uri.setFP('rpctoken', rpcToken);
> +  }
> +
>   return uri.toString();
>  };
>
>
> Modified: shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js
> URL: http://svn.apache.org/viewvc/shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js?rev=1325372&r1=1325371&r2=1325372&view=diff
> ==============================================================================
> --- shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js (original)
> +++ shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js Thu Apr 12 16:46:04 2012
> @@ -149,6 +149,12 @@ shindig.uri = (function() {
>
>     function parseParams(str) {
>       var params = [];
> +      // When the string is empty, split returns an array containing one empty string,
> +      // rather than an empty array.
> +      if(str === "") {
> +        return params;
> +      }
> +
>       var pairs = str.split('&');
>       for (var i = 0, j = pairs.length; i < j; ++i) {
>         var kv = pairs[i].split('=');
>
> Modified: shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js
> URL: http://svn.apache.org/viewvc/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js?rev=1325372&r1=1325371&r2=1325372&view=diff
> ==============================================================================
> --- shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js (original)
> +++ shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js Thu Apr 12 16:46:04 2012
> @@ -58,7 +58,7 @@ GadgetHolderTest.prototype.testNew = fun
>  GadgetHolderTest.prototype.testRenderWithoutRenderParams = function() {
>   var element = {};
>   var gadgetInfo = {
> -      'iframeUrls' : {'default' : 'http://shindig/gadgets/ifr?url=gadget.xml'},
> +      'iframeUrls' : {'default' : 'http://shindig/gadgets/ifr?url=gadget.xml#rpctoken=1234'},
>       'url' : 'gadget.xml'
>   };
>   this.setupGadgetsRpcSetupReceiver();
> @@ -79,14 +79,14 @@ GadgetHolderTest.prototype.testRenderWit
>       ' id="__gadget_123"' +
>       ' name="__gadget_123"' +
>       ' src="http://shindig/gadgets/ifr?url=gadget.xml&debug=0&nocache=0&testmode=0' +
> -          '&view=default&parent=http%3A//container.com&mid=0"' +
> +          '&view=default&parent=http%3A//container.com&mid=0#rpctoken=1234"' +
>       ' ></iframe>',
>       element.innerHTML);
>  };
>
>  GadgetHolderTest.prototype.testRenderWithRenderRequests = function() {
>   var gadgetInfo = {
> -      'iframeUrls' : {'default' : 'http://shindig/gadgets/ifr?url=gadget.xml'},
> +      'iframeUrls' : {'default' : 'http://shindig/gadgets/ifr?url=gadget.xml#rpctoken=1234'},
>       'url' : 'gadget.xml'
>   };
>   var renderParams = {
> @@ -120,7 +120,7 @@ GadgetHolderTest.prototype.testRenderWit
>       ' width="222"' +
>       ' name="__gadget_123"' +
>       ' src="http://shindig/gadgets/ifr?url=gadget.xml&debug=1&nocache=1&testmode=1' +
> -          '&view=default&libs=caja&caja=1&parent=http%3A//container.com&mid=0"' +
> +          '&view=default&libs=caja&caja=1&parent=http%3A//container.com&mid=0#rpctoken=1234"' +
>       ' ></iframe>',
>       element.innerHTML);
>  };
>
> Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java
> URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java?rev=1325372&r1=1325371&r2=1325372&view=diff
> ==============================================================================
> --- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java (original)
> +++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java Thu Apr 12 16:46:04 2012
> @@ -136,10 +136,7 @@ public abstract class AbstractSpecFactor
>         // Convert external "internal error" to gateway error:
>         retcode = HttpResponse.SC_BAD_GATEWAY;
>       }
> -      throw new GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT,
> -                                "Unable to retrieve spec for " + query.specUri + ". HTTP error " +
> -                                response.getHttpStatusCode(),
> -                                retcode);
> +      throw new SpecRetrievalFailedException(query.specUri, retcode);
>     }
>
>     try {
> @@ -217,6 +214,10 @@ public abstract class AbstractSpecFactor
>       try {
>         T newSpec = fetchFromNetwork(query);
>         cache.addElement(query.specUri, newSpec, refresh);
> +      } catch (SpecRetrievalFailedException se) {
> +        if (LOG.isLoggable(Level.INFO)) {
> +          LOG.logp(Level.INFO, classname, "SpecUpdater", MessageKeys.UPDATE_SPEC_FAILURE_APPLY_NEG_CACHE, new Object[] {query.specUri});
> +        }
>       } catch (GadgetException e) {
>         if (old != null) {
>           if (LOG.isLoggable(Level.INFO)) {
>
> Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
> URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java?rev=1325372&r1=1325371&r2=1325372&view=diff
> ==============================================================================
> --- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java (original)
> +++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java Thu Apr 12 16:46:04 2012
> @@ -78,30 +78,60 @@ public class DefaultRequestPipeline impl
>
>   public HttpResponse execute(HttpRequest request) throws GadgetException {
>     normalizeProtocol(request);
> +
> +    HttpResponse cachedResponse = checkCachedResponse(request);
> +
>     HttpResponse invalidatedResponse = null;
>     HttpResponse staleResponse = null;
>
> -    if (!request.getIgnoreCache()) {
> -      HttpResponse cachedResponse = httpCache.getResponse(request);
> -      // Note that we don't remove invalidated entries from the cache as we want them to be
> -      // available in the event of a backend fetch failure.
> -      // Note that strict no-cache entries are dummy responses and should not be used.
> -      if (cachedResponse != null && !cachedResponse.isStrictNoCache()) {
> -        if (!cachedResponse.isStale()) {
> -          if(invalidationService.isValid(request, cachedResponse)) {
> -            return cachedResponse;
> -          } else {
> -            invalidatedResponse = cachedResponse;
> -          }
> +    // Note that we don't remove invalidated entries from the cache as we want them to be
> +    // available in the event of a backend fetch failure.
> +    // Note that strict no-cache entries are dummy responses and should not be used.
> +    if (cachedResponse != null && !cachedResponse.isStrictNoCache()) {
> +      if (!cachedResponse.isStale()) {
> +        if (invalidationService.isValid(request, cachedResponse)) {
> +          return cachedResponse;
>         } else {
> -          if (!cachedResponse.isError()) {
> -            // Remember good but stale cached response, to be served if server unavailable
> -            staleResponse = cachedResponse;
> -          }
> +          invalidatedResponse = cachedResponse;
> +        }
> +      } else {
> +        if (!cachedResponse.isError()) {
> +          // Remember good but stale cached response, to be served if server unavailable
> +          staleResponse = cachedResponse;
>         }
>       }
>     }
> +    HttpResponse fetchedResponse = fetchResponse(request);
> +    fetchedResponse = fixFetchedResponse(request, fetchedResponse, invalidatedResponse, staleResponse);
> +    return fetchedResponse;
> +  }
> +
> +  protected void normalizeProtocol(HttpRequest request) throws GadgetException {
> +    // Normalize the protocol part of the URI
> +    if (request.getUri().getScheme()== null) {
> +      throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
> +          "Url " + request.getUri().toString() + " does not include scheme",
> +          HttpResponse.SC_BAD_REQUEST);
> +    } else if (!"http".equals(request.getUri().getScheme()) &&
> +        !"https".equals(request.getUri().getScheme())) {
> +      throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
> +          "Invalid request url scheme in url: " + Utf8UrlCoder.encode(request.getUri().toString()) +
> +            "; only \"http\" and \"https\" supported.",
> +            HttpResponse.SC_BAD_REQUEST);
> +    }
> +  }
> +
> +  protected HttpResponse checkCachedResponse(HttpRequest request) {
> +    HttpResponse cachedResponse;
> +    if (!request.getIgnoreCache()) {
> +      cachedResponse = httpCache.getResponse(request);
> +    } else {
> +      cachedResponse = null;
> +    }
> +    return cachedResponse;
> +  }
>
> +  protected HttpResponse fetchResponse(HttpRequest request) throws GadgetException {
>     HttpResponse fetchedResponse;
>     switch (request.getAuthType()) {
>       case NONE:
> @@ -117,7 +147,11 @@ public class DefaultRequestPipeline impl
>       default:
>         return HttpResponse.error();
>     }
> +    return fetchedResponse;
> +  }
>
> +  protected HttpResponse fixFetchedResponse(HttpRequest request, HttpResponse fetchedResponse,
> +      HttpResponse invalidatedResponse, HttpResponse staleResponse) throws GadgetException {
>     if (fetchedResponse.isError() && invalidatedResponse != null) {
>       // Use the invalidated cached response if it is not stale. We don't update its
>       // mark so it remains invalidated
> @@ -142,6 +176,14 @@ public class DefaultRequestPipeline impl
>
>     // Set response hash value in metadata (used for url versioning)
>     fetchedResponse = HttpResponseMetadataHelper.updateHash(fetchedResponse, metadataHelper);
> +
> +    // cache the fetched response if possible
> +    fetchedResponse = cacheFetchedResponse(request, fetchedResponse);
> +
> +    return fetchedResponse;
> +  }
> +
> +  protected HttpResponse cacheFetchedResponse(HttpRequest request, HttpResponse fetchedResponse) {
>     if (!request.getIgnoreCache()) {
>       // Mark the response with invalidation information prior to caching
>       if (fetchedResponse.getCacheTtl() > 0) {
> @@ -149,22 +191,8 @@ public class DefaultRequestPipeline impl
>       }
>       httpCache.addResponse(request, fetchedResponse);
>     }
> -    return fetchedResponse;
> -  }
>
> -  protected void normalizeProtocol(HttpRequest request) throws GadgetException {
> -    // Normalize the protocol part of the URI
> -    if (request.getUri().getScheme()== null) {
> -      throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
> -          "Url " + request.getUri().toString() + " does not include scheme",
> -          HttpResponse.SC_BAD_REQUEST);
> -    } else if (!"http".equals(request.getUri().getScheme()) &&
> -        !"https".equals(request.getUri().getScheme())) {
> -      throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
> -          "Invalid request url scheme in url: " + Utf8UrlCoder.encode(request.getUri().toString()) +
> -            "; only \"http\" and \"https\" supported.",
> -            HttpResponse.SC_BAD_REQUEST);
> -    }
> +    return fetchedResponse;
>   }
>
>   /**
>
>

Re: Accidental commit for svn commit: r1325372 - in /shindig/trunk: features/src/main/javascript/features/container.site.gadget/ features/src/main/javascript/features/shindig.uri/ features/src/test/javascript/features/container/ java/gadgets/src/main/jav

Posted by Stanton Sievers <ss...@us.ibm.com>.
No worries.  I don't see any issues here.

-Stanton



From:   Henry Saputra <he...@gmail.com>
To:     dev@shindig.apache.org, 
Date:   04/12/2012 12:51
Subject:        Accidental commit for svn commit: r1325372 - in 
/shindig/trunk: 
features/src/main/javascript/features/container.site.gadget/ 
features/src/main/javascript/features/shindig.uri/ 
features/src/test/javascript/features/container/ java/gadgets/src/main/jav



Uff super mea culpa from me =(
I accidentally checked in some changes from different reviews:

Checkin for this comment ("Throw SpecRetrievalFailedException
exception instead of generic GadgetException to avoid cache at the
AbstractSpecFactory level because it could be cache in the
RequestPipeline with some logic to set the right cache content) :
 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java

https://reviews.apache.org/r/4703/   => Add rpctoken to common
container render flow
 
shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
   shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js
 
shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js

https://reviews.apache.org/r/4706/  => Refactor DefaultRequestPipeline
for easy of extension (need comments per Stanton comment, will add to
them in next commit. Sorry)
 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java


Please let me know if anyone object to any of the patch then I could 
revert

- Henry

On Thu, Apr 12, 2012 at 9:46 AM,  <hs...@apache.org> wrote:
> Author: hsaputra
> Date: Thu Apr 12 16:46:04 2012
> New Revision: 1325372
>
> URL: http://svn.apache.org/viewvc?rev=1325372&view=rev
> Log:
> Throw SpecRetrievalFailedException exception instead of generic 
GadgetException to avoid cache at the AbstractSpecFactory level because it 
could be cache in the RequestPipeline with some logic to set the right 
cache content.
>
> Modified:
>   
 shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
>   
 shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js
>   
 shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js
>   
 shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java
>   
 shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
>
> Modified: 
shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
> URL: 
http://svn.apache.org/viewvc/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js?rev=1325372&r1=1325371&r2=1325372&view=diff

> 
==============================================================================
> --- 
shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 
(original)
> +++ 
shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 
Thu Apr 12 16:46:04 2012
> @@ -261,6 +261,12 @@ osapi.container.GadgetHolder.prototype.g
>     uri.setFP('view-params', gadgetParamText);
>   }
>
> +  // add rpctoken fragment to support flash transport if not in the uri
> +  if(typeof(uri.getFP('rpctoken')) === 'undefined' ) {
> +    var rpcToken = (0x7FFFFFFF * Math.random()) | 0;
> +    uri.setFP('rpctoken', rpcToken);
> +  }
> +
>   return uri.toString();
>  };
>
>
> Modified: 
shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js
> URL: 
http://svn.apache.org/viewvc/shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js?rev=1325372&r1=1325371&r2=1325372&view=diff

> 
==============================================================================
> --- 
shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js 
(original)
> +++ 
shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js Thu 
Apr 12 16:46:04 2012
> @@ -149,6 +149,12 @@ shindig.uri = (function() {
>
>     function parseParams(str) {
>       var params = [];
> +      // When the string is empty, split returns an array containing 
one empty string,
> +      // rather than an empty array.
> +      if(str === "") {
> +        return params;
> +      }
> +
>       var pairs = str.split('&');
>       for (var i = 0, j = pairs.length; i < j; ++i) {
>         var kv = pairs[i].split('=');
>
> Modified: 
shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js
> URL: 
http://svn.apache.org/viewvc/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js?rev=1325372&r1=1325371&r2=1325372&view=diff

> 
==============================================================================
> --- 
shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 
(original)
> +++ 
shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 
Thu Apr 12 16:46:04 2012
> @@ -58,7 +58,7 @@ GadgetHolderTest.prototype.testNew = fun
>  GadgetHolderTest.prototype.testRenderWithoutRenderParams = function() {
>   var element = {};
>   var gadgetInfo = {
> -      'iframeUrls' : {'default' : 
'http://shindig/gadgets/ifr?url=gadget.xml'},
> +      'iframeUrls' : {'default' : 
'http://shindig/gadgets/ifr?url=gadget.xml#rpctoken=1234'},
>       'url' : 'gadget.xml'
>   };
>   this.setupGadgetsRpcSetupReceiver();
> @@ -79,14 +79,14 @@ GadgetHolderTest.prototype.testRenderWit
>       ' id="__gadget_123"' +
>       ' name="__gadget_123"' +
>       ' src="
http://shindig/gadgets/ifr?url=gadget.xml&debug=0&nocache=0&testmode=0' +
> -          '&view=default&parent=http%3A//container.com&mid=0"' +
> +         
 '&view=default&parent=http%3A//container.com&mid=0#rpctoken=1234"' +
>       ' ></iframe>',
>       element.innerHTML);
>  };
>
>  GadgetHolderTest.prototype.testRenderWithRenderRequests = function() {
>   var gadgetInfo = {
> -      'iframeUrls' : {'default' : 
'http://shindig/gadgets/ifr?url=gadget.xml'},
> +      'iframeUrls' : {'default' : 
'http://shindig/gadgets/ifr?url=gadget.xml#rpctoken=1234'},
>       'url' : 'gadget.xml'
>   };
>   var renderParams = {
> @@ -120,7 +120,7 @@ GadgetHolderTest.prototype.testRenderWit
>       ' width="222"' +
>       ' name="__gadget_123"' +
>       ' src="
http://shindig/gadgets/ifr?url=gadget.xml&debug=1&nocache=1&testmode=1' +
> -         
 '&view=default&libs=caja&caja=1&parent=http%3A//container.com&mid=0"' +
> +         
 '&view=default&libs=caja&caja=1&parent=http%3A//container.com&mid=0#rpctoken=1234"' 
+
>       ' ></iframe>',
>       element.innerHTML);
>  };
>
> Modified: 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java
> URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java?rev=1325372&r1=1325371&r2=1325372&view=diff

> 
==============================================================================
> --- 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java 
(original)
> +++ 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java 
Thu Apr 12 16:46:04 2012
> @@ -136,10 +136,7 @@ public abstract class AbstractSpecFactor
>         // Convert external "internal error" to gateway error:
>         retcode = HttpResponse.SC_BAD_GATEWAY;
>       }
> -      throw new 
GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT,
> -                                "Unable to retrieve spec for " + 
query.specUri + ". HTTP error " +
> -                                response.getHttpStatusCode(),
> -                                retcode);
> +      throw new SpecRetrievalFailedException(query.specUri, retcode);
>     }
>
>     try {
> @@ -217,6 +214,10 @@ public abstract class AbstractSpecFactor
>       try {
>         T newSpec = fetchFromNetwork(query);
>         cache.addElement(query.specUri, newSpec, refresh);
> +      } catch (SpecRetrievalFailedException se) {
> +        if (LOG.isLoggable(Level.INFO)) {
> +          LOG.logp(Level.INFO, classname, "SpecUpdater", 
MessageKeys.UPDATE_SPEC_FAILURE_APPLY_NEG_CACHE, new Object[] 
{query.specUri});
> +        }
>       } catch (GadgetException e) {
>         if (old != null) {
>           if (LOG.isLoggable(Level.INFO)) {
>
> Modified: 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
> URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java?rev=1325372&r1=1325371&r2=1325372&view=diff

> 
==============================================================================
> --- 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 
(original)
> +++ 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 
Thu Apr 12 16:46:04 2012
> @@ -78,30 +78,60 @@ public class DefaultRequestPipeline impl
>
>   public HttpResponse execute(HttpRequest request) throws 
GadgetException {
>     normalizeProtocol(request);
> +
> +    HttpResponse cachedResponse = checkCachedResponse(request);
> +
>     HttpResponse invalidatedResponse = null;
>     HttpResponse staleResponse = null;
>
> -    if (!request.getIgnoreCache()) {
> -      HttpResponse cachedResponse = httpCache.getResponse(request);
> -      // Note that we don't remove invalidated entries from the cache 
as we want them to be
> -      // available in the event of a backend fetch failure.
> -      // Note that strict no-cache entries are dummy responses and 
should not be used.
> -      if (cachedResponse != null && !cachedResponse.isStrictNoCache()) 
{
> -        if (!cachedResponse.isStale()) {
> -          if(invalidationService.isValid(request, cachedResponse)) {
> -            return cachedResponse;
> -          } else {
> -            invalidatedResponse = cachedResponse;
> -          }
> +    // Note that we don't remove invalidated entries from the cache as 
we want them to be
> +    // available in the event of a backend fetch failure.
> +    // Note that strict no-cache entries are dummy responses and should 
not be used.
> +    if (cachedResponse != null && !cachedResponse.isStrictNoCache()) {
> +      if (!cachedResponse.isStale()) {
> +        if (invalidationService.isValid(request, cachedResponse)) {
> +          return cachedResponse;
>         } else {
> -          if (!cachedResponse.isError()) {
> -            // Remember good but stale cached response, to be served if 
server unavailable
> -            staleResponse = cachedResponse;
> -          }
> +          invalidatedResponse = cachedResponse;
> +        }
> +      } else {
> +        if (!cachedResponse.isError()) {
> +          // Remember good but stale cached response, to be served if 
server unavailable
> +          staleResponse = cachedResponse;
>         }
>       }
>     }
> +    HttpResponse fetchedResponse = fetchResponse(request);
> +    fetchedResponse = fixFetchedResponse(request, fetchedResponse, 
invalidatedResponse, staleResponse);
> +    return fetchedResponse;
> +  }
> +
> +  protected void normalizeProtocol(HttpRequest request) throws 
GadgetException {
> +    // Normalize the protocol part of the URI
> +    if (request.getUri().getScheme()== null) {
> +      throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
> +          "Url " + request.getUri().toString() + " does not include 
scheme",
> +          HttpResponse.SC_BAD_REQUEST);
> +    } else if (!"http".equals(request.getUri().getScheme()) &&
> +        !"https".equals(request.getUri().getScheme())) {
> +      throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
> +          "Invalid request url scheme in url: " + 
Utf8UrlCoder.encode(request.getUri().toString()) +
> +            "; only \"http\" and \"https\" supported.",
> +            HttpResponse.SC_BAD_REQUEST);
> +    }
> +  }
> +
> +  protected HttpResponse checkCachedResponse(HttpRequest request) {
> +    HttpResponse cachedResponse;
> +    if (!request.getIgnoreCache()) {
> +      cachedResponse = httpCache.getResponse(request);
> +    } else {
> +      cachedResponse = null;
> +    }
> +    return cachedResponse;
> +  }
>
> +  protected HttpResponse fetchResponse(HttpRequest request) throws 
GadgetException {
>     HttpResponse fetchedResponse;
>     switch (request.getAuthType()) {
>       case NONE:
> @@ -117,7 +147,11 @@ public class DefaultRequestPipeline impl
>       default:
>         return HttpResponse.error();
>     }
> +    return fetchedResponse;
> +  }
>
> +  protected HttpResponse fixFetchedResponse(HttpRequest request, 
HttpResponse fetchedResponse,
> +      HttpResponse invalidatedResponse, HttpResponse staleResponse) 
throws GadgetException {
>     if (fetchedResponse.isError() && invalidatedResponse != null) {
>       // Use the invalidated cached response if it is not stale. We 
don't update its
>       // mark so it remains invalidated
> @@ -142,6 +176,14 @@ public class DefaultRequestPipeline impl
>
>     // Set response hash value in metadata (used for url versioning)
>     fetchedResponse = 
HttpResponseMetadataHelper.updateHash(fetchedResponse, metadataHelper);
> +
> +    // cache the fetched response if possible
> +    fetchedResponse = cacheFetchedResponse(request, fetchedResponse);
> +
> +    return fetchedResponse;
> +  }
> +
> +  protected HttpResponse cacheFetchedResponse(HttpRequest request, 
HttpResponse fetchedResponse) {
>     if (!request.getIgnoreCache()) {
>       // Mark the response with invalidation information prior to 
caching
>       if (fetchedResponse.getCacheTtl() > 0) {
> @@ -149,22 +191,8 @@ public class DefaultRequestPipeline impl
>       }
>       httpCache.addResponse(request, fetchedResponse);
>     }
> -    return fetchedResponse;
> -  }
>
> -  protected void normalizeProtocol(HttpRequest request) throws 
GadgetException {
> -    // Normalize the protocol part of the URI
> -    if (request.getUri().getScheme()== null) {
> -      throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
> -          "Url " + request.getUri().toString() + " does not include 
scheme",
> -          HttpResponse.SC_BAD_REQUEST);
> -    } else if (!"http".equals(request.getUri().getScheme()) &&
> -        !"https".equals(request.getUri().getScheme())) {
> -      throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
> -          "Invalid request url scheme in url: " + 
Utf8UrlCoder.encode(request.getUri().toString()) +
> -            "; only \"http\" and \"https\" supported.",
> -            HttpResponse.SC_BAD_REQUEST);
> -    }
> +    return fetchedResponse;
>   }
>
>   /**
>
>