fix: create a different cacheId if present for count query in getManyAndCount

[eugenio165]

Description of change

Creates a different cacheId (if provided) for the count query in getManyAndCount so that it does not retreive the cache for the previous query (the select immediately above it). Previously, when providing a cacheId and using getManyAndCount or findAndCount, the count query would use the exact same cacheId as the select query above it, causing the count value to return as 0 (because it doesnt even exist in the select query result). Providing a different cacheId for the count query solves the problem. Only happens when explicitly defining a cacheId.

Fixes #4277

Pull-Request Checklist

• Code is up-to-date with the master branch
• npm run lint passes with this change
• npm run test passes with this change
• This pull request links relevant issues as Fixes #0000
• There are new or updated unit tests validating the change
• Documentation has been updated to reflect this change N/A
• The new commits follow conventions explained in CONTRIBUTING.md

[pleerock]

Seems good. Thank you for contribution!

[eugenio165]

@pleerock It's a pleasure, i thank you guys for the great work!

But i've seemed to encounter another issue related to this. When joins are present, TypeORM has to make three queries to the DB, the first being to retreive the unique id's of the entities that match the query, the second to actually retreive these entities, and the third (in the case of pagination) to count the total entities.

In my experience, when i explicitly define a cacheId for a large query with many joins, and later manually remove the cache using this cacheId, it only clears the query that retreives the entities, because the first query is cached not with the cacheId, but by the query.

So what happens is, the only query that gets fresh data is the one that retreives entities, that were already present in the 1st query. When entities are removed or added, the first query (which returns the ids) is not going to retreive the fresh results because its still cached with the previous result.

Hope this makes sense.

Anyways, i was thinking about this, and my idea was to also identify the first request with a unique cacheId, like: ${cacheId}-ids (to follow the pattern of this PR's changes). And also, in the QueryResultCache's, in the concrete implementations, when a remove is called, add the two other keys to be removed. So if somebody calls connection.queryCacheResults.remove with the cacheId: posts, the QueryResultCache would remove the cacheId's: posts, posts-ids, posts-count.

What do you think? Im happy to work on this as needed to improve TypeORM, lets discuss a reasonable way to aproach this and i'll get working.

Again, many thanks!

[pleerock]

Sounds reasonable. But can't we simply store "posts" and "posts-count"? Since data is already cached we might not need posts-ids since the data we need to retrieve is already stored in posts and we don't need to make another requests to ids?

[eugenio165]

You're right, good catch. I'll start working on it as soon as i can, thanks!

[shrujalshah28]

This does solve #4277 but now we have another issue, as the count cache key is added separately and internally, it does not remove the count cache even if we remove cache by cache-id by that we added and so count in return value does not change until cache expired.