On Mon, Jul 19, 2010 at 11:38 AM, Michael Young <[email protected]>wrote:
> > > There's not, though I know of several installations that implement a > > general-purpose context object using a ThreadLocal in the background, for > > access to a wide array of generally useful data. We (@ Goog) have such an > > implementation that I've been interested for some time in porting to > > Shindig. > > > > The trickiest piece has to do with consistent access to such object for > > multithreaded operations, such as a few metadata operations to name a > few. > > We have a "SharedThreadLocal" implementation that copies ThreadLocal > state > > from parent to child, in a custom overridden ExecutorService when > executing > > a new job. > > > > Building this "full" implementation sounds a bit heavyweight at the > moment, > > though if you're keen to tackle it I'd be happy to sync w/ you. > > It sounds a bit much to me too at the moment. Unless there's a broad use > case for it among the community I'd generally like to avoid until that need > arises. > > > > > The simplest, and likely preferred, alternative is simply to @Inject > > Provider<String> currentHostProvider into classes that need it, and > provide > > a default implementation of the Provider that grabs currentHost from > > HttpServletRequest in a Filter. This can be augmented w/ relatively > little > > trouble later if/when building out such a context object. > > This is exactly what I've done and is super simple and concise (testing at > the moment). I would be happy to contribute this. Is the best way just to > create JIRA ticket and attach a diff? I've seen people submit code for > review, but I'm not familiar with that process. > JIRA is appreciated for documentation of new functionality. All code reviews are sent as patches to http://codereview.appspot.com, repository Shindig - current trunk as of 2/2010, reviewers: [email protected] I'd be happy to take a look, as I'm sure would others. --j > > > > > What do you think? > > > > John > > > > > >> > >> > >> > >> - Mike > >> -- > >> Liferay West Coast Symposium > >> September 8-9, 2010 > >> Anaheim, CA > >> www.liferay.com/wcs > >> -- > >> Follow us on Twitter: liferay > >> > >> On Jul 17, 2010, at 10:06 AM, John Hjelmstad wrote: > >> > >>> On Fri, Jul 16, 2010 at 3:26 PM, Michael Young < > >> [email protected]>wrote: > >>> > >>>> Hi John, > >>>> > >>>> I've begun doing this, but I ran into a few problems: > >>>> > >>>> 1) Right now it's not very easy to override the Guice binding to > >>>> JsUriManager. I had to subclass DefaultGuiceModule and UriModule and > >> copy > >>>> the configure() methods in my subclasses to swap it out. > >>>> > >>>> 2) DefaultJsUriManager does not like %host%. Specifically, somewhere > in > >>>> Uri.parse(jsHost) it creates a java.net.Uri, and it doesn't like > %host% > >>>> naturally. I can work around this by completely reimplementing > >>>> DefaultJsUriManager. While possible, it seems to be a lot of bandaging > >> to > >>>> get some old functionality back. > >>> > >>> > >>>> 3) container.js still references %host%. Obviously this must work in > >>>> somebody's test environnment, but not the sample / default env. > >>>> > >>>> You mentioned that in your implementation you have overridden behavior > >> to > >>>> support %host%. It works for you? > >>>> > >>> > >>> True about the %host% parsing issue. I can think of two fixes: > >>> 1. Modify DefaultJsUriManager's getReqConfig(config, JS_HOST_PARAM) > line > >> to > >>> delegate to a new protected method getHost(), whose default impl uses > >>> ContainerConfig. > >>> 2. Simulate the same thing by creating a delegating ContainerConfig > >>> implementation that replaces %host% for values it returns. You'll need > a > >>> context object for this; a ThreadLocal could do in a pinch, while > waiting > >>> for the base class to be updated as per #1. > >>> > >>> --j > >>> > >>> > >>>> > >>>> Thanks again for the help. > >>>> > >>>> - Mike > >>>> -- > >>>> Liferay West Coast Symposium > >>>> September 8-9, 2010 > >>>> Anaheim, CA > >>>> www.liferay.com/wcs > >>>> -- > >>>> Follow us on Twitter: liferay > >>>> > >>>> On Jul 9, 2010, at 5:17 PM, John Hjelmstad wrote: > >>>> > >>>>> %host% is a regression to some degree... though is useful, and is in > >> need > >>>> of > >>>>> a replacement. Given 2.0 is a breaking API release, it was removed. > >>>>> Rationale is that its logic was being sprinkled throughout the code, > >> with > >>>>> the substitution coming from gadget.getContext().getHost(). IMO > better > >>>> would > >>>>> be to have a more general injected context object that, for instance, > >>>>> ContainerConfig would use. > >>>>> > >>>>> To support it in the interim, you can subclass DefaultJsUriManager > and > >>>>> perform the replacement: > >>>>> Uri uri = super.makeExternJsUri(); > >>>>> if (uri.toString().contains("%host%")) { > >>>>> uri = new UriBuilder(uri.replaceAll("%host%", > >>>>> gadget.getContext().getHost())).toUri(); > >>>>> } > >>>>> return uri; > >>>>> > >>>>> %js% was replaced with a better-structured host/path based config > >>>> mechanism. > >>>>> Let me know if there's a use case its removal breaks. > >>>>> > >>>>> --j > >>>>> > >>>>> On Fri, Jul 9, 2010 at 4:29 PM, Michael Young < > >> [email protected] > >>>>> wrote: > >>>>> > >>>>>> DefaultURLGenerator used to substitute these tokens. Is this a > >>>> regresssion, > >>>>>> or is there an alternative way to specify values for host and js at > >>>> runtime? > >>>>>> > >>>>>> > >>>>>> > >>>>>> - Mike > >>>>>> -- > >>>>>> Liferay West Coast Symposium > >>>>>> September 8-9, 2010 > >>>>>> Anaheim, CA > >>>>>> www.liferay.com/wcs > >>>>>> -- > >>>>>> Follow us on Twitter: liferay > >>>>>> > >>>>>> > >>>> > >>>> > >> > >> > >
