You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by li...@apache.org on 2009/12/07 19:58:27 UTC
svn commit: r888082 - in /incubator/shindig/trunk/java:
common/conf/shindig.properties
gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java
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";
}
Re: svn commit: r888082 - in /incubator/shindig/trunk/java:
common/conf/shindig.properties gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java
Posted by John Hjelmstad <fa...@google.com>.
On Mon, Dec 7, 2009 at 11:34 AM, Paul Lindner <li...@inuus.com> 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 <fa...@google.com> 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, <li...@apache.org> 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";
> > > }
> > >
> > >
> > >
> >
>
Re: svn commit: r888082 - in /incubator/shindig/trunk/java:
common/conf/shindig.properties gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java
Posted by Paul Lindner <li...@inuus.com>.
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 <fa...@google.com> 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, <li...@apache.org> 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";
> > }
> >
> >
> >
>
Re: svn commit: r888082 - in /incubator/shindig/trunk/java:
common/conf/shindig.properties gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java
Posted by John Hjelmstad <fa...@google.com>.
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, <li...@apache.org> 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";
> }
>
>
>