make file cache updates more robust #10397
Conversation
schiessle
added
bug
3. to review
schiessle
force-pushed
the
encryption-s3-fix
branch
from
a70f16a
to
deebd55
Compare
schiessle
force-pushed
the
encryption-s3-fix
branch
from
deebd55
to
0512d91
Compare
|
@schiessle can you rebase so CI can run? |
only update the encrypted version after the write operation is finished and the stream is closed Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
schiessle
force-pushed
the
encryption-s3-fix
branch
from
0512d91
to
a9e22c5
Compare
|
@rullzer done |
|
I would prefer if it used a (you can use the write callback to set the |
|
@icewind1991 I agree with you in general. But as I want to backport it to 13 as well I would like to keep the fix as small as possible. Suggestion: Let's merge this for 14 and backport it for 13 and I will do a follow up PR for master (15) to add a CallbackWrapper. |
| 'encryptedVersion' => 2, | ||
| ]); | ||
| $cache->expects($this->any())->method('get')->willReturn($entry ); | ||
| $cache->expects($this->any())->method('update')->with(5, ['encrypted' => 3, 'encryptedVersion' => 3]); |
There was a problem hiding this comment.
change this to $this->once() please
There was a problem hiding this comment.
It is not possible. The stream, created in the method is used in various places. Sometimes the method is never called and some tests uses the instance multiple times so it is used more then once. All I want to make sure is that every time it is called, it should be called with the right parameters.
Sounds good to me |
|
Let's get this in for the beta3 so that it gets some test coverage... |
|
@schiessle Could you prepare the backport? |
Only update the encrypted version after the write operation is finished and the stream is closed
With Amazon S3 as primary storage it happened that we tried to update the "encryption version" before the file was added to the file cache which lead to a wrong version number. By updating the version only after the stream was closed I could solve the issue.
@icewind1991 please have a look if this makes sense. Thanks!
fix #8299