Checksum won't be reset if there is a new checksum to be set

[jvillafanez]

Description

Don't reset the checksum when the same file is reuploaded. In addition, copy the checksum of the original file to the version when a new version is successfully created.

There has been some problems with oracle and the updating of the file cache which are now solved

Related Issue

https://github.com/owncloud/enterprise/issues/4206

Motivation and Context

How Has This Been Tested?

Manually checked with oracle DB:

1. Upload a file
2. Verify checksum is there
3. Reupload the same file
4. Verify the checksum of the file is there, and that the version also has the corresponding checksum

Additional checks:

• Trashbin still keeps the checksums (it seems to be working slightly different in this regard, so no changes were required)
• Checked also with part_file_in_storage => false

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[jvillafanez]

It seems we've hit a weird issue with oracle.
Some debugging data related to the failing test against an Oracle DB:

BEFORE UPDATE->Array
(
    [fileid] => 5411
    [storage] => home::1-verifycheksumsTwgKP6NtbqNh6
    [path] => files/dir/nested/somefile.txt
    [parent] => 5410
    [name] => somefile.txt
    [mimetype] => text/plain
    [mimepart] => text
    [size] => 12
    [mtime] => 1600880740
    [storage_mtime] => 1600880740
    [encrypted] => 
    [etag] => ba631abbdf1a8fa28c659ce0778f1b01
    [permissions] => 27
    [checksum] => SHA1:2ef7bde608ce5404e97d5f042f95f89f1c232871 MD5:ed076287532e86365e841e92bfc50d8c ADLER32:1c49043e
    [encryptedVersion] => 0
)
QUERY -> UPDATE `*PREFIX*filecache` SET `checksum`=? WHERE (`checksum` <> ? OR `checksum` IS NULL) AND `fileid` = ? 
Array
(
    [0] => 
    [1] => 
    [2] => 5411
)
AFTER UPDATE->Array
(
    [fileid] => 5411
    [storage] => home::1-verifycheksumsTwgKP6NtbqNh6
    [path] => files/dir/nested/somefile.txt
    [parent] => 5410
    [name] => somefile.txt
    [mimetype] => text/plain
    [mimepart] => text
    [size] => 12
    [mtime] => 1600880740
    [storage_mtime] => 1600880740
    [encrypted] => 
    [etag] => ba631abbdf1a8fa28c659ce0778f1b01
    [permissions] => 27
    [checksum] => SHA1:2ef7bde608ce5404e97d5f042f95f89f1c232871 MD5:ed076287532e86365e841e92bfc50d8c ADLER32:1c49043e
    [encryptedVersion] => 0
)

That's the before and after states when the update happens in https://github.com/owncloud/core/blob/checksum_reset_fix/apps/files/tests/Command/VerifyChecksumsTest.php#L309 along with the executed query.

It seems the update query fails to execute somehow and the checksum isn't reset in the test, causing the failure.

Removing https://github.com/owncloud/core/pull/37934/files#diff-0b9e2319ad842ab51e0c0f31b25cae21R213 fixes the test, but I guess it's due to the checksum not being set in the first place, so this isn't a solution.

[jvillafanez]

Confirmed that the "update" statement doesn't do anything in oracle:

SQL> UPDATE "oc_filecache" SET "checksum"='' WHERE ("checksum" <> '' OR "checksum" IS NULL) AND "fileid" = 2436;

0 rows updated.

SQL> select * from "oc_filecache" where "fileid" = 2436;

    fileid    storage path			     path_hash				  parent name				mimetype mimepart	size	  mtime storage_mtime  encrypted unencrypted_size etag					   permissions checksum
---------- ---------- ------------------------------ -------------------------------- ---------- ------------------------------ -------- -------- ---------- ---------- ------------- ---------- ---------------- ---------------------------------------- ----------- ---------------------------------------------------------------------------------------------------------
      2436	  242 files/welcome.txt 	     c49a144e772727f51b75705170e88a43	    2435 welcome.txt			       4	3	 163 1600875669    1600875669	       0		0 5a2dc2933e241234a655cc4970e50745		    27 SHA1:eeb2c08011374d8ad4e483a4938e1aa1007c089d MD5:368e3a6cb99f88c3543123931d786e21 ADLER32:c5ad3a63

[codecov]

Codecov Report

Merging #37934 into master will increase coverage by 0.01%.
The diff coverage is 97.36%.

@@             Coverage Diff              @@
##             master   #37934      +/-   ##
============================================
+ Coverage     64.71%   64.72%   +0.01%     
- Complexity    19442    19449       +7     
============================================
  Files          1286     1286              
  Lines         76022    76047      +25     
  Branches       1336     1336              
============================================
+ Hits          49196    49220      +24     
- Misses        26432    26433       +1     
  Partials        394      394              

Flag	Coverage Δ	Complexity Δ
#javascript	54.06% <ø> (ø)	0.00 <ø> (ø)
#phpunit	65.89% <97.36%> (+0.01%)	19449.00 <0.00> (+7.00)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Files/Stream/Checksum.php	84.72% <87.50%> (-0.13%)	26.00 <0.00> (+2.00)	⬇️
apps/files_versions/lib/Storage.php	72.34% <100.00%> (+0.23%)	96.00 <0.00> (+1.00)
lib/private/Files/Cache/Cache.php	97.75% <100.00%> (+0.10%)	110.00 <0.00> (+3.00)
lib/private/Files/Cache/Scanner.php	87.08% <100.00%> (ø)	119.00 <0.00> (+1.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 5a444c5...5f3ba76. Read the comment docs.

[VicDeo]

Looks good

[phil-davis]

@jvillafanez this needs a rebase so that it runs the latest CI and does the SonarCloud-analysis stuff...

[jvillafanez]

Rebased

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
No Duplication information