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

Cache API needs clarification #1037

Closed
fulmeek opened this issue Feb 17, 2019 · 2 comments
Closed

Cache API needs clarification #1037

fulmeek opened this issue Feb 17, 2019 · 2 comments
Labels
Documentation Documentation needs updating question Indicates that an issue or pull request needs more information

Comments

@fulmeek
Copy link
Contributor

fulmeek commented Feb 17, 2019

Hi there,

imagine you like to write a new cache implementation based on the documentation in the wiki, like I did in PR #1035. It basically describes the functions loadData, saveData, getTime and purgeCache that are defined in the CacheInterface class. So, you know how to save, load, delete and get the age of your data.

However, following the key-value storage principle, there's something missing: a key or token to identify your data. Looking at the FileCache implementation, there are two functions that get called first: setPath($path) and setParameters(array $param). They are undocumented and declared with file based caching in mind.

Basically, setPath gets the absolute path where to store the particular data entry. Not sure if this is always the same, probably it's not. I consider this to be some kind of scope. And setParameters appears to get the parameters of the feed. So, this is the key the data is related to.

I think this needs some clarification.

@logmanoriginal
Copy link
Member

You are right, CacheInterface was originally designed with FileCache in mind (this is actually still the same implementation as in the early days of RSS-Bridge). That is also the reason why documentation and interface are very basic. Cache types like SQLite (#1035) and Memcached (#1000) have different requirements which need to be taken into consideration.

For example:

  • Allow cache specific configuration parameters (i.e. file name, path, password, etc...)
  • Allow cache to specify requirements that are checked before execution (i.e. 'sqlite3' or 'memcache(d)') (see f450b2e for reference)
  • Replace setPath by i.e. setHeader to specify which kind of cache to use (i.e. 'feed' for caching the actual feeds, 'page' for caching replies from servers, etc...). For SQLite that could mean different tables, folders for FileCache, servers for Memcached, etc....

You and @em92 can probably name more requirements. Please don't hesitate to open PRs to address these changes.

@logmanoriginal logmanoriginal added question Indicates that an issue or pull request needs more information Documentation Documentation needs updating labels Mar 2, 2019
@logmanoriginal
Copy link
Member

Closing because #1060 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation needs updating question Indicates that an issue or pull request needs more information
Projects
None yet
Development

No branches or pull requests

2 participants