Your change makes sense now. When I started making this override mechanism it was only intended to override the default shindig.properties file for the Shindig host, port and contextRoot. Back then we had a separate file for the db persistence. A few weeks later these properties files were merged. Using a system property to locate the custom properties file is still not the most elegant solution (noticed that when we actually started deploying Rave on a server and had to modify Tomcat's setenv.sh). Any improvement is welcome.
Jasha Joachimsthal Europe - Amsterdam - Oosteinde 11, 1017 WT Amsterdam - +31(0)20 522 4466 US - Boston - 1 Broadway, Cambridge, MA 02142 - +1 877 414 4776 (toll free) www.onehippo.com On 6 January 2012 14:36, Ciancetta, Jesse E. <[email protected]> wrote: > Hi Jasha, > > After just taking a closer look at ConfigurablePropertiesModule -- I'm > wondering if maybe the intent was to have it read from > rave.shindig.properties by default instead of shindig.properties? I think > if that were the case things would make more sense. Here is what the > default file is now: > > private static final String DEFAULT_PROPERTIES = "shindig.properties"; > > but I'm wondering if that is actually supposed to be: > > private static final String DEFAULT_PROPERTIES = "rave.shindig.properties"; > > If that were the case then the same properties file > (rave.shindig.properties) would be used to initialize both the Spring and > Guice environments. > > WDYT? > > Thanks, > > --Jesse > > >-----Original Message----- > >From: Jasha Joachimsthal [mailto:[email protected]] > >Sent: Thursday, January 05, 2012 4:40 PM > >To: [email protected] > >Subject: Re: rave.shindig.properties > > > >Jesse, > > > >I'm afraid you ran into one of the ugliest solutions I've ever written. > >There were a few issues I had to deal with: > >1 override the default shindig.properties location to modify settings for > >host, port, scheme, location for the container.js in a single file outside > >the war for deployments on different servers (DTAP) > >2 keep the Shindig override mechanism to set the host and contextRoot > >through individual system properties (for compatibility) > >3 most properties were handled through Guice modules > >4 some properties are handled through Spring (persistence etc) > >5 use a single file for the rave shindig properties because non-committers > >won't know which properties are handled by what module and the oauth > >properties are even shared with both Spring and Guice > > > > > >Jasha > > > > > >On 5 January 2012 21:26, Ciancetta, Jesse E. <[email protected]> wrote: > > > >> >-----Original Message----- > >> >From: marijan milicevic [mailto:[email protected]] > >> >Sent: Thursday, January 05, 2012 3:02 PM > >> >To: [email protected] > >> >Subject: Re: rave.shindig.properties > >> > > >> >Hi Jesse, > >> > > >> >On 01/05/2012 08:54 PM, Ciancetta, Jesse E. wrote: > >> >> I dug a little further yet and tried removing all of the copy/paste > >> >shindig.properties values from rave.shindig.properties and found that > >> three > >> >of the properties: > >> >> > >> >> shindig.signing.key-name > >> >> shindig.signing.key-file > >> >> shindig.signing.global-callback-url > >> >> > >> >> are used to initialize the > >> >org.apache.rave.gadgets.oauth.inject.DefaultOAuthStore via Spring > >property > >> >replacement in rave-shindig-applicationContext.xml -- but none of the > rest > >> >are being used by any Spring components and as I mentioned earlier I > >don't > >> >see anywhere where these values are making it from > >rave.shindig.properties > >> >into any kind of guice context which means (as far as I can tell) they > >> would > >> >only be used by Spring components. > >> >> > >> >> I was able to delete all of the shindig.properties values from > >> >rave.shindig.properties except the three mentioned above and Rave still > >> >seemed to run just fine. > >> >> > >> >> I also noticed that the three values at the top of the file: > >> >> > >> >> shindig.host > >> >> shindig.port > >> >> shindig.contextroot > >> > > >> >these are used here: > >> > > >> > >>org.apache.rave.commoncontainer.ConfigurablePropertiesModule#overrida > >b > >> >leProperties > >> > >> Thanks for having a look marijan! I'm really not very familiar with > this > >> area of the codebase so I appreciate the extra eyes on it. > >> > >> I'd found those references too and thought it was a usage of those > >> properties at first glance as well -- but it actually isn't -- or, at > least > >> I don't think it is :) > >> > >> If you really dig into what that usage is -- it's actually generating a > >> list of system properties that the > >> > >org.apache.rave.commoncontainer.ConfigurablePropertiesModule#override > >PropertyValuesWithSystemProperties > >> method uses to scan for system properties (the ones defined in that > list) > >> and inject them into the configuration bundle that the > >> ConfigurablePropertiesModule builds. > >> > >> One other note -- the ConfigurablePropertiesModule class actually deals > >> with the shindig.properties file and not the rave.shindig.properties. > >> > >> >cheers > >> >marijan > >> >> don't appear to be in use anywhere either -- so I tried deleting > those > >> too > >> >and running Rave and everything still seemed fine. > >> >> > >> >> So unless anyone objects I'd like to propose that we rename the three > >> >shindig.signing.X properties that are currently in use to something > else > >> (so > >> >that people don't confuse them with the ones already defined in > >> >shindig.properties) and remove all of the other properties which don't > >> appear > >> >to be in use. > >> >> > >> >> Any objections? > >> >> > >> >> --Jesse > >> >> > >> >>> -----Original Message----- > >> >>> From: Ciancetta, Jesse E. [mailto:[email protected]] > >> >>> Sent: Thursday, January 05, 2012 2:11 PM > >> >>> To: [email protected] > >> >>> Subject: rave.shindig.properties > >> >>> > >> >>> Hi All, > >> >>> > >> >>> I'm working on documenting how to get OpenSocial gadgets rendered > >on > >> >>> locked-domains using Rave and am running into an issue with the > >> >>> rave.shindig.properties file. > >> >>> > >> >>> That properties file has some custom Rave specific properties > defined > >> at > >> >the > >> >>> top, but also has a copy/paste of all of the default properties from > >> the > >> >>> shindig.properties file at the bottom. The issue I'm running into > is > >> that > >> >>> changing one of those shindig specific properties (shindig.locked- > >> >>> domain.enabled) doesn't seem to be making its way to shindig at > >> runtime. > >> >>> > >> >>> I dug into this a bit and from what I can tell it appears that the > >> copy/paste > >> >of > >> >>> the shindig.properties probably isn't supposed to actually be there > as > >> >those > >> >>> values are never (from what I can see) injected into guice where the > >> >shindig > >> >>> components would then receive them. The rave.shindig.properties > file > >> >>> appears to only be read by the > >> >>> org.apache.rave.util.OverridablePropertyPlaceholderConfigurer class, > >> and > >> >it > >> >>> appears that classes job is to do some manipulation of the > properties > >> read > >> >in > >> >>> and then make them available to the Spring runtime. > >> >>> > >> >>> Digging further -- I found that we actually have a custom guice > module > >> >>> (org.apache.rave.commoncontainer.ConfigurablePropertiesModule) > >> >which is > >> >>> responsible for loading the shindig.properties contents from either > the > >> >>> shindig.properties file proper or a custom file/location specified > as a > >> >system > >> >>> property (shindig.override.properties). > >> >>> > >> >>> So is the copy/paste of all the shindig.properties values into > >> >>> rave.shindig.properties a carryover from something we were using in > >the > >> >past > >> >>> and no longer need, or am I just misunderstanding something? > >> >>> > >> >>> Thanks! > >> >>> > >> >>> --Jesse > >> > >> >
