No worries. I don't see any issues here.
-Stanton
From: Henry Saputra <henry.sapu...@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, <hsapu...@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;
> }
>
> /**
>
>
