SerializerMethodResourceRelatedField fails with queryset and many kwargs

[glowka]

Three cases below are OK, but the fourth one is surprisingly crashing

read_only is generally ok and non-read_only without many is also ok

SerializerMethodResourceRelatedField(
    model=Member, source='get_member', read_only=True
)

SerializerMethodResourceRelatedField(
    model=Member, source='get_members', read_only=True, many=True
)

SerializerMethodResourceRelatedField(
   queryset=Member.objects.all(), source='get_member'
)

But non-read_only with many crashes

relations.SerializerMethodResourceRelatedField(
    queryset=Member.objects.all(), source='get_members', many=True
)

Message:

tests/test_serializer_schema.py:388: in <module>
    source='get_members', many=True),
/site-packages/rest_framework_json_api/relations.py:361: in __new__
    return cls.many_init(*args, **kwargs)
/site-packages/rest_framework_json_api/relations.py:379: in many_init
    return cls(**list_kwargs)
/site-packages/rest_framework_json_api/relations.py:370: in __init__
    super(SerializerMethodResourceRelatedField, self).__init__(*args, **kwargs)
/site-packages/rest_framework_json_api/relations.py:192: in __init__
    super(ResourceRelatedField, self).__init__(**kwargs)
/site-packages/rest_framework_json_api/relations.py:76: in __init__
    super(HyperlinkedMixin, self).__init__(**kwargs)
/site-packages/rest_framework/relations.py:247: in __init__
    super().__init__(**kwargs)
/site-packages/rest_framework/relations.py:108: in __init__
    'Relational field must provide a `queryset` argument, '
E   AssertionError: Relational field must provide a `queryset` argument, 
override `get_queryset`, or set read_only=`True`

It looks like queryset is not passed correctly along the initialization.

My interpretation is that is probably due to a bit hacky implementation of SerializerMethodResourceRelatedField which tries to by SerializerMethodResourceRelatedField and ManySerializerMethodResourceRelatedField at the same time which looks to me prone to bugs like this.

So I would like to report this issue with SerializerMethodResourceRelatedField in the first place.

Additionally, if my perception of reasons of problems with SerializerMethodResourceRelatedField is more widely accepted I could refactor it and fix the bug at the same time.

[sliverc]

The SerializerMethodResourceRelatedField follows the idea of the DRF SerializerMethodField so it should actually always be read only and applying a queryset doesn't really make sense therefore. This means to address this issue a read_only flag should be passed on to base class.

This field class does have other issues though like #639 so if you want to work on a PR which addresses these issues you are most welcome. Just keep in mind backwards compatibility respectively a deprecation path when refactoring.

[glowka]

Thanks for immediate response.

The SerializerMethodResourceRelatedField follows the idea of the DRF SerializerMethodField so it should actually always be read only and applying a queryset doesn't really make sense therefore. This means to address this issue a read_only flag should be passed on to base class.

OK, that makes sense now. Because of issues raised in #639 as well as no read_only enforcement (as in DRF SerializerMethodField ) I had not recognized it followed the same idea, actually when I realized the arguments (#639) worked in different manner I thought it might be implemented with a bit different approach. Hence the issue.

This field class does have other issues though like #639 so if you want to work on a PR which addresses these issues you are most welcome. Just keep in mind backwards compatibility respectively a deprecation path when refactoring.

I can't sign up for that now, but I feel the pain of #639, so in coming weeks I'll seriously consider doing that.