Code review comment for lp:~jameinel/juju-core/api-register-standard-facade

Reviewers: mp+219660_code.launchpad.net,

Message:
Please take a look.

Description:
state/apiserver/common: RegisterStandardFacade

So my previous patch introduced a Registry that can be used for
registering and then looking up Facades. However, almost all of our
existing code has methods like:
   func NewKeyManagerAPI(
       st *state.State,
       resources *common.Resources,
       authorizer common.Authorizer,
   ) (*KeyManagerAPI, error) {

And that has the property that it is != o
   type FacadeFactory func(
       st *state.State, resources *Resources, authorizer Authorizer, id
string,
   ) (interface{}, error)

Because the return type for the latter is interface{} and the former is
a concrete type (that implements that interface, of course).

I could think of 3 ways to work around this, and this seemed the most
tasteful.

1) Rewrite all of the New*API() functions to return interface{}, and
then rewrite all of their tests to cast it back to the concrete type. I
like that New* returns a concrete type, though, so I'd rather keep that.

2) Wrap all of the calls to RegisterFacade with a bespoke func() that
just does the type currying, then all of the init() functions looked
like:

   func init() {
     common.RegisterFacade("KeyManager", 0, func(
           st *state.State, resources *Resources, authorizer Authorizer,
id string,
    ) (interface{}, error) {
     if id != "" {
       return nil, common.ErrBadId
     }
     return NewKeyManagerAPI(st, resources, authorizer), nil
    })
   }

That isn't terrible, but it is a lot of boilerplate to write.

3) Use reflect to create the boilerplate functions. Then all of the
actual init lines just become:

   func init() {
       common.RegisterStandardFacade("KeyManager", 0, NewKeyManagerAPI)
   }

So at the cost of having RegisterStandardFacade have to do reflect
introspection, it gives us really nice syntax when we want to add more.
(And note that when we have multiple versions of Facades, (2) would have
to have a bespoke func() for each one.)

I could go back to (2) if people don't like the reflect magic, but I
think it does make things nicer overall.

https://code.launchpad.net/~jameinel/juju-core/api-register-standard-facade/+merge/219660

Requires:
https://code.launchpad.net/~jameinel/juju-core/api-facade-registry/+merge/219654

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/99290043/

Affected files (+211, -0 lines):
   A [revision details]
   A state/apiserver/common/export_test.go
   M state/apiserver/common/registry.go
   M state/apiserver/common/registry_test.go