On Wed, May 4, 2011 at 4:35 PM, Henry Saputra <[email protected]>wrote:
> Mike, > > This format change could break custom URLs to get external features > for old container and gadgets. > > &loaded= is a new and optional functionality, and only affects JS dynamic compilation. To my knowledge, no-one is using this functionality (as it is broken, in certain cases). The new format (the presence of !load1:load in the path) is also optional, so all existing JS URLs should continue to work. I'd like to get this new syntax right sooner (as discussed internally as preferred), then having to support two modes. Are you aware of any clients that may get tripped over this? What are examples of custom URLs being used now? Would you or John will create a doc or wiki to describe the changes > for JavaScript serving mechanism? > Is there a wiki that I can piggyback on? > > - Henry > > On Wed, May 4, 2011 at 4:22 PM, <[email protected]> wrote: > > Author: mhermanto > > Date: Wed May 4 23:22:32 2011 > > New Revision: 1099636 > > > > URL: http://svn.apache.org/viewvc?rev=1099636&view=rev > > Log: > > Change loaded features syntax. > > from: /gadgets/js/x:y.js?loaded=a:b > > to: /gadgets/js/x:y!a:b.js > > > > http://codereview.appspot.com/4483043/ > > > > Modified: > > > > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java > > > > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriCommon.java > > > > shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultJsUriManagerTest.java > > > > Modified: > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java > > URL: > http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java?rev=1099636&r1=1099635&r2=1099636&view=diff > > > ============================================================================== > > --- > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java > (original) > > +++ > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java > Wed May 4 23:22:32 2011 > > @@ -67,13 +67,14 @@ public class DefaultJsUriManager impleme > > jsPath.append('/'); > > } > > jsPath.append(addJsLibs(ctx.getLibs())); > > - jsPath.append(JS_SUFFIX); > > - uri.setPath(jsPath.toString()); > > - > > + > > // Add the list of already-loaded libs > > if (!ctx.getLoadedLibs().isEmpty()) { > > - uri.addQueryParameter(Param.LOADED_LIBS.getKey(), > addJsLibs(ctx.getLoadedLibs())); > > + jsPath.append("!").append(addJsLibs(ctx.getLoadedLibs())); > > } > > + > > + jsPath.append(JS_SUFFIX); > > + uri.setPath(jsPath.toString()); > > > > // Standard container param, as JS may be container-specific. > > uri.addQueryParameter(Param.CONTAINER.getKey(), container); > > @@ -173,12 +174,9 @@ public class DefaultJsUriManager impleme > > path = path.substring(1); > > } > > > > - Collection<String> libs = getJsLibs(path); > > - String haveParam = > uri.getQueryParameter(Param.LOADED_LIBS.getKey()); > > - if (haveParam == null) { > > - haveParam = ""; > > - } > > - Collection<String> have = getJsLibs(haveParam); > > + String[] splits = path.split("!"); > > + Collection<String> libs = getJsLibs(splits.length >= 1 ? splits[0] : > ""); > > + Collection<String> have = getJsLibs(splits.length >= 2 ? splits[1] : > ""); > > UriStatus status = UriStatus.VALID_UNVERSIONED; > > String version = uri.getQueryParameter(Param.VERSION.getKey()); > > if (version != null && versioner != null) { > > > > Modified: > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriCommon.java > > URL: > http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriCommon.java?rev=1099636&r1=1099635&r2=1099636&view=diff > > > ============================================================================== > > --- > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriCommon.java > (original) > > +++ > shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriCommon.java > Wed May 4 23:22:32 2011 > > @@ -50,7 +50,6 @@ public interface UriCommon { > > COMPILE_MODE("compile"), > > JSLOAD("jsload"), > > ONLOAD("onload"), > > - LOADED_LIBS("loaded"), > > NO_HINT("nohint"), > > REPOSITORY_ID("r"), > > > > > > Modified: > shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultJsUriManagerTest.java > > URL: > http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultJsUriManagerTest.java?rev=1099636&r1=1099635&r2=1099636&view=diff > > > ============================================================================== > > --- > shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultJsUriManagerTest.java > (original) > > +++ > shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultJsUriManagerTest.java > Wed May 4 23:22:32 2011 > > @@ -192,8 +192,8 @@ public class DefaultJsUriManagerTest { > > assertFalse(manager.hadError()); > > assertEquals("http", jsUri.getScheme()); > > assertEquals("www.js.org", jsUri.getAuthority()); > > - assertEquals("/gadgets/js/" + addJsLibs(extern) + JS_SUFFIX, > jsUri.getPath()); > > - assertEquals("another:onemore", > jsUri.getQueryParameter(Param.LOADED_LIBS.getKey())); > > + assertEquals("/gadgets/js/" + addJsLibs(extern) + "!" + > addJsLibs(loaded) + > > + JS_SUFFIX, jsUri.getPath()); > > assertEquals(CONTAINER, > jsUri.getQueryParameter(Param.CONTAINER.getKey())); > > assertEquals("0", jsUri.getQueryParameter(Param.NO_CACHE.getKey())); > > assertEquals("0", jsUri.getQueryParameter(Param.DEBUG.getKey())); > > > > > > > > > > -- > Thanks, > Henry >
