Exception when using a regex for a related field #258
Comments
|
hi @AlexDaniel, i suspect this somehow related to your serializer. we do similar things a lot and it has not produced any problems yet. also this is covered in the tests. i would need more information to reproduce this. |
|
and my guess is the regex plays no role here |
|
@tfranzel thanks! That's reassuring. However, my serializer isn't really that special either: Although… wait, which serializer are we talking about exactly? Could it be that my custom user model is affecting it? But there is no other serializer that can be relevant here, so where exactly should I be looking at? I'll try to provide a full example if I have time. |
|
that is really strange. it could be the model, but i highly doubt it. we have a custom user model too and it works fine. this is the test i have written with your example # tests/test_regressions.py
def test_foo(no_warnings):
# custom user
class M3(models.Model):
pass # pragma: no cover
# Foo
class M4(models.Model):
field = models.ForeignKey(M3, on_delete=models.PROTECT, editable=False)
class M4Serializer(serializers.ModelSerializer):
class Meta:
fields = '__all__'
model = M4
class XViewset(viewsets.ModelViewSet):
serializer_class = M4Serializer
queryset = M4.objects.none()
# should throw because of the bug
schema = generate_schema('x', XViewset)so this should be your example but it works. we need more context to find the problem. ideally you would create a reproduction with this snippet. |
|
Good! I'll try the snippet as soon as I wake up. However, something seems to be missing in your snippet? Did I already mention that this: router.register(r'user/(?P<user_id>[0-9a-f-]{36})/foo', mystuff.views.FooViewSet)Throws an exception like I have shown above. But this: router.register(r'user/(?P<blahblah>[0-9a-f-]{36})/foo', mystuff.views.FooViewSet)doesn't. I'm not entirely sure where in your example you're actually using your field. |
|
sry, the error message threw me off. it is in fact the URL parameter that is causing problems. i have a reproduction. will look into it |
|
@AlexDaniel so i debugged it. it wasn't the regex itself, but the related field resolution for the path variable. there were 4 other ways to do this and you chose the only one that didn't work 😄 i won't even try to explain it, because it's just whacky internal DRF details. please confirm that it works and close. thanks! |
tfranzel
added
bug
Hey, I'm actually wondering, what are the other ways to do it? When it comes to implementing nested paths like this, there are:
But you're saying there are more ways to do it with router.register? For example, I did try using path converters, which avoids the regex and drf-spectacular seems to be picking them up perfectly, but drf itself does not support them: encode/django-rest-framework#6789. |
|
its not what you did, it is how you did it. nesting is fine, but you created very special circumstances for which the code path was broken. these examples would have worked without the fix: router.register(r'user/<uuid:user>/foo', mystuff.views.FooViewSet)OR class M4(models.Model):
# primary_key changes DRF interM4nals that make discovery easier
field = models.ForeignKey(M3, primary_key=True, on_delete=models.PROTECT, editable=False)would have taken different code paths, that would have worked. |
|
@tfranzel I'm like 99% certain that the first snippet does not work. It's about the drf ticket I mentioned earlier. Yes, drf-spectacular will generate the schema perfectly, but that route is unusable in DRF itself. Try making a request to it and it won't match any routes. It could also be that I did something wrong in my code though, but I'm pretty sure I tried real hard to make that work. |
|
ahh sry, you are right. after just reading the ticket i realized that. however, the second one would have most likely worked. there is a small difference between what drf-spectacular and DRF will accept. in some cases like the first one, spectacular will allow more than DRF. well we fixed it so it should now work for you. would be nice if you could confirm that with the master state. |
|
Just installed 0.13 and indeed, it works now! Both Thank you very much! |
In DRF I'm trying to register a path like this (the id in this case is UUID):
This is roughly what I have in the model:
I'm pretty sure it works, but I get an exception when I'm trying to open swagger-ui:
For what it's worth, both
useranduser_idgive an exception.The text was updated successfully, but these errors were encountered: