Reviewers: shindig.remailer_gmail.com, jasvir, Jon Weygandt,

Description:
The feature.xml and JS system has served us well, yet various new
requirements/requests have arisen that have stressed the current
implementation. Among them:
* Ability to cleanly represent "code" features and "JS" features in a
dependency tree w/ one another. This allows, for instance, a clean way
for "opensocial" to depend on "security-token" (not yet introduced),
which in turn depends on "locked-domain" to ensure token security.
* Ability to let a gadget opt out of all "core" JavaScript, instead only
including sub-components that are used, to shrink render size.
* Ability to serve JS in a context-sensitive fashion ie.
browser-specific code.
* Ability to serve JS in a feature-sensitive fashion ie. only inject
certain code when feature="caja" is enabled/active.
* Ability in development mode to easily serve JS that changes when edits
are made on it.

For these and general code-cleanup purposes, I've rewritten the
Feature/JS inclusion implementation as presented in this CL.

This implementation is functionally 100% backward-compatible with its
predecessor. It is only NOT backward-compatible with existing extensions
(outside of Shindig's code base) that override
JsFeatureLoader/GadgetFeatureRegistry.

With this change, the unit of extension in the features system becomes
the <script ...> line in a feature.xml. The processor for this line is a
FeatureResourceLoader, for which a default implementation is provided.
Each <script> line yields a FeatureResource object, which is in turn
used to resolve content and debugContent by rendering/JS code.

Other components are overridable/subclassable as well, but to date I've
yet to hear a JS/features extension use case that does not fit in the
ResourceLoader model (or, in the current model with dynamically
register()'ed features). Hopefully that's right.

An overview of the implementation follows.

FeatureRegistry
- Replaces GadgetFeatureRegistry and JsFeatureLoader
- Performs both register() and getFeaturesXXX() functions
- Does NOT implicitly include "core*" APIs anymore! These are included
in the dependency tree. @see GadgetSpec for more below.
- register(...)
  + Registers one or more features in the system, incorporating them
into the list of known features and building a validated dependency tree
with them. If a circular dependency exists, an error is thrown.
  + Method may be called at ANY time -- the registry is not
programmatically locked. This allows (though this use is not tested!)
for dynamic feature injection at some later point.
- getFeatureResources(...)
  + Queries the feature tree for resources matching the specified
context (GADGET || CONTAINER), which satisfy all the needed dependencies
given in the query. Optionally, fills in unknown/unsupported features
from the needed list.
- getFeatures(List<String>)
  + Returns feature strings in dependency order that satisfy the given
needed features.

FeatureResourceLoader
- @Injected into FeatureRegistry, main extension point for feature
system.
- Loads a given FeatureResource from a feature.xml <script src="..."/>
line (inline features are handled by FeatureRegistry).
- Key API: load(URI, Map<String,String> attribs)
  + URI is pre-resolved to be absolute by FeatureParser (relative to
parent, if a relative URI in feature.xml)
  + "attribs" allows for arbitrary context-sensitive extensions. Example
to come soon: limiting by feature="..." to include Caja tamings.js
per-feature only when Caja is available.

FeatureParser
- Mostly an implementation detail of FeatureRegistry for now
(package-private).
- Parses a String into an intermediary object for further processing.
- Resolves URIs of Resources relative to parent.

GadgetSpec
- If no feature is explicitly <Optional> or <Require>'d starting with
"core", automatically inserts the "core" feature for backward
compatibility.
- No way (in this CL) to include "absolutely no core" yet.

Gadget
- Modified how addFeature and removeFeature work. They now manage a
"directlyRequestedDeps" list, which in turn is used by
rendering/processing code for feature resolution.

RenderingGadgetRewriter
- Update injectFeatureLibraries(...) to use new API.
- Hopefully more straightforward: obtain resources requested by the
gadget, and resources specified externally. Subtract latter from the
former and inline those. Extern the rest. No "core" assumptions, as
"core" will be included by the GadgetSpec if needed.

Core features
- Break "core" into core.config, core.json, core.prefs et al.
- "core" now an aggregating feature that has no JS of its own but
includes all sub-libs.

All other features
- Specific assignment of dependencies on core libs.

Other
- feature.xml updates, pom.xml updates for separated core libs
- test updates across the board for all affected classes


Please review this at http://codereview.appspot.com/143046

Affected files:
  features/pom.xml
  features/src/main/javascript/features/analytics/feature.xml
  features/src/main/javascript/features/auth-refresh/feature.xml
  features/src/main/javascript/features/core.auth/feature.xml
  features/src/main/javascript/features/core.config/feature.xml
  features/src/main/javascript/features/core.io/feature.xml
  features/src/main/javascript/features/core.json/feature.xml
  features/src/main/javascript/features/core.legacy/feature.xml
  features/src/main/javascript/features/core.log/feature.xml
  features/src/main/javascript/features/core.prefs/feature.xml
  features/src/main/javascript/features/core.util/feature.xml
  features/src/main/javascript/features/core/auth-init.js
  features/src/main/javascript/features/core/auth.js
  features/src/main/javascript/features/core/config.js
  features/src/main/javascript/features/core/core.js
  features/src/main/javascript/features/core/feature.xml
  features/src/main/javascript/features/core/json.js
  features/src/main/javascript/features/core/legacy.js
  features/src/main/javascript/features/core/log.js
  features/src/main/javascript/features/core/prefs.js
  features/src/main/javascript/features/core/util.js
  features/src/main/javascript/features/features.txt
  features/src/main/javascript/features/flash/feature.xml
  features/src/main/javascript/features/i18n/feature.xml
  features/src/main/javascript/features/opensocial-0.6/feature.xml
  features/src/main/javascript/features/opensocial-current/feature.xml
  features/src/main/javascript/features/opensocial-data/feature.xml
  features/src/main/javascript/features/opensocial-jsonrpc/feature.xml
  features/src/main/javascript/features/opensocial-reference/feature.xml
  features/src/main/javascript/features/osapi/feature.xml
  features/src/main/javascript/features/rpc/feature.xml
  features/src/main/javascript/features/setprefs/feature.xml
  features/src/main/javascript/features/skins/feature.xml
  features/src/main/javascript/features/views/feature.xml
  java/common/src/main/java/org/apache/shindig/common/Pair.java
java/gadgets/src/main/java/org/apache/shindig/gadgets/BrowserSpecificRpcJsFeatureLoader.java java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultUrlGenerator.java
  java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java
  java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeature.java
java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java
  java/gadgets/src/main/java/org/apache/shindig/gadgets/JsFeatureLoader.java
  java/gadgets/src/main/java/org/apache/shindig/gadgets/JsLibrary.java
java/gadgets/src/main/java/org/apache/shindig/gadgets/features/BrowserSpecificFeatureResource.java java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureParser.java java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResourceLoader.java java/gadgets/src/main/java/org/apache/shindig/gadgets/process/Processor.java java/gadgets/src/main/java/org/apache/shindig/gadgets/render/OpenSocialI18NGadgetRewriter.java java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java
  java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java
java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java
  java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Preload.java
java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/FlashTagHandler.java java/gadgets/src/test/java/org/apache/shindig/gadgets/BrowserSpecificRpcJsFeatureLoaderTest.java java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultUrlGeneratorTest.java java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetFeatureRegistryTest.java
  java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTest.java
java/gadgets/src/test/java/org/apache/shindig/gadgets/HashLockedDomainServiceTest.java java/gadgets/src/test/java/org/apache/shindig/gadgets/JsFeatureLoaderTest.java
  java/gadgets/src/test/java/org/apache/shindig/gadgets/JsLibraryTest.java
java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureParserTest.java java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureResourceLoaderTest.java java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriterTest.java java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/PipelineDataGadgetRewriterTest.java java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/TemplateRewriterTest.java java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/tags/FlashTagHandlerTest.java


Reply via email to