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.
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. 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"; > > } > > > > > > >
