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.

Sign up for GitHub

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

Jump to bottom

Object store move fixes and more #48160

Merged
merged 12 commits into from
Sep 18, 2024
Merged

Object store move fixes and more #48160

merged 12 commits into from
Sep 18, 2024

Conversation

icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Sep 18, 2024 •
edited
Loading

Fixes some issues in cross-storage moves with object storage found by the tests added in #47986

I'm not sure how much of these issues would occur in the real world or only in the artificial test situations.

Comes with a bonus lower overhead file_put_contents implementation.

Fixes #48169

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Sep 18, 2024
@icewind1991 icewind1991 added this to the Nextcloud 31 milestone Sep 18, 2024
@icewind1991 icewind1991 requested review from a team, artonge, yemkareems and provokateurin and removed request for a team September 18, 2024 09:21
@icewind1991 icewind1991 mentioned this pull request Sep 18, 2024
Merged
@provokateurin
Copy link
Member

It seems to uncover a lot of different problems according to the tests.

@icewind1991 icewind1991 added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Sep 18, 2024
@icewind1991
fix: get source file before moving the cache item in objectstore move…
879fcc4 
…FromStorage

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
fix: create intermediate directories for objectstore moveFromStorage
3e94ee3 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
fix: cleanup objectstore file_put_content
60f6347 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
fix: fix object store id for test object store
e0fb25f 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
fix: verify that parent exists in cache when inserting
75cf17e 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
test: fix incorrect ltrim usage in test
74adaeb 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
fix: ensure parent folder exists when writing a file to object storage
3c0c81b 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 mentioned this pull request Sep 18, 2024
Closed
@cfiehe
Copy link

cfiehe commented Sep 18, 2024

@icewind1991
Thank you very much for fixing the issues 😃. Then in the very end the new test cases - even with initial bugs - were actually successful.

@icewind1991 icewind1991 changed the title Object store move fixes Object store move fixes and more Sep 18, 2024
@icewind1991 icewind1991 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 18, 2024
@icewind1991
Copy link
Member Author

phpunit is green now

@icewind1991
test: put parent items into cache in tests
1db3d94 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
test: fix service overwrite in PartFileInRootUploadTest
627980e 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
test: fix ShareAPIControllerTest.php
7661a7e 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
test: fix trashbin StorageTest
d8ed846 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
test: fix ShareControllerTest.php
1ad58d6 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@provokateurin
Copy link
Member

Seems like the sharding bug is back on oracle.

@icewind1991
Copy link
Member Author

from what I can tell SharesReminderJob never worked on oracle, I'm trying to fix it, but you know, oracle...

@blizzz blizzz merged commit 33928fc into master Sep 18, 2024
162 of 174 checks passed
@blizzz blizzz deleted the object-store-move-fixes branch September 18, 2024 15:50
@icewind1991
Copy link
Member Author

/backport to stable30

@backportbot backportbot bot mentioned this pull request Sep 19, 2024
Closed
2 tasks
This was referenced Sep 19, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yemkareems yemkareems yemkareems approved these changes

@provokateurin provokateurin provokateurin approved these changes

@come-nc come-nc come-nc approved these changes

@artonge artonge Awaiting requested review from artonge artonge was automatically assigned from nextcloud/server-backend

Assignees
No one assigned
Labels
3. to review Waiting for reviews feature: object storage
Projects
None yet
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.

[Bug]: Method getStorageId() of StorageObjectStore returns null instead of the storage/bucket identifier
7 participants
@icewind1991 @provokateurin @cfiehe @yemkareems @come-nc @joshtrichards @blizzz