Hi all:
While working on the katello related auth bits, I got annoyed by having to throw UnsupportedOperationException from all of the methods, and started looking at how we might refactor to replace those with multiple interfaces for our adapters.
In summary, the UserServiceAdapter interface defines a number of calls to perform CRUD operations on users (and roles). The default adapter, which stores users in candlepin's db, implements all of the methods. The katello adapter implements _none_ of them (all of the auth logic is handled in katello, before a call reaches candlepin). The adapter used by rhn.redhat.com does not implement creation/deletion. Any unimplemented calls throw UnsupportedOperationExceptions. Other adapters follow the same pattern (in particular, the ProductServiceAdapter).
Current practice is to have a caller be aware of which methods might not be implemented, and have them catch the exception and notify the api user as appropriate. Not all callers do this; its understandably hard to keep track of this convention's usage. In actual deployments, the REST endpoints that expose apis which hit these conditionally implemented methods are masked off via proxies, or responded to by katello, etc. If your deployment does not have an adapter that responds to the create user call, you don't expose POST /candlepin/users.
For a POC, i've taken the ProductServiceAdapter and ripped out all of the methods that wouldn't be in the rhn.redhat.com adapter into a new interface, WriteableProductServiceAdapter (I love the name, too). In the main code base, the DefaultProductServiceAdapter just implements both interfaces.
Going further up the stack, REST operations that use the new WriteableProductServiceAdapter calls get moved to WriteableProductResource (but are still exposed under /products, at the same urls).
Lastly, the guice calls to bind() both the new interface and the new resource are put into their own guice module, StandaloneConfig, which is not read by default at candlepin startup (you must tell candlepin to load it via the module.config config directive). If StandaloneConfig is read at startup, then candlepin will respond to product creation rest calls; if not, then the client will get a 404.
My hope is that we can do the same for all adapters: pull out any calls that are commonly not implemented, put them under their own interface, and extract out a new resource. For each deployment style, we can then write a new guice module that will be responsible for loading the implementations of all adapters they use, and the common resource frontends as applicable (to make it clear, WriteableProductResource would be the same code across all deployment types, it might just not be used at all, if they don't implement the required adapter).
Please have a look at the adaptergate branch and let me know what you think of this approach. Is it uglier than the current style? Do we want to change the exposed urls or have them always exist, but just throw errors?
Lastly, even if we don't use this, it's kind of a neat POC of how we could do 'plugins' to provide api extensions.
-James
On Fri, Jul 22, 2011 at 10:07 AM, James Bowes jbowes@redhat.com wrote:
Hi all:
While working on the katello related auth bits, I got annoyed by having to throw UnsupportedOperationException from all of the methods, and started looking at how we might refactor to replace those with multiple interfaces for our adapters.
In summary, the UserServiceAdapter interface defines a number of calls to perform CRUD operations on users (and roles). The default adapter, which stores users in candlepin's db, implements all of the methods. The katello adapter implements _none_ of them (all of the auth logic is handled in katello, before a call reaches candlepin). The adapter used by rhn.redhat.com does not implement creation/deletion. Any unimplemented calls throw UnsupportedOperationExceptions. Other adapters follow the same pattern (in particular, the ProductServiceAdapter).
Current practice is to have a caller be aware of which methods might not be implemented, and have them catch the exception and notify the api user as appropriate. Not all callers do this; its understandably hard to keep track of this convention's usage. In actual deployments, the REST endpoints that expose apis which hit these conditionally implemented methods are masked off via proxies, or responded to by katello, etc. If your deployment does not have an adapter that responds to the create user call, you don't expose POST /candlepin/users.
For a POC, i've taken the ProductServiceAdapter and ripped out all of the methods that wouldn't be in the rhn.redhat.com adapter into a new interface, WriteableProductServiceAdapter (I love the name, too). In the main code base, the DefaultProductServiceAdapter just implements both interfaces.
Going further up the stack, REST operations that use the new WriteableProductServiceAdapter calls get moved to WriteableProductResource (but are still exposed under /products, at the same urls).
Lastly, the guice calls to bind() both the new interface and the new resource are put into their own guice module, StandaloneConfig, which is not read by default at candlepin startup (you must tell candlepin to load it via the module.config config directive). If StandaloneConfig is read at startup, then candlepin will respond to product creation rest calls; if not, then the client will get a 404.
My hope is that we can do the same for all adapters: pull out any calls that are commonly not implemented, put them under their own interface, and extract out a new resource. For each deployment style, we can then write a new guice module that will be responsible for loading the implementations of all adapters they use, and the common resource frontends as applicable (to make it clear, WriteableProductResource would be the same code across all deployment types, it might just not be used at all, if they don't implement the required adapter).
Please have a look at the adaptergate branch and let me know what you think of this approach. Is it uglier than the current style? Do we want to change the exposed urls or have them always exist, but just throw errors?
Lastly, even if we don't use this, it's kind of a neat POC of how we could do 'plugins' to provide api extensions.
-James
Unsupported API calls disappearing completely, I quite like that. Assuming the hosted dudes don't mind a little refactoring to sync up with it this looks great to me.
candlepin@lists.stg.fedorahosted.org
-
Devan Goodwin -
James Bowes