Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Cache API #6013

Merged

Conversation

MissAllSunday
Copy link
Contributor

@MissAllSunday MissAllSunday commented Feb 11, 2020

This PR refactors the current cache API

  • separates the abstract and the interface into their own classes
  • namespace usage
  • each implementation now has extend the basic abstract class and implement the cache interface otherwise SMF won't see it as a valid option.
  • cleaner loadCacheAPIs() function, it does not depends on regex to get the correct class
  • cleaner structure, each API implementation goes into the Sources/cache/APIs folder
  • cleans up Sources folder a bit.
  • support for upgrades, it attempts to know the current cache option been used and change $cache_accelerator accordingly.
  • $cache_accelerator now holds the cache class name.
    Tested on Mac Mojave and Ubuntu 18.04.4 using Sqlite, File based and Memcached options.

Fixes #6110

Sources/ManageServer.php Outdated Show resolved Hide resolved
Sources/ManageServer.php Outdated Show resolved Hide resolved
Sources/cache/CacheApiInterface.php Outdated Show resolved Hide resolved
other/Settings.php Outdated Show resolved Hide resolved
other/Settings.php Outdated Show resolved Hide resolved
@MissAllSunday MissAllSunday changed the title [WIP] Implement Redis cache support [WIP] Refactor Cache API Feb 13, 2020
@MissAllSunday MissAllSunday force-pushed the implement_redis branch 2 times, most recently from a20141b to 5a0ed1d Compare February 13, 2020 23:06
@Sesquipedalian
Copy link
Member

Note: Travis build is currently failing due to a problem with Travis itself.

@Sesquipedalian
Copy link
Member

To fix Travis builds, please merge release-2.1 into this branch.

@MissAllSunday
Copy link
Contributor Author

This is now considered done. Please test it specially with the other cache options.

@MissAllSunday MissAllSunday changed the title [WIP] Refactor Cache API Refactor Cache API Feb 17, 2020
Sources/Load.php Outdated Show resolved Hide resolved
@MissAllSunday
Copy link
Contributor Author

It seems GitHub ate a lot of commits, will investigate why or try to bring them back.

@MissAllSunday
Copy link
Contributor Author

Found the issue, GitHub for some reason took the same branch as a different one, merged both.

@live627
Copy link
Contributor

live627 commented Feb 25, 2020

IMO any major refactoring should be postponed for the next major version.

@MissAllSunday
Copy link
Contributor Author

MissAllSunday commented Feb 25, 2020

I agree. However, this isn't a major refactor. The abstract class and interface already exists, there is no change in how the cache apis work and are implemented (besides how the current cache info is stored).

This changes makes sure anyone wanting to add another cache engine should follow our structure and makes it easier to add more engines.

@live627
Copy link
Contributor

live627 commented Feb 25, 2020 via email

Sources/Load.php Outdated Show resolved Hide resolved
Sources/Cache/CacheApi.php Outdated Show resolved Hide resolved
@MissAllSunday
Copy link
Contributor Author

Bump, we still need a proper API implementation. $cache_accelerator still holds a plain string and a few fixes to the current cache implementation are also included.

Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
…ache versions

Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
…sion, fallback to N/A

Fixes SimpleMachines#6110

Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Ref: SimpleMachines#5969
Signed-off-by: Jessica González <suki@missallsunday.com>
Ref: SimpleMachines#5969
Signed-off-by: Jessica González <suki@missallsunday.com>
… class

Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
Signed-off-by: Jessica González <suki@missallsunday.com>
@MissAllSunday MissAllSunday merged commit 52cc4a3 into SimpleMachines:release-2.1 Oct 24, 2020
/**
* Creates the json_encoded array for the current cache option.
*
* @return string a json_encoded array with the selected API options
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the JSON?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is not json entry anymore, it still is a plain string I just forgot to update the dock block

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memcache errors
6 participants