Sculley
Sculley

Reputation: 33

Django-Filters Empty String Query Param Cause Validation Error

When using django-filters with django rest framework, the default query filtering form will add all query params for all fields, and the empty fields end up as empty strings passed to the backend. Empty strings are not None, so the status = self.request.query_params.get('status', None) will still add the empty string to the variable, and this will make its way to the queryset.filter function. This is very bad when the field being filtered is not a string.

So my question is am I doing something wrong? Is there a way to check for empty string params? I'm not sure if I'm even doing filtering right (I'm probably filtering twice for no reason, but I wanted django-filters in there because of the built in integration with django-rest-frameworks browseable API.

My workaround is the ternary operator checking for the empty string after the call to query_params.get

Below is my view code for the API:

class JobList(generics.ListCreateAPIView):
    serializer_class = JobCreateSerializer
    permission_classes = (permissions.IsAuthenticatedOrReadOnly,)

    filter_backends = (filters.OrderingFilter, DjangoFilterBackend)
    filterset_fields = ('status', 'success', 'worker_id', 'owner', 'job_type')
    ordering_fields = ('priority', 'submitted', 'assigned', 'completed')

    def perform_create(self, serializer):
        serializer.save(owner=self.request.user)

    def get(self, request, format=None):
        # IMPORTANT: use self.get_queryset() any place you want filtering to be enabled
        # for built in filtering or ordering you must call self.filter_queryset
        jobs = self.filter_queryset(self.get_queryset())
        serializer = JobSerializer(jobs, context={'request': request}, many=True)

        return Response(serializer.data)

    def get_queryset(self):
        """
        This view should return a list of all the purchases
        for the currently authenticated user.
        """
        queryset = Job.objects.all()

        status = self.request.query_params.get('status', None)
        status = status if not status == '' else None
        success = self.request.query_params.get('success', None)
        success = success if not success == '' else None
        worker_id = self.request.query_params.get('worker_id', None)
        worker_id = worker_id if not worker_id == '' else None
        owner_id = self.request.query_params.get('owner', None)
        owner_id = owner_id if not owner_id == '' else None
        job_type = self.request.query_params.get('job_type', None)
        job_type = job_type if not job_type == '' else None

        if status is not None:
            queryset = queryset.filter(status=status)
        if success is not None:
            queryset = queryset.filter(success=success)
        if worker_id is not None:
            queryset = queryset.filter(worker_id=worker_id)
        if owner_id is not None:
            queryset = queryset.filter(owner=owner_id)
        if job_type is not None:
            queryset = queryset.filter(job_type=job_type)

        return queryset

Upvotes: 2

Views: 4311

Answers (3)

Sculley
Sculley

Reputation: 33

Thank you for your answers, I think they are valid approaches but they both are doing manual checks. I was looking for a built in way to convert the JSON native types that come in with the query_params to Python native types (false to False, null to None, etc). Hoping to avoid manually checking and converting each query_param.

It actually blows my mind if such a thing isn't available in the Django Rest Framework. So I created a helper function that can do a simple one level deep conversion from query_params to Python dict. I hope this helps someone else, or if anyone knows of a similar convenience function in DRF, I'd appreciate it. Or if someone could tell me why this is a bad approach, I'd appreciate that too!

def query_params_parser(self, fields_list):

        json_values_dict = {}

        for field in fields_list:
            value = '"' + self.request.query_params.get(field) + '"' if not self.query_params.get(field, '') == '' else 'null'
            json_values_dict['"' + field + '"'] = value

        json_string = '{'

        for idx, (key, value) in enumerate(json_values_dict.items()):
            json_string += f'{key}: {value}'

            if idx < len(json_values_dict) - 1:
                json_string += ','

        json_string += '}'

        final_dict = json.loads(json_string)

        return final_dict


def get_queryset(self):
        """
        This view should return a list of all the purchases
        for the currently authenticated user.
        """
        queryset = Job.objects.all()

        # parse json query_params
        # query_params, list of fields
        # returns dict

        params_dict = self.query_params_parser(['status',
                                                'success',
                                                'worker_id',
                                                'owner',
                                                'job_type'])

        status = params_dict['status']
        success = params_dict['success']
        worker_id = params_dict['worker_id']
        owner = params_dict['owner']
        job_type = params_dict['job_type']

        if status is not None:
            queryset = queryset.filter(status=status)
        if success is not None:
            queryset = queryset.filter(success=success)
        if worker_id is not None:
            queryset = queryset.filter(worker_id=worker_id)
        if owner is not None:
            queryset = queryset.filter(owner=owner)
        if job_type is not None:
            queryset = queryset.filter(job_type=job_type)

        return queryset

The above function works under limited testing, so it probably has a lot of bugs.

Upvotes: 0

Endre Both
Endre Both

Reputation: 5740

If you use if status: instead of if status is not None:, both empty strings and None give False. This is dangerous with ints as 0 would return false, but that's not an issue here as all parameters are strings.

You can also define '' as the default, and then check for non-zero-length strings:

status = self.request.query_params.get('status', '')
# ...
if len(status):

Upvotes: 2

wmorrell
wmorrell

Reputation: 5337

Why not set the default to an empty string, and use that as your guard value instead of None?

status = self.request.query_params.get("status", "")
success = self.request.query_params.get("success", "")
# ...
if status:
    queryset = queryset.filter(status=status)
if success:
    queryset = queryset.filter(success=success)
# ...

Upvotes: 1

Related Questions