Code review comment for lp:~leonardr/lazr.restful/generate-multiversion-collections

Revision history for this message
Leonard Richardson (leonardr) wrote :

This branch makes the @collection_default_content annotation
version-aware. For different versions of the web service you can
specify different methods to be used as the collection's find()
implementation, and different values for that method's keyword
arguments.

I was able to refactor some code because this is the second branch
that makes our code generation version-aware. I also simplified the
implementation of register_adapter_for_version. Previously if the
version in question was None (indicating the earliest version), I
looked up which version was the earliest and then looked up its
version-specific marker interface. Now I simply use the generic web
service request interface, IWebServiceClientRequest. This prevents
test failures where all that changed is that a lookup that failed with
IWebServiceClientRequest needed to be changed to
IWebServiceBetaClientRequest.

Here are some questions I asked myself during development. They all had to do
with whether version differences were better solved with annotations
or left to navigation code.

Q: Do I need an analogue to @operation_removed_in_version?
A: No. If a collection disappears in a certain version, it needs to
   disappear from the navigation. Hiding the
   collection_default_content for that version won't solve anything.

   The reason we need @operation_removed_in_version is that lookup of
   named operation happens after the navigation is done. The
   navigation takes you to an entry or collection, and then the named
   operation lookup needs to succeed or fail depending on the version.

   If it turns out to be too much trouble to change the navigation, we
   could create a @collection_removed_in_version and make the
   lazr.restful behavior to raise a 404 instead of calling find(), but
   for now I say YAGNI.

Q: What happens if a collection changes its name between versions?
A: That, too is a navigation issue. And again, we had to deal with it
   specially for named operations because operation lookup is not
   navigation.

Q: What happens if a collection doesn't have a
   @collection_default_content for the earliest version? Don't I
   need to prevent that?
A: That corresponds to a case where the collection didn't exist in the
   earliest version and was added later. This is a legitimate case,
   and it will work fine if accompanied by appropriate navigation
   code.

« Back to merge proposal