On Wed, May 4, 2011 at 5:02 PM, Henry Saputra <[email protected]>wrote:
> But will this URL format will be used when getting features from JSServlet? > Yes. What I meant by custom URL is when proxy gadget is trying to generate > <script> tag to access Shindig features. > I totally see this use-case, but not with loaded= functionality. > I could only think of one case of this is with recursive gadget where > a gadget open iframe to include dynamic content that include script > tag that access features from Shindig. So the script tag is not > generated by Shindig but by the proxy gadget server. > By default in Shindig, dynamic compilation is off, and hence, ignores loaded=. FMI, what is proxy gadget server, and why is it using undocumented/unadvertised functionality? I'd be happy to put backward-compatibility with plans for complete deprecation some day. ie: I'll warn for now, and remove in 1 week. Sounds good? > - Henry > > On Wed, May 4, 2011 at 4:54 PM, Michael Hermanto <[email protected]> > wrote: > > 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 > >> > > > > > > -- > Thanks, > Henry >
