CodingAleCR / http_interceptor

A lightweight, simple plugin that allows you to intercept request and response objects and modify them if desired.
MIT License
133 stars 67 forks source link

Refactor Interception logic to use base classes and be compatible with all request types #98

Closed lukaskurz closed 2 years ago

lukaskurz commented 2 years ago

Hi again :sweat_smile:

This PR is closely related to #94, being an alternative approach to solve the same problem.

Most of what i did and the Inspiration for this is already in #94 and in this comment.

TLDR;

The code uses the Request and Response class in the interception logic, which then gets wrapped in a RequestData and ResponseData class. Request and Response are used for specific types of requests and only extend the base class used inside the http logic. This is results in incompatability of the other types of requests, such as:

I just replaced the current usages with the base classes, which enables compatability with all request types. The one downside to this is that you have to check or cast the base classes to the type of request that you are intercepting, in order to manipulate it (such as changing the body of a Response).

To make this easier, I included copyWith extension methods for all requests and responses

CodingAleCR commented 2 years ago

I reaaaally like your approach here, it does feel right. Now, that said, I have a couple of thoughts around it:

Uri's property of queryParameters is actually an unmodifiable map (An exception is thrown in the example: Unsupported operation: Cannot modify unmodifiable map). That is the main reason for RequestData and ResponseData to exist. Now I don't quite like having the custom classes, like I said, your approach feels more natural, so, my suggestion would be to add an extension to clone the complete Uri class, that way we can still use the copyWith methods to clone requests and responses.

The reason why this is not noted in the tests is because the tests available at the moment only cover the model classes and not the complete library, which is a pain when implementing code that will/might break other stuff.

Anyway, in the meantime I am thinking of other options to solve this, so feel free to take a stab at cloning Uri or leave the PR here and I'll work on it when I get a bit of time soon.

I had completely forgotten about the AddParameters extension already built-in. This is great! I'll probably do some more testing but I'm loving the new API.

CodingAleCR commented 2 years ago

@all-contributors add @lukaskurz for tests, ideas and code

allcontributors[bot] commented 2 years ago

@CodingAleCR

I've put up a pull request to add @lukaskurz! :tada: