Page MenuHomePhorge

Remove unused save() method in PhabricatorUserCache
ClosedPublic

Authored by aklapper on May 16 2024, 10:42.

Details

Summary

Seems to be unused. Plus it calls Filesystem::digestForIndex() which does not exist. So if this method was ever called then it would through an exception and not reach the parent::save() call either in the line after.

Test Plan

Grep and read the codebase.

Tried things that may use cache and they still work, including:

  • running ./bin/cache purge --caches user successfully shows Purging "user" cache...
  • the button "Purge cache" in /config/cache/ still work.
  • editing a user
  • changing user picture using default one
  • uploading new user picture
  • creating Calendar event and inviting yourself
  • visiting user settings
  • changing a user setting, including Timezone, Display preferences

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

It would be nice to understand why nothing calls save() 🤔 since I guess nothing calls it, or we would have a crash report.

plus, who is calling the parent::save(). Boh.

It would be nice to understand why nothing calls save() 🤔 since I guess nothing calls it, or we would have a crash report.

Indeed, I could not find a spot calling save() in the codebase, and great-grandparent LiskDAO does not define as abstract. Maybe it's just time to remove it here?

OK. Giving that also the parent::save() will fail, I think yes, feel free to nuke this method to see what happens. lol

Remove unused save() method in PhabricatorUserCache

aklapper retitled this revision from Fix call to non-existing Filesystem::digestForIndex() in PhabricatorUserCache to Remove unused save() method in PhabricatorUserCache.Jul 1 2024, 15:39
aklapper edited the summary of this revision. (Show Details)
aklapper edited the test plan for this revision. (Show Details)
valerio.bozzolan edited the test plan for this revision. (Show Details)

Thanks. I'm quite sure that everything is just calling writeCache().

This revision is now accepted and ready to land.Jul 1 2024, 18:28