FWIW here's my implementation, which isn't that hard to maintain. The
only tricky part is when SocialApiGuiceModule changes. That requires
executing svn log/diff on that file when bindings are added or changed.
First the web.xml
<context-param>
<param-name>guice-modules</param-name>
<param-value>
com.hi5.os.Hi5PropertiesModule:
com.hi5.os.Hi5GuiceModule:
org.apache.shindig.gadgets.servlet.AuthenticationModule:
org.apache.shindig.gadgets.oauth.OAuthModule
</param-value>
</context-param>
Need to use our own custom properties:
public class Hi5PropertiesModule extends PropertiesModule {
public void configure() {
super.configure();
}
static private Properties genProps() {
Properties props =
ConfigurationConverter
.getProperties(Config.getInstance().getFriendConfig());
String keyfile = props.getProperty("shindig.signing.key-file");
keyfile = Config.getInstance().locateFileInPath(keyfile);
props.setProperty("shindig.signing.key-file", keyfile);
return props;
}
/**
* Creates module with standard properties.
*/
public Hi5PropertiesModule() {
// Initialize with properties from Friend.properties
super(genProps());
}
}
And then here's everything else:
public class Hi5GuiceModule extends DefaultGuiceModule {
public void configure() {
super.configure();
bind(PersonService.class).to(Hi5PeopleService.class);
bind(AppDataService.class).to(Hi5AppDataService.class);
bind(ActivityService.class).to(Hi5ActivityService.class);
bind(MessageService.class).to(Hi5MessageService.class);
bind(HttpFetcher.class).to(Hi5HttpFetcher.class);
bind(SecurityTokenDecoder.class).to(Hi5SecurityTokenDecoder.class);
bind
(HandlerDispatcher.class).toProvider(HandlerDispatcherProvider.class);
// cut and paste junk from SocialApiGuiceModule, removed for
brevity..
}
public Hi5GuiceModule() {
super();
}
/**
* Provider for the HandlerDispatcher. Adds the hi5 albums handler
* at "albums".
*/
static class HandlerDispatcherProvider implements
Provider<HandlerDispatcher> {
private final HandlerDispatcher dispatcher;
@Inject
public HandlerDispatcherProvider(StandardHandlerDispatcher
dispatcher,
Provider<Hi5AlbumHandler> albumHandler) {
dispatcher.addHandler("albums", albumHandler);
this.dispatcher = dispatcher;
}
public HandlerDispatcher get() {
return dispatcher;
}
On Dec 3, 2008, at 2:12 PM, Adam Winer wrote:
On Wed, Dec 3, 2008 at 1:57 PM, Henning P. Schmiedehausen
<[EMAIL PROTECTED]> wrote:
Adam Winer <[EMAIL PROTECTED]> writes:
This is bad. You now have to merge in any code changes to this
module into
your own customized module... forever. Someone tell me that this
is fixed
in Guice 2.0.
Yes, Guice 2.0 has good support for overriding modules.
http://publicobject.com/2008/05/overriding-bindings-in-guice.html
Ah, now I see your (and Kevin's and Paul's) reasoning:
You are argueing that e.g. DefaultGuiceModule and
SocialApiGuiceModule
are good enough to be used in production environments. Basically, the
stuff set up in there is golden and can be used everywhere.
No, that's the opposite of the point. If it was, then @ImplementedBy
never would have been used, since a large quantity of boilerplate
would just go in those modules, and no one would need to override
them. The existing modules are far from perfect, and can't be used in
production. SocialApiGuiceModule referencing samplecontainer code
immediately disqualifies it. And "DefaultGuiceModule" - which should
be named GadgetsModule - almost certainly shouldn't supply its own
Executor/ExecutorService.
This is where I disagree. I consider the various parts of Shindig
components that are put together by a frame which uses Guice and turn
them into an app.
We e.g. use a very different frame to turn the Shindig components
into
a working opensocial container. And I use the supplied Guice modules
solely as a reference on which components must be
implemented/touched/used. And finding this reference is very hard
with
it being spread out all over the code base.
Guice modules are configuration files. Treat them like this.
In part, yes. But no one should have to set up a large - and, when
@ImplementedBy was introduced, constantly changing! - Guice module
just to use a library, anymore then you'd hand them a couple hundred
lines of config files and say "start from here".
Ciao
Henning
The problem with XStream081Configuration is just a design flaw
and should be
fixed. Isn't there a Guice specific way to identify the proper
implementation class for an interface? This problem crops up in
other
unmarshaling code too, for example JAXB has this same problem
when creating
classes from XML. At least Guice has an explicit binding here,
so I think
it should be possible for us to fix this and use the proper
implementation
classes.
Indeed. The XStream code needs to be fixed. This will be easier in
Guice 2.0, which adds introspection APIs.
On Dec 3, 2008, at 10:08 AM, Henning P. Schmiedehausen wrote:
So we are integrating Shindig into our own code base. Which is
large,
complex and I try to keep a few personal guidelines that go
against
the "current industry trends" but 25 years of computer programming
should be good for something. :-) One of them is "everything that
happens automatically is bound to break". Always.
What we want to do is provide our own implementations of some
opensocial model classes. There is a reason for this, mainly
because
we have more fields in compliance with
http://www.opensocial.org/Technical-Resources/opensocial-spec-v081#TOC-Compliance
using <prefix>.specialField. Which are represented by additional
fields in our objects.
(Which BTW leads the whole idea of using getter/setter based bean
classes for Shindig ad absurdum. The current model classes clamp
down
exactly what is in the spec without allowing the dynamic extension
mechanisms shown there. Using Enums is equally bad, because
extending
these leads to necessary Shindig code modifications. But let's
not go
there).
And it breaks. It breaks in a very puzzling way. Because at some
point, the PersonImpl classes started to bleed into our code, even
though our custom GuiceModule contains
bind(Person.class).to(MyCustomPerson.class);
The reason for this is the following piece of code in
InterfaceClassMapper. A class that can only be replaced by
ripping out
the whole XStream081Configuration class, because it is hard
coded in
there.
--- cut ---
private Class<?> getImplementation(Class<?> clazz) {
// get the guice default implementation on the class.
// we might use the injector to discover this
Class<?> cl = clazz;
ImplementedBy implementedBy =
clazz.getAnnotation(ImplementedBy.class);
if (implementedBy != null) {
Class<?> c = implementedBy.value();
if (log.isDebugEnabled()) {
log.debug("===================Class " + clazz + " is
implemented by "
+ c);
}
cl = c;
} else {
if (log.isDebugEnabled()) {
log.debug("===================Class " + clazz
+ " no implementation, assume concrete ");
}
}
return cl;
}
--- cut ---
This method *TIES* the @ImplementedBy implementations of the model
interfaces to the interfaces. And you can not override this
without
recompiling Shindig. So why use IoC at all? And argueing that this
*helps* implementors to understand how Shindig works is simply
wrong. This is error-prone, difficult to read and understand and
the
current code structure makes it *harder* for implementors to
understand where they can and must tweak the code base for
integrating
it into their project.
So I beg to differ to Ian: @ImplementedBy makes the code base
harder
to read and understand. Because there is no centralized point of
entry
where the parts are tied together. Which was (up until now) my
understanding why people are using IoC containers in the first
place.
Why you have Modules in Guice and config files in Spring. But
that is
just me.
Ciao
Henning
--
Henning P. Schmiedehausen - Palo Alto, California, U.S.A.
[EMAIL PROTECTED] "We're Germans and we use Unix.
[EMAIL PROTECTED] That's a combination of two
demographic groups
known to have no sense of humour
whatsoever."
-- Hanno Mueller,
de.comp.os.unix.programming
Paul Lindner
[EMAIL PROTECTED]
--
Henning P. Schmiedehausen - Palo Alto, California, U.S.A.
[EMAIL PROTECTED] "We're Germans and we use Unix.
[EMAIL PROTECTED] That's a combination of two demographic
groups
known to have no sense of humour
whatsoever."
-- Hanno Mueller,
de.comp.os.unix.programming
Paul Lindner
[EMAIL PROTECTED]
