> On 2011-09-30 22:37:19, Henry Saputra wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminModule.java, > > line 54 > > <https://reviews.apache.org/r/1666/diff/7/?file=47001#file47001line54> > > > > Why do we inject BasicGadgetAdminStore instead of GadgetAdminStore? > > > > Are we expecting Shindig implementors to override the > > GadgetAdminModule? I believe the config/gadget-admin.json file contain > > initial whitelisting list of gadgets? > > > > It would be better if we could find a way to provide plugin or > > injectable hook to different data source to load this initial whitelist > > gadgets def. > > Ryan Baxter wrote: > I was thinking the people who want to use something else than the > gadget-admin.json file they would override the DefaultGuiceModule. They > could choose to override the GadgetAdminStore if they want... > > Henry Saputra wrote: > I would think its better to have the admin store injection as provider > and same as the initial whitelist gadgets. We should try to make code that > does not need to be modified to be flexible enough to allow extension in the > right place rather than override the whole DefaultGuiceModule.
Henry I am not sure I understand what you are suggesting....I can place the GadgetAdminModule directly in the web.xml so there is no need to override the DefaultGuiceModule just to replace the GadgetAdminModule, is that what you are suggesting? - Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1666/#review2240 ----------------------------------------------------------- On 2011-09-30 19:21:13, Ryan Baxter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1666/ > ----------------------------------------------------------- > > (Updated 2011-09-30 19:21:13) > > > Review request for shindig, Paul Lindner, Henry Saputra, johnfargo, Dan > Dumont, and Stanton Sievers. > > > Summary > ------- > > We want to add some administration features to Shindig. This patch > adds/changes 3 things in Shindig. > > 1. Remove the existing blacklist functionality in Shindig which currently is > enabled by pointing Shindig to a text file with a list of gadgets to > blacklist. The new functionality uses a whitelist instead of a blacklist and > is indexed on a per container basis. Meaning an admin could whitelist some > gadgets in one container but not another. This functionality is enabled > through a Guice module allowing implementors to use something else besides a > text file. > > 2. Add the ability for administrators to specify which features are allowed > and denied for a specific gadget in a specific container. This information > is checked in two places, when the metadata request is made and when the > gadget is rendered. > > 3. Add the ability for containers to secure RPC requests made by gadgets. > The RPC code now has the ability to arbitrate all RPC calls made. Containers > can specify their own arbitrator. The common container has its own default > arbitrator available. > > > This addresses bug SHINDIG-1601. > https://issues.apache.org/jira/browse/SHINDIG-1601 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1177430 > http://svn.apache.org/repos/asf/shindig/trunk/config/gadget-admin.json > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/rpc/rpc.js > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/logging/i18n/MessageKeys.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/resources/org/apache/shindig/common/logging/i18n/resource.properties > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/resources/org/apache/shindig/common/logging/i18n/resource_en_US.properties > 1177430 > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/pom.xml 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetBlacklist.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetBlacklist.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetException.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/RenderingContext.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStore.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/ContainerAdminData.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/FeatureAdminData.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminData.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminModule.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminStore.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/ServerAdminData.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/process/Processor.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetBlacklistTest.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStoreTest.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ContainerAdminDataTest.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/FeatureAdminDataTest.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/GadgetAdminDataTest.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ServerAdminDataTest.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/process/ProcessorTest.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriterTest.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeProcessor.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestServletTest.java > 1177430 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java > 1177430 > > Diff: https://reviews.apache.org/r/1666/diff > > > Testing > ------- > > Created/updated unit tests. > > > Thanks, > > Ryan > >
