HAL List Serializer

[claytondaley]

This PR adds HAL-compliant list serialization to HalModelSerializers:

• Add HalListSerializer (inherits from ListSerializer) to generate HAL-compliant output.
  • this only happens in calls to .data to avoid poisoning list of related objects
• Update HalModelSerializer to use HalListSerializer by default (__new__ calls many_init so the change is found here)

This PR is not as clean as I'd like:

• Unfortunately, the default list serializer is hard coded in BaseSerializer.many_init so I had to conditionally inject the new default in Meta.list_serializer_class.
  • Ideally, the default_list_serializer logic (or something similar) would be pushed up to DRF's BaseSerializer so we could just set a new default in this class.
• There is no existing infrastructure for resolving the list view for a model
  • HalListSerializer.build_view_url parrots the approach found in DRF (smeared over a bunch of helper functions)
  • I introduced a new Meta attribute (i.e. base_name) to support manual override
  • DRF might accept base_name (or something similar) since it would support both -list and -detail.

I'm not going to actually suggest anything to DRF until the collaborators here are satisfied with the approach.

EDIT: Updated to reflect latest code.

[houndci-bot]

'rest_framework' imported but unused

[claytondaley]

Sorry. Had a face-palm moment when I realized that data and to_representation distinguished top-level from related usage.

[jstaffans]

Thanks for this!

We implemented lists of links a while ago, but skipped over embedding them, in order to not blow up JSON responses too much for the default case. That's why there is a test serializer which uses many=True, but does not embed anything. I think the user should generally have to opt-in to embedding lists of related resources.

It would be interesting to hear if this PR somehow changes that, or if it's only about list-type fields that are part of a resource and not "related" to it.

A few test cases would help very much for understanding this PR!

[claytondaley]

The current state of this PR only affects the top-level representation of a collection endpoint (e.g. object-list or /objects). The PR doesn't affect nested/related lists -- probably for the same reason you state.

Here's the outline of the "conceptual" test case that fails in the current codebase:

• Some users can create books and others cannot
• A pure-HAL UI should be able to deduce which users can create books from the _links section. Specifically, the backend creates an entry (e.g. create) for users who are permitted to take that action.
• However, DRF's ListSerializer returns a data structure like this (note the top-level list):

  [{
      "_links": {
          "self": {
              "href": "/library/Fight-Club"
          }
      },
      "author": "Chuck Palahniuk",
      "title":  "Fight Club"
  }, ... ]
  

• While a _links section exists, it's actually located on the individual objects in the list. It's philosophically the wrong place for a create entry. The reason for this is easily demonstrated by the initial state (no books):

  []
  

I actually discovered the issue by looking at examples of HAL-compliant top-level collections (exemplified by this SO answer). Even for an un-paged collection, they indicate a barebones format like:

{
    "_links": {
        "self": {
            "href": "/library"
        }
    },
    "_embedded": {
        "item": [{
            "_links": {
                "self": {
                    "href": "/library/Fight-Club"
                }
            },
            "author": "Chuck Palahniuk",
            "title":  "Fight Club"
        }, ... ]
    }
}

Obviously, this structure includes a _links section where a create link could be included. The PR ensures that collections based on the HalModelSerializer are rendered with a HAL-compliant top-level structure (i.e. a _links section with a self link and an embedded section where objects are found).

[claytondaley]

I'll try to find time to add test cases as well. The test harness is... less unittest than I'm used... to so there's a non-trivial learning curve there too.

[jstaffans]

Yeah, the test harness is one of the things that is still implemented as it was in the original repo. We'll try to clean it up when time permits..

To your ListSerializer issue: what if you just use HalModelSerializer? If you run the "abundant resources" test case, the response is rendered along the lines of HAL:

{
  "_links": {
    "self": {
      "href": "http://testserver/abundant-resources/"
    },
    "next": {
      "href": "http://testserver/abundant-resources/?cursor=cD0yMDE3LTA4LTE0KzIwJTNBMzQlM0E1NS4wMDcxNzU%3D"
    }
  },
  "_embedded": {
    "items": [
      {
        "_links": {
          "self": {
            "href": "http://testserver/abundant-resources/50/"
          }
        },
        "name": "Abundant Resource 49"
      },
      ...
   }
}

Does this solve your issue?

[claytondaley]

I don't think so. In DRF:

1. A single item is returned as a dict
2. A paginated collection is (usually) returned as a dict (to support prev and next)
3. An unpaginated collection is returned as a list

Because your test case includes "next", I'm pretty sure it's an example of (2). My PR is intended to address (3).

FWIW I dislike that DRF behaves this way. I think it's confusing that adding pagination requires a rewrite of the UI, but I've reported far more serious violations of REST semantics that are still unresolved. I know better than to tilt with that DRF windmill, but this package should definitely conform to spec.

edit: if prev/next links are placed in headers (not the default), the body of a paginated DRF response may be rendered as a list as well

[jstaffans]

Gotcha, I agree that the response should always be a dict. If you take a look at the test case again, though, the first assertion is for a resource that is not paginated (no prev or next links). This also renders as a dict.

I could however reproduce your issue by turning off pagination for the test project completely, in settings.py: 'PAGE_SIZE' = None. I imagine the test case for this needs to override the project settings.

[claytondaley]

Yes. I think "no pages" is testing the case where all of the items fit on one page. The resource is still paginated (technically speaking) but no links are rendered. The = None case is the truly unpaginated resource example that falls back to DRF's inconsistent behavior.

[ambsw-technology]

Test case added.

[jstaffans]

Sorry for the late approval, great work!

[claytondaley]

Or should I be using (e.g. here):

{'href': self.request.build_absolute_uri()}

[jstaffans]

I think we need to build URI:s the same way everywhere, so I would go with self.request.build_absolute_uri(), if that is enough for your use case.

[claytondaley]

It certainly looks like it should work (and eliminates the need for some extra code). At some point, we're going to need to update that code to address #27 but it's not urgent/blocker. I'll push the change.