Bishwas-py / djapy

No bullshit, Django Rest API Framework
https://djapy.io
61 stars 3 forks source link

Fix: Avoid reparsing data when actual schema type is returned #25

Open Microturbine opened 5 months ago

Microturbine commented 5 months ago

Add type checking for response_data in accordance with schema_dict_returned

15

Bishwas-py commented 5 months ago

Have you tested this code on your end?

Bishwas-py commented 5 months ago

In this case isinstance(response_data, schema_dict_returned), the response_data might just be an schema object, which won't be an parsable response for django. The data should be wrapped by HttpResponse or JsonResponse.

Microturbine commented 5 months ago

Have you tested this code on your end?

What a mistake, I was testing with the file before cutting the branch with add#5. I'll check it out, but I haven't touched python much, so it may take some time and assistance.

Bishwas-py commented 5 months ago

@Microturbine sure, anytime.

Microturbine commented 5 months ago

In this case isinstance(response_data, schema_dict_returned), the response_data might just be an schema object, which won't be an parsable response for django. The data should be wrapped by HttpResponse or JsonResponse.

Sorry if I am missing the point, If we simply add the process of converting response_data to an HttpResponse or JsonResponse object

if isinstance(response_data, schema_dict_returned): 
    return JsonResponse(response_data)

but change it according to the type of response_data,

if isinstance(response_data, schema_dict_returned): 
    if isinstance(response_data, (HttpResponse, JsonResponse)):  
        return response_data 
    else: return JsonResponse(response_data, JsonResponse)
        return JsonResponse(response_data)

Is it correct to write

Bishwas-py commented 5 months ago

Yes, that is pretty much if it, but:

response_from_view_func = view_func(request, *args, **_input_data) # for httpresponse and jsonresponse, we are doing this
if issubclass(response_from_view_func.__class__, HttpResponseBase):
    return response_from_view_func

status_code, response_data = response_from_view_func \
    if isinstance(response_from_view_func, tuple) else (200, response_from_view_func)

So, you might wanna write:

if isinstance(response_data, schema_dict_returned.get(status_code)):
    parsed_data = response_data.model_dump(mode="json", by_alias=True)
else:
    parser = ResponseDataParser(status_code, response_data, schema_dict_returned, request, _input_data)
    parsed_data = parser.parse_response_data()

response.status_code = status_code
response.content = json.dumps(parsed_data, cls=DjangoJSONEncoder)

Please test the code. I am just pasting an idea over here. Thanks :)

Microturbine commented 5 months ago

Yes, that is pretty much if it, but:

response_from_view_func = view_func(request, *args, **_input_data) # for httpresponse and jsonresponse, we are doing this
if issubclass(response_from_view_func.__class__, HttpResponseBase):
    return response_from_view_func

status_code, response_data = response_from_view_func \
    if isinstance(response_from_view_func, tuple) else (200, response_from_view_func)

So, you might wanna write:

if isinstance(response_data, schema_dict_returned.get(status_code)):
    parsed_data = response_data.model_dump(mode="json", by_alias=True)
else:
    parser = ResponseDataParser(status_code, response_data, schema_dict_returned, request, _input_data)
    parsed_data = parser.parse_response_data()

response.status_code = status_code
response.content = json.dumps(parsed_data, cls=DjangoJSONEncoder)

Please test the code. I am just pasting an idea over here. Thanks :)

Sorry for my low level, but I can't make a good test method. Could you please tell me how to test it even for my later study? Sorry for the inconvenience.

Microturbine commented 5 months ago

It seems that the Django settings I used to have on my computer were still in place and were causing problems. One step forward.

Microturbine commented 5 months ago

Changed #25 Did I upload it successfully? This code didn't cause any problems in my environment.

Bishwas-py commented 1 month ago
                if isinstance(response_data, schema_dict_returned.get(status_code)):
                    parsed_data = response_data.model_dump(mode="json", by_alias=True)
                    print("Parsed")
                else:
                    parser = ResponseDataParser(status_code, response_data, schema_dict_returned, request, _input_data) # execution 1
                    parsed_data = parser.parse_response_data() # execution 2.1

                response.status_code = status_code
                response.content = json.dumps(parsed_data, cls=DjangoJSONEncoder)

                print("Re parsed")
                parser = ResponseDataParser(status_code, response_data, schema_dict_returned, request, _input_data) # execution 2
                parsed_data = parser.parse_response_data() # execution 2.2

You can see things are being repeated.

image

Bishwas-py commented 1 month ago

Try removing from line 247-251 in core/dec.py:

                response.status_code = status_code
                response.content = json.dumps(parsed_data, cls=DjangoJSONEncoder)

                parser = ResponseDataParser(status_code, response_data, schema_dict_returned, request, _input_data)
                parsed_data = parser.parse_response_data()