Ok -- I'm going to go ahead and change the ConfigurablePropertiesModule to load properties from rave.shindig.properties then instead of shindig.properties then.
So this means that our rave.shindig.properties file actually replaces the default shindig.properties file, but since we have all the properties from the default shindig.properties file in there already I think that makes sense and is what people would expect (I know that's what I was expecting when I saw all the default shindig properties in there). I'm just testing out that change one more time now and then will commit. --Jesse >-----Original Message----- >From: Jasha Joachimsthal [mailto:[email protected]] >Sent: Friday, January 06, 2012 10:18 AM >To: [email protected] >Subject: Re: rave.shindig.properties > >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#overri >da >> >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#overrid >e >> >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 >> >> >> >> >>
