antonio-war/SwiftyNetworking

@Request Making Unnecessary Requests

Closed this issue · 5 comments

Hey, I noticed that the way your @request property wrapper is working is causing some issues. Currently the way that it works is it is relying on the DynamicProperty protocols update method to trigger the network request. This works, however, update gets called every time a Views body gets redrawn. This means that there may be hundreds of requests happening a second. Caching may help prevent accidental DDOSing of an API; however, it is extremely CPU and Disc intensive. This should be a pretty easy fix, but there are a lot of ways you could go about fixing this, so I figured it would be better to file an issue over fixing it myself!

Hey @Lukas-Simonson thank you for this great find. I'll look into it more today, it is interesting!

I created a temporary solution here. Considering that from the next release the Request property wrapper will have a different interface.

I would be happy to have your opinion, do you think it could be a good solution?

This should fix the issue for some situations; however, I would be wary of how it is handling errors. With the shouldUpdate Boolean, if you receive an error back from the request, you will continue to make requests until you no longer have an error.

I would probably replace the model == nil || error != nil with a check for model == nil && error == nil to prevent infinitely making requests when there is a consistent error.

This also will still make a lot of requests for a device with a slower connection. As update could be called multiple times before you get a response back from the network.

I would merge the fix, but take these items into consideration while you are reworking the @Request interface.

Great point of view, thank you! I will avoid also multiple request in case of slower connection or slower device!

Issue solved, pull request merged, change will be available in 0.5.1 !