colymba / silverstripe-restfulapi

SilverStripe RESTful API with a default JSON serializer.
BSD 3-Clause "New" or "Revised" License
64 stars 34 forks source link

do not record login event for every tokenAuth #79

Closed oetiker closed 7 years ago

oetiker commented 7 years ago

doing a full login is an expensive process in silverstripe ... so if we go for token auth, let's have some performance benefit as well .. for single record ops this can give us 300% more performance.

bummzack commented 7 years ago

Do you think that a 300% speedup on logging in is worth ditching the SilverStripe login hooks and/or the login counter? People might rely on them?

I think the whole authenticator is also missing a canLogIn check somewhere… but I guess that's another issue :)

oetiker commented 7 years ago

Since I have to issue about 800 PUTs in a row it does make a difference :) I can't imagine the Admin UI does a full login for each request when running in AJAX mode, or else usability would be very bad ... at least in my setup, a login takes about 600ms. But maybe there is a better way.

I spent some time analyzing ss3 with the xdebug profiler ... I better had not done this OMG ...

bummzack commented 7 years ago

I think the admin UI just uses the session to authenticate the user. Eg. pass the session cookie with the AJAX request to the backend.

With a token, that's basically the same approach, so you're right. Running through all the login hooks might even be counter-productive, as this would generate a login for every request to the REST API…

oetiker commented 7 years ago

it does do a full login with all the trimmings, that is why it is so slow ... it also sets the session cookie which might be suboptimal too

colymba commented 7 years ago

👍 for skipping the login. and if it's a performance boost even better. thanks @oetiker if we need to track/count logins through the api, I had rather we find another approach that the default logIn() method...