Have schemas.is_list_view recognise RetrieveAPIView subclasses

[carltongibson]

If you create a RetrieveAPIView with a fixed URL (using request.user in get_object(), say) then rest_framwork.schemas.is_list_view will return True leasing to schemas with list as the key, where retrieve is desired.

It would be good if is_list_view could recognise this kind of instance.

[matteius]

@carltongibson Are you already looking at the fix for this one? I was thinking the description made sense and I could try to setup a test case for it and then possibly a fix ...

[carltongibson]

@matteius I'm very happy for you to have a look at it. 🙂

[matteius]

@carltongibson So I was looking at the code and it seemed your case was falling into the final branch on whether or not the URL ends with a variable. I tried to make the logic more concrete to start that if the view inherits from a ListModelMixin that its a list style and if not when its a RetrieveApiView then its not a list style. That probably doesn't cover every case but gets us further, especially for model views. Let me know what you think of this and if you can think of some additional cases to cover. My next step would be to add test coverage of this change.

• matteius@421d546

[carltongibson]

@matteius I think that would work for my use-case.

I'm not sure I particularly like the direction of the logic though... it seems to me that the answer to is_list_view should reside with the view itself—which is surely the relevant expert—rather than in an ever more baroque introspection function, ever expanded to handle more edge-cases. (This is more general observation than specific feedback on your change.)

[matteius]

I guess mine is a slightly different take on it. Having worked with this api framework for many different internal APIs a common learning is that people sub-class from the wrong primitive view types. The fact is ListModelMixin is an interface for looking up a list of things, and if you inherit your view from it (directly or indirectly) then you should both support a list retrieval and the scehama should reflect this. the generic RetrieveAPIView is also an interface for expressing a view that looks up a singular item.

In my opinion both of these checks are more concrete than logic I left come after it which introspect the URL to see if it ends with a variable. To me that is definitely an assumption that ending in a variable means specific item lookup vs otherwise imply a list. To me, the classes the view inherits from are the most internal thing about the view itself and definitely resides within it.

[carltongibson]

@matteius No (or "Yes"? 🙂) , in this case I agree. The check is fine.

My issue is more with the "top-down" nature of the schema generation: this leaves the generator (and helper classes) needing to know too much, and quite inflexible, I'm finding. I don't (yet) have a solution for that. Your fix may be appropriate here, but I think it hints at a smell.

[matteius]

@carltongibson Hmmm, I so wrote some test cases to try and show this was working, and ended up showing its not working. I am not sure why yet, but the isinstance check isn't detecting the test view as being of that type. Here are my tests: matteius@01297b3

[carltongibson]

@matteius — errr... 🙂

I guess, how about opening a PR for this: as a small improvement I think it's probably worth suggesting — getting list for a retrieve view isn't great.

We can look at the isinstance check and see what's going on there.