On Mon, Jul 19, 2010 at 10:53 AM, Michael Young <[email protected]>wrote:
> I think #2 would probably be a more reusable solution. Is this something > that would be a worthy contribution? > I think it would be. > > If yes, I could think of several places where I could set the TheadLocal. > Is there a certain pattern already in place I can follow? > 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. 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. 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 > >>>> > >>>> > >> > >> > >
