search

rfink / sequelize-redis-cache

Small fluent interface for caching sequelize database query results in redis more easily
MIT License
174 stars 47 forks source link

Added support for findAndCount method, and allow for multiple instances of Cacher #14

Closed mccloud-jeff closed 8 years ago

mccloud-jeff commented 8 years ago

Pretty simple :)

mccloud-jeff commented 8 years ago

The 2nd commit is not related to findAndCount. This change was to fix a rather major bug. The internal variables for the Sequelize and Redis instances passed into the the init() function were not 'private' and thus were shared (stepped on) by subsequent calls to init(). For example, if you have 2 Sequelize instances you want to cache (two different databases), the 2nd call to init the cache would blow away the internal variables captured during the 1st call. This leads to "Xyz has not been defined" errors, where the cache can't find a previously defined model (because it was defined by a previous Sequelizer). So.... I moved those variables, and the rest of the code, inside the init() closure, so that each call maintains its own pointers to Sequelize and Redis instances.

rfink commented 8 years ago

Hey, can these be two separate pull requests? One is a minor release and one would be a major (semver) release. I can't remember why I created the API as a singleton, but I'm definitely going to change that with the next major release. I should have a branch soon.

mccloud-jeff commented 8 years ago

It turned out I needed something a little more specific than this, with cache busting after writes and merging in deltas, so I am working on something more specific to my project for now. Love what you did with this! If I produce anything worth sharing publicly, I'll be sure to cite this project as an inspiration. In the meantime, I'll close the PR since the proposed changes don't seem essential at this time. Mahalo! ("thanks", from Hawaii !)