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

[objectstore] Fix checksums not being updated on modifying shared file #32337

Merged
merged 1 commit into from
Aug 16, 2018

Conversation

mrow4a
Copy link
Contributor

@mrow4a mrow4a commented Aug 14, 2018

Issue

Failing smashbox tests with sharing on objectstore.

Solution

SharingScanner did not update checksums in file cache for object store, since logic for obcjectstorage has not been implemented (SharingScanner assumes by default Local Filesystem). This PR adds logic for objectstorage.
@DeepDiver1975 @patrickjahns

@codecov
Copy link

codecov bot commented Aug 14, 2018

Codecov Report

Merging #32337 into master will increase coverage by <.01%.
The diff coverage is 87.5%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #32337      +/-   ##
============================================
+ Coverage     64.01%   64.01%   +<.01%     
- Complexity    18564    18567       +3     
============================================
  Files          1171     1171              
  Lines         69851    69861      +10     
  Branches       1267     1267              
============================================
+ Hits          44714    44722       +8     
- Misses        24767    24769       +2     
  Partials        370      370
Flag Coverage Δ Complexity Δ
#javascript 52.84% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.29% <87.5%> (ø) 18567 <4> (+3) ⬆️
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/ObjectStore/NoopScanner.php 87.5% <100%> (+2.88%) 6 <2> (+1) ⬆️
apps/files_sharing/lib/Scanner.php 88.88% <75%> (-6.12%) 9 <2> (+2)

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 25b8148...a038741. Read the comment docs.

public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true) {
$sourceScanner = $this->getSourceScanner();
if ($sourceScanner instanceof NoopScanner) {
return [];
list(, $internalPath) = $this->storage->resolvePath($file);
return parent::scan($internalPath, $reuseExisting, $parentId, $cacheData, $lock);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DeepDiver1975 @PVince81 this is an easy fix for the issue.. but I think this class needs larger refactoring to make it support both local filesystem and objectstorage..

@DeepDiver1975
Copy link
Member

Looks good. Lets merge this to master and See .....
THX

@DeepDiver1975 DeepDiver1975 merged commit 422bc84 into master Aug 16, 2018
@phil-davis
Copy link
Contributor

Looks good - see comment on PR with test scenarios #32353 (comment)

@mrow4a mrow4a deleted the obj_fix_checks_shared branch August 18, 2018 08:06
@lock lock bot locked as resolved and limited conversation to collaborators Aug 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants