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

fix(crud): TypeOrm Cache was not working #688

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

rewiko
Copy link
Collaborator

@rewiko rewiko commented Apr 2, 2021

TypeOrm Cache is not working.

builder.getQueryAndParameters() returns an array of 2 items, first one is the SQL generated and second one is an array of given parameters. link

TypeOrm cache function can have either 2 parameters with the first one the key name as a string and the TTL or only the latter and the key will be generated automatically.

2 options are possible the first one would be to concatenate the getQueryAndParameters array or let TypeOrm generates the key.

Fixes #397 #407 #569

The cache should not be used to get resource while we try to update/delete them.

@rewiko
Copy link
Collaborator Author

rewiko commented Apr 16, 2021

Hello @zMotivat0r,
I noticed there are a few PRs fixing or improving functionalities.
Would you be able to have a look?
I am happy to help of course 😃

Copy link

@atjsh atjsh left a comment

Choose a reason for hiding this comment

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

I had same issue. This fix looks great to me.

@elvinmezacb
Copy link

Any plans on merging this? I'm facing the same issue.

@rewiko
Copy link
Collaborator Author

rewiko commented Nov 27, 2021

Merged and released on my own repo Cache is not working rewiko#3
https://github.com/rewiko/crud/blob/master/CHANGELOG.md

@rewiko rewiko mentioned this pull request Nov 28, 2021
19 tasks
@dss-ivelin-gorchovski
Copy link

Why isnt this merged. a simple debug of the library clearly shows the bug? Any ideas when this will be fixed?

@dss-ivelin-gorchovski
Copy link

Can the reviewers provide some update on this Merge request?

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

Successfully merging this pull request may close these issues.

Cache option not working
4 participants