Fix link generation for extension web API resources

Review Request #3082 — Created April 25, 2012 and submitted

Information

Djblets

Reviewers

This change adds the Web API resources defined in an extension's `resources` attribute to the link tree. Now each resource will be added to the 'links' dictionary of the extension resource, and in the list of extensions. These links will only appear when the extension is enabled.

Also fixed is improper generation of the resource URLs. The URLs were generated with the wrong attribute, causing them to use underscores instead of dashes.
Tested with my new extension's resources, and the extensions with resources found here: https://github.com/mikeconley/RB-Toy-Extensions
Description From Last Updated

Managers, no, but unless an extension has its own ExtensionManager, there will only be one. (Maybe we should add an …

chipx86chipx86

Blank line between these.

chipx86chipx86

And here.

chipx86chipx86

Can we be sure that this will be true for every extension resource? I've written one or two toy extension …

mike_conleymike_conley
chipx86
  1. 
      
  2. djblets/extensions/models.py (Diff revision 1)
     
     
     
     
    Managers, no, but unless an extension has its own ExtensionManager, there will only be one. (Maybe we should add an ID for the ExtensionManager and include that as a field for the extension's model).
    
    As for the extension, the extension ID is the class name (see line 436). So, you can just call get_installed_extension(self.class_name).
    1. I was wondering where the id was set, thanks :D
  3. djblets/extensions/resources.py (Diff revision 1)
     
     
     
    Blank line between these.
  4. djblets/extensions/resources.py (Diff revision 1)
     
     
     
    And here.
  5. 
      
SM
mike_conley
  1. Hey Steven,
    
    Just one question - see below,
    
    -Mike
  2. djblets/extensions/resources.py (Diff revision 2)
     
     
    Can we be sure that this will be true for every extension resource?
    
    I've written one or two toy extension that use PUT and DELETE, and don't respond to GET...
    
    Or am I missing an assumption here?
    1. Thanks for pointing this out, I probably should have explained myself:
      
      I'm hard coding 'GET' here to be consistent with the regular resource tree.
      If you look at the `djblets.webapi.resources.WebAPIResource` class, in
      `get_links()` you will see:
      
          for resource in resources:
              links[resource.name_plural] = {
                  'method': 'GET',
                  'href': '%s%s/' % (clean_base_href, resource.uri_name),
              }
      
      `resources` is an argument (either list_child_resources, or
      item_child_resources). So any child resources are always linked like this,
      even when they don't have a 'GET' method.
      
      At first this seemed like a bug to me, but I figure it's probably done
      this way for a reason. the structure of the links in a resource go
      like this:
      
          "links": {
              "self": { "method": "GET", "href": ... },
              "create": { "method": "POST", "href": ...},
              ...
              "child_resource_name": { "method": "GET", "href": ... },
              ...
          }
      
      "self" always exists, and elements like "create" are there if the method
      is supported. For each child resource, you only get one entry with the
      resources name, and the method is always "GET", even if GET isn't
      supported. Since we can only have a single method, it probably makes sense
      to default it to "GET".
      
      The only other option I can think of is to set some sort of priority, such
      that the highest priority method supported is displayed. The problem with
      this is you can't decide which methods a resource supports without some
      extra knowledge not present in the tree.
      
      Taking all that into consideration, I'd say a good way to deal with
      resources is always have a "GET" method. This ensures the URL for
      the resource will always return something meaningful when traversing the
      tree, and you can discover exactly which methods are supported by looking
      at the resource's links. Even if your resource won't return anything
      meaningful, just use the default GET handler, and it will display the links
      and an empty set for the resources data.
      
      I hope this clears everything up, please let me know how you think I should
      proceed with this.
    2. I wonder what others out there do.
      
      I'm not a huge fan of hard-coding GET, but it may be a necessary evil right now. We *should* be able to detect whether the resource supports GET, though, by looking at allowed_methods.
      
      Perhaps we can check if GET is supported, and if not, change that link to be OPTIONS. No payload to fetch, but OPTIONS would give us the list of methods allowed.
    3. I'm going to drop this for now, and take a look at fixing it in general sometime down the road.
  3. 
      
mike_conley
  1. Ship It!
  2. 
      
chipx86
  1. Ship It!
  2. 
      
SM
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (dfa8116)
Loading...