Properly reset the checksum if the file data has changed

[jvillafanez]

Description

Properly reset the checksum if the file has changed

Related Issue

• Fixes https://github.com/owncloud/enterprise/issues/2561

Motivation and Context

Checksum wasn't being reset in that scenario. The previous condition was wrong

How Has This Been Tested?

Manually tested with the scenario described in the issue

• test environment:
• test case 1:
• test case 2:
• ...

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

Open tasks:

• Backport (if applicable set "backport-request" label and remove when the backport was done)

[PVince81]

@jvillafanez would be good to add a unit test

at first look I have a hard time to understand what you actually changed, might explain the difference before vs after ?

[jvillafanez]

The code is fundamentally the same: if the size, storage_mtime, mtime or etag has changed, then reset the checksum. The new code is better because the old one used array_intersect_key and in_array while the new one just uses isset

The problem was that the $data[$key] != $newData[$key] condition wasn't working properly because the values where the same, so the checksum wasn't being reset.

[PVince81]

@jvillafanez will this change have other side effects ?

would be good to check and extend the unit tests to confirm this works correctly

[codecov]

Codecov Report

Merging #32248 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master   #32248      +/-   ##
============================================
+ Coverage     64.01%   64.02%   +<.01%     
- Complexity    18559    18560       +1     
============================================
  Files          1171     1171              
  Lines         69834    69836       +2     
  Branches       1267     1267              
============================================
+ Hits          44706    44710       +4     
+ Misses        24758    24756       -2     
  Partials        370      370

Flag	Coverage Δ	Complexity Δ
#javascript	52.84% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	65.29% <100%> (ø)	18560 <0> (+1)	⬆️

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Files/Cache/Scanner.php	86.6% <100%> (+0.12%)	114 <0> (+1)	⬆️
apps/files/lib/Command/VerifyChecksums.php	88.57% <0%> (+2.85%)	26% <0%> (ø)	⬇️

---

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 874acce...1587e6c. Read the comment docs.

[jvillafanez]

There are still some unittests failing, but I'm not sure where the problem is. Updated code still works and makes sense, so there shouldn't be a problem with the changes but the checksum repair tests are failing. I'm not sure if there are some problems with the tests.

[IljaN]

@jvillafanez Try breaking the checksum intentionally and then repairing it via repair command to see if checksum repair works as expected. If this is the case we can debug the test to see what is going wrong

[jvillafanez]

I've modifed the data in the DB and it seems the command works.

If the checksum has been reset:

sudo -u www-data ./occ files:checksums:verify -vvv -r
.....
Skipping synctest.log => No Checksum

If not:

sudo -u www-data ./occ files:checksums:verify -vvv -r
....
Checking synctest.log => SHA1:73cbb1d69ba106bf6fe1d7ec1111111111111111 MD5:d451afb7ec8b2aaa1111111111111111 ADLER32:d2999111
Mismatch for synctest.log:
 Filecache:	SHA1:73cbb1d69ba106bf6fe1d7ec1111111111111111 MD5:d451afb7ec8b2aaa1111111111111111 ADLER32:d2999111
 Actual:	SHA1:73cbb1d69ba106bf6fe1d7ec5e3a8214d1f15f81 MD5:d451afb7ec8b2aaa14389bbbf7ce15c4 ADLER32:d2999e73
Repairing synctest.log

[DeepDiver1975]

I assume this requires backport ....

[DeepDiver1975]

I assume this requires backport ....

looks like no needed - @jvillafanez please clarify - thx

[patrickjahns]

@DeepDiver1975
stable10 is required - hence blue-ticket !

Also - might not be solved - still waiting for customer feedback

[DeepDiver1975]

stable10 is required - hence blue-ticket !

upon cherry-pick I get an empty commit ... so I assume this was merged with some other pr already ...

this is why I was asking for clarification.

[jvillafanez]

Backport was #32284 (already merged)

[PVince81]

how to test this manually, is there a shortcut ?

Steps I tried:

1. Setup SFTP mount as "/sftp"
2. Create a file "/sftp/test.txt"
3. Download the file to generate a checksum
4. Check checksum in oc_filecache
5. Directly on the SFTP storage, do a touch with a date in the past
6. In web UI, refresh the folder again

Result: the mtime of the file changed in the web UI

On both v10.0.9 and v10.0.10RC1 the checksum gets cleared properly.

Are there any more subtle things to setup ? @jvillafanez

[jvillafanez]

Other than the steps in the original ticket I'm not sure if this can be easily reproduced. I think the key point is to have the same checksum with different etag and size over the same file in different moments.