On Mon, Dec 7, 2009 at 11:34 AM, Paul Lindner <[email protected]> wrote:
> Is there a use-case where you want to force javascript to be inlined? > Obviously this CL has a global on/off switch for that behavior, so you're > safe there plus libs always take precedence. > Pretty much when optimizing for the cold cache case. I agree that the change is completely safe, and is a good idea, especially when generating URLs ad hoc. > > The way I see it, requiring the front-end to compute the libs parameter > requires that it have much more information about the gadget metadata that > it doesn't necessarily need. > Agreed on this as well. It just so happens that, for a variety of reasons, at Google all our clients generate URLs server-side before displaying a gadget, so we're able to perform various optimizations such as libs sharing. -j > > > On Mon, Dec 7, 2009 at 11:29 AM, John Hjelmstad <[email protected]> wrote: > > > FWIW: > > > > While I'm fine w/ this CL as implemented, I'll note that we've dealt with > > this at the URL generation side rather than gadget rendering side, as it > > affords more flexibility on a render-by-render basis. > > > > 2c, > > John > > > > On Mon, Dec 7, 2009 at 6:58 PM, <[email protected]> wrote: > > > > > Author: lindner > > > Date: Mon Dec 7 18:58:24 2009 > > > New Revision: 888082 > > > > > > URL: http://svn.apache.org/viewvc?rev=888082&view=rev > > > Log: > > > SHINDIG-1227 | Patch from chirag shah | Allow JavaScript features > > required > > > by a gadget to be externalized on demand > > > > > > Modified: > > > incubator/shindig/trunk/java/common/conf/shindig.properties > > > > > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java > > > > > > Modified: incubator/shindig/trunk/java/common/conf/shindig.properties > > > URL: > > > > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/common/conf/shindig.properties?rev=888082&r1=888081&r2=888082&view=diff > > > > > > > > > ============================================================================== > > > --- incubator/shindig/trunk/java/common/conf/shindig.properties > > (original) > > > +++ incubator/shindig/trunk/java/common/conf/shindig.properties Mon Dec > > 7 > > > 18:58:24 2009 > > > @@ -63,6 +63,10 @@ > > > #shindig.gadget-rewrite.default-forced-libs=core:core.io > > > shindig.gadget-rewrite.default-forced-libs= > > > > > > +# > > > +# Allow supported JavaScript features required by a gadget to be > > > externalized on demand > > > +shindig.gadget-rewrite.externalize-feature-libs=false > > > + > > > # Configuration for image rewriter > > > shindig.image-rewrite.max-inmem-bytes = 1048576 > > > shindig.image-rewrite.max-palette-size = 256 > > > > > > Modified: > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java > > > URL: > > > > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java?rev=888082&r1=888081&r2=888082&view=diff > > > > > > > > > ============================================================================== > > > --- > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java > > > (original) > > > +++ > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java > > > Mon Dec 7 18:58:24 2009 > > > @@ -41,6 +41,7 @@ > > > import org.apache.shindig.gadgets.spec.ModulePrefs; > > > import org.apache.shindig.gadgets.spec.UserPref; > > > import org.apache.shindig.gadgets.spec.View; > > > +import org.apache.commons.lang.StringUtils; > > > import org.w3c.dom.Document; > > > import org.w3c.dom.Element; > > > import org.w3c.dom.Node; > > > @@ -99,6 +100,8 @@ > > > protected final RpcServiceLookup rpcServiceLookup; > > > protected Set<String> defaultExternLibs = ImmutableSet.of(); > > > > > > + protected Boolean externalizeFeatures = false; > > > + > > > /** > > > * @param messageBundleFactory Used for injecting message bundles > into > > > gadget output. > > > */ > > > @@ -117,11 +120,16 @@ > > > > > > @Inject > > > public void > > > > > > setDefaultForcedLibs(@Named("shindig.gadget-rewrite.default-forced-libs")String > > > forcedLibs) { > > > - if (forcedLibs != null && forcedLibs.length() > 0) { > > > + if (StringUtils.isNotBlank(forcedLibs)) { > > > defaultExternLibs = > > > ImmutableSortedSet.copyOf(Arrays.asList(forcedLibs.split(":"))); > > > } > > > } > > > > > > + @Inject(optional = true) > > > + public void > > > > > > setExternalizeFeatureLibs(@Named("shindig.gadget-rewrite.externalize-feature-libs")Boolean > > > externalizeFeatures) { > > > + this.externalizeFeatures = externalizeFeatures; > > > + } > > > + > > > public void rewrite(Gadget gadget, MutableContent mutableContent) > > throws > > > RewritingException { > > > // Don't touch sanitized gadgets. > > > if (gadget.sanitizeOutput()) { > > > @@ -215,40 +223,37 @@ > > > // TODO: If there isn't any js in the document, we can skip this. > > > Unfortunately, that means > > > // both script tags (easy to detect) and event handlers (much more > > > complex). > > > GadgetContext context = gadget.getContext(); > > > - String externParam = context.getParameter("libs"); > > > > > > - // List of extern libraries we need > > > - Set<String> extern; > > > + // Set of extern libraries requested by the container > > > + Set<String> externForcedLibs = defaultExternLibs; > > > > > > // gather the libraries we'll need to generate the extern libs > > > - if (externParam == null || externParam.length() == 0) { > > > - // Don't bother making a mutable copy if the list is empty > > > - extern = (defaultExternLibs.isEmpty()) ? defaultExternLibs : > > > - Sets.newTreeSet(defaultExternLibs); > > > - } else { > > > - extern = Sets.newTreeSet(Arrays.asList(externParam.split(":"))); > > > + String externParam = context.getParameter("libs"); > > > + if (StringUtils.isNotBlank(externParam)) { > > > + externForcedLibs = > > > Sets.newTreeSet(Arrays.asList(externParam.split(":"))); > > > } > > > - > > > - if (!extern.isEmpty()) { > > > - String jsUrl = urlGenerator.getBundledJsUrl(extern, context); > > > + > > > + if (!externForcedLibs.isEmpty()) { > > > + String jsUrl = urlGenerator.getBundledJsUrl(externForcedLibs, > > > context); > > > Element libsTag = > > headTag.getOwnerDocument().createElement("script"); > > > libsTag.setAttribute("src", jsUrl); > > > headTag.appendChild(libsTag); > > > } > > > - > > > + > > > List<String> unsupported = Lists.newLinkedList(); > > > - List<FeatureResource> externResources = > > > - featureRegistry.getFeatureResources(gadget.getContext(), > extern, > > > unsupported); > > > + > > > + List<FeatureResource> externForcedResources = > > > + featureRegistry.getFeatureResources(context, externForcedLibs, > > > unsupported); > > > if (!unsupported.isEmpty()) { > > > LOG.info("Unknown feature(s) in extern &libs=: " + > > > unsupported.toString()); > > > unsupported.clear(); > > > } > > > - > > > + > > > // Get all resources requested by the gadget's requires/optional > > > features. > > > Map<String, Feature> featureMap = > > > gadget.getSpec().getModulePrefs().getFeatures(); > > > List<String> gadgetFeatureKeys = > > > Lists.newArrayList(gadget.getDirectFeatureDeps()); > > > List<FeatureResource> gadgetResources = > > > - featureRegistry.getFeatureResources(gadget.getContext(), > > > gadgetFeatureKeys, unsupported); > > > + featureRegistry.getFeatureResources(context, > gadgetFeatureKeys, > > > unsupported); > > > if (!unsupported.isEmpty()) { > > > List<String> requiredUnsupported = Lists.newLinkedList(); > > > for (String notThere : unsupported) { > > > @@ -260,13 +265,33 @@ > > > if (!requiredUnsupported.isEmpty()) { > > > throw new > > > UnsupportedFeatureException(requiredUnsupported.toString()); > > > } > > > + } > > > + > > > + // Inline or externalize the gadgetFeatureKeys > > > + List<FeatureResource> inlineResources = Lists.newArrayList(); > > > + List<String> allRequested = Lists.newArrayList(gadgetFeatureKeys); > > > + > > > + if (externalizeFeatures) { > > > + Set<String> externGadgetLibs = > > > Sets.newTreeSet(featureRegistry.getFeatures(gadgetFeatureKeys)); > > > + externGadgetLibs.removeAll(externForcedLibs); > > > + > > > + if (!externGadgetLibs.isEmpty()) { > > > + String jsUrl = urlGenerator.getBundledJsUrl(externGadgetLibs, > > > context); > > > + Element libsTag = > > > headTag.getOwnerDocument().createElement("script"); > > > + libsTag.setAttribute("src", jsUrl); > > > + headTag.appendChild(libsTag); > > > + } > > > + } else { > > > + inlineResources.addAll(gadgetResources); > > > } > > > - > > > + > > > // Calculate inlineResources as all resources that are needed by > the > > > gadget to > > > // render, minus all those included through externResources. > > > // TODO: profile and if needed, optimize this a bit. > > > - List<FeatureResource> inlineResources = > > > Lists.newArrayList(gadgetResources); > > > - inlineResources.removeAll(externResources); > > > + if (!externForcedLibs.isEmpty()) { > > > + allRequested.addAll(externForcedLibs); > > > + inlineResources.removeAll(externForcedResources); > > > + } > > > > > > // Precalculate the maximum length in order to avoid excessive > > garbage > > > generation. > > > int size = 0; > > > @@ -280,8 +305,6 @@ > > > } > > > } > > > > > > - List<String> allRequested = Lists.newArrayList(gadgetFeatureKeys); > > > - allRequested.addAll(extern); > > > String libraryConfig = > > > getLibraryConfig(gadget, > > > featureRegistry.getFeatures(allRequested)); > > > > > > @@ -348,8 +371,7 @@ > > > } > > > > > > addHasFeatureConfig(gadget, config); > > > - > addOsapiSystemListMethodsConfig(gadget.getContext().getContainer(), > > > config, > > > - gadget.getContext().getHost()); > > > + addOsapiSystemListMethodsConfig(context.getContainer(), config, > > > context.getHost()); > > > addSecurityTokenConfig(context, config); > > > return "gadgets.config.init(" + JsonSerializer.serialize(config) + > > > ");\n"; > > > } > > > > > > > > > > > >
