silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
720 stars 823 forks source link

`write()` is called unnecessarily on `Member` during login. #10968

Open GuySartorelli opened 11 months ago

GuySartorelli commented 11 months ago

Prior to this patch, Member->afterMemberLoggedIn() did the following things:

This means that every time a Member is considered 'logged in', the member ends up being written to the database 3 times, when only once is necessary.

This wouldn't normally be a particular problem as it normally only happens explicitly when a member logs in via the login form (or at worst, on some random page via the auto-login cookie). However for a site that is protected by Basic Auth (e.g. with BasicAuthMiddleware), every single page loads re-authenticates the Member and therefore requires 3 additional writes.

Options

  1. Simply make registerSuccessfulLogin() and regenerateTempID() not call write(). Developers will need to explicitly call write() after these methods, which allows them to also make additional changes after calling those methods before writing to the DB.
  2. Add a new parameter to those methods to determine whether they should call write() or not (as per the example PR)

Example PR (option 2)

madmatt commented 11 months ago

The particular issue I had in the original ticket was not specifically that Member->write() was called 3 times during login, but that we had a bunch of expensive API calls linked in onBeforeWrite() or onAfterWrite().

The Member record was considered 'dirty' each time, which mean it went through the full ->write() codepath every time and physically wrote changes to the DB, even though with two of those calls, nothing on the record actually changed. I dunno how that affects the onBefore/onAfter calls - but yeah, thought that extra info might help.