Make sure to filter filecache subfolders in sql query instead of memory

[mrow4a]

Related issue: https://github.com/owncloud/enterprise/issues/2925
Goal: avoid having peaks in memory for large directory contents

While deleting directories with a lot of files inside usingview->unlink, we do not need to put in memory unnecessarily records. Filter for directories when only subdirs needed.

Example:

• we have folder with 50k files and 100 subfolders
• some parent function already loaded all 50100 records in memory
• now we are going to run unlink on that folder contents
• previously, all 50100 records were in memory in unlink
• now, only 100 records are in memory unlink
• resulting peak memory is 50200 instead of 100200

[codecov]

Codecov Report

Merging #36565 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #36565   +/-   ##
=========================================
  Coverage     64.69%   64.69%           
- Complexity    19114    19116    +2     
=========================================
  Files          1269     1269           
  Lines         74808    74816    +8     
  Branches       1327     1327           
=========================================
+ Hits          48395    48403    +8     
  Misses        26022    26022           
  Partials        391      391           

Flag	Coverage Δ	Complexity Δ
#javascript	54.09% <ø> (-0.01%)	0.00 <ø> (ø)
#phpunit	65.87% <100.00%> (+<0.01%)	19116.00 <5.00> (+2.00)	⬇️

Impacted Files	Coverage Δ	Complexity Δ
core/js/sharedialoglinkexpirationview.js	76.19% <0.00%> (-0.38%)	0.00% <0.00%> (ø%)
lib/private/Files/Cache/Cache.php	97.64% <0.00%> (+0.06%)	106.00% <0.00%> (+2.00%)	⬇️
lib/private/Share20/Manager.php	96.30% <0.00%> (ø)	273.00% <0.00%> (ø%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 920de6f...5dd63aa. Read the comment docs.

[mrow4a]

I will probably add 1 more unit test

[jvillafanez]

Code looks good but we need to fix the tests.

[mrow4a]

yes I noticed that my test that I wrote failed because of some wrapping logic

[jvillafanez]

I don't really like this...
If the problem is that the parent test is calling a private method, can we adjust that test to call a public one ensuring that the call goes through the code we want? If needed, add 2 different tests, one per case.

[mrow4a]

@jvillafanez I actually did as some other examples above in that test case. I am not sure if this is relevant to build testcase here.

[jvillafanez]

My worry is that, if we ever need to change the implementation in the CacheJail class, we might assume that there are tests, and due to the test passing we could think the code works even though it might be broken.

I think it's easier to call a public method in the CacheTest, and then copy & paste the test to adjust the expectations, if needed, in the rest of the implementations

[mrow4a]

@jvillafanez ok now I remember why I commented with not supported as parent CacheTest calls private method of Cache class. Because if I try to call this private function I get exceptions on connection being null.

Please note that this function under test is private! It is called from public function remove, that currently has no unit tests.

[jvillafanez]

What if you test the public functions that call that private function instead?

I'm not sure if it's a better alternative that we mark the test as skipped or incomplete instead.

[mrow4a]

Ok I will test public function remove, but it means also we cannot test filtering mechanism itself (coverage will however be happy)

[mrow4a]

@jvillafanez done

[mrow4a]

@micbar release note for this PR in #36602

[micbar]

@mrow4a code style, double empty line