Remove unwanted URL registrations from related endpoints #369
Conversation
| response = admin_api_client.get(url, data={}) | ||
| else: | ||
| response = admin_api_client.get(url) | ||
| assert response.status_code == 404 |
There was a problem hiding this comment.
All of these tests were failing before this patch.
| @@ -186,12 +174,33 @@ def test_association_router_related_viewset_all_mapings(db): | |||
| expected_urls = [ | |||
| 'my_test_basename-detail', | |||
| 'my_test_basename-list', | |||
| 'my_test_basename-teams-detail', | |||
There was a problem hiding this comment.
I'm not going to lie, this kind of test assertion is a little bit upsetting. This URL should not be registered, but someone probably added it in just to make the test pass.
|
| @@ -159,6 +162,11 @@ def get_serializer_class(self): | |||
| return super().get_serializer_class() | |||
|
|
|||
|
|
|||
| @property | |||
| def attribute_raiser(cls): | |||
| raise AttributeError | |||
There was a problem hiding this comment.
Is an Attribute Error correct for these? Shouldn't this return a MethodNotAllowed response?
There was a problem hiding this comment.
Not a 405, a 404. See title, unwanted URL registrations, meaning endpoints that shouldn't exist were giving 500 errors. These are endpoints that no method is valid for. If you have the wrong method (GET/POST) but another method would have worked, that's a 405, but that's not what this is fixing. This is how we get that behavior. This is guided missile to programmed to a specific hasattr in SimpleRouter.get_method_map:
bound_methods = {}
for method, action in method_map.items():
if hasattr(viewset, action):
bound_methods[method] = action
return bound_methodsI skipped ahead without explaining this in great detail. But this is the way viewsets work, if a method is defined on a viewset, then that's a URL. That method can have multiple valid methods. I need to remove the retrieve method and others. If left in, these would create "phantom" endpoints, as in, a detail endpoint nested in the association endpoint, because it got left in the association viewset. So we need to leave it in the original (parent) viewset (because it is used for other things) and remove it as we subclass it.
The unorthodox thing is that viewset is a class, and we need a subclass that gives False for hasattr(viewset, 'retrieve'). That is what AttributeError does. It's a method-scrubber, isolated to the new subclass.
|
Other PR is so good I think I want that instead. #370 |
Fixes #275
I finally hit a wall with this trying to add related
resourcefields to the user/team association endpoints. I went into http://127.0.0.1:8000/api/v1/docs/#/role_team_assignments/role_team_assignments_create and it had duplicate view names. This was caused by the same problem as 275. Random views were unintentionally getting registered.So I had to bite the bullet and fix this. This bug was... kind of bad already. Those custom actions look concerning. It is true that the lookup field probably makes it unusable, but this isn't a good thing to rely on.