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

[WIP] Delete external storage cache after deletion #11544

Closed
wants to merge 1 commit into from

Conversation

PVince81
Copy link
Contributor

🚧 DO NOT MERGE 🚫

Delete external storage cache entries after deletion.

One issue is that it requires the configuration to be valid. Only with a valid config the Storage instance can be initialized, and only a properly initialized instance will be able to deliver the storage ID that is needed for deletion...

Needs further work.

@icewind1991

@PVince81
Copy link
Contributor Author

  • Issue with misconfigured storages: cannot init/delete them
  • "Undo" notification when deleting
  • Unit tests

@PVince81
Copy link
Contributor Author

Depends on #11545 to properly clear the file cache.

@PVince81
Copy link
Contributor Author

  • BUG: renaming mount point deletes + recreates storage which would lose the shares

@PVince81
Copy link
Contributor Author

Rebased onto master to get the changes from #11694

@PVince81
Copy link
Contributor Author

PVince81 commented Nov 4, 2014

Fix for #10820

@PVince81
Copy link
Contributor Author

@DeepDiver1975 setting this to OC 8.1.

Now that @icewind1991 brought the "stabler" ext storage ids, it might be possible to rename/delete more safely now

@PVince81
Copy link
Contributor Author

The new "storage_id" is a cool addition, but won't be usable realiable until we fix the endpoints to also work with storage ids. Raised here: #14058

@scrutinizer-notifier
Copy link

The inspection completed: 3 new issues, 1 updated code elements

@ghost
Copy link

ghost commented Feb 10, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/9149/
Test PASSed.

@DeepDiver1975 DeepDiver1975 modified the milestones: 8.1-current, 8.2-next Apr 2, 2015
@mmattel
Copy link
Contributor

mmattel commented Apr 4, 2015

I have two questions:

  1. is with this PR also the removal of oc_storages made?
  2. If the cleanup is done in oc_filecache and oc_storages, are there any possible leftovers to be taken care for, like shares. eg user 1 shares something with user 2 ect but the storage has been removed.

@PVince81
Copy link
Contributor Author

PVince81 commented Apr 7, 2015

  1. This PR is for the removal of "oc_storages" entries for external storages that have been deleted in the UI.
    One problem though is that if someone used the "$user" variable in any of the settings, it is likely that a single mount configuration produced multiple "oc_storages" entries, which makes it impossible to find which entries matched the config entry, because so far there is no information in the database that accurately maps ext storage config entries to storage entries (but could be added in the future)
    So for now this PR will focus on deleting the predictable entries (when the magic "$user" was not used).
  2. So far there will be possible leftover shares in the database, but they could be cleant up by a future background job, see Delete orphaned shares in a background job #14676

@PVince81
Copy link
Contributor Author

PVince81 commented Apr 7, 2015

  • raise ticket for complex "$user" case

@mmattel
Copy link
Contributor

mmattel commented Apr 7, 2015

#11830 Redesign/revamp oc_storages to fix various issues

@ghost
Copy link

ghost commented Apr 30, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/12077/
💣 Test FAILed. 💣

nooo432

@ghost
Copy link

ghost commented Jul 15, 2015

🚀 Test PASSed.🚀
chuck

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 1, 2015

Since we still have the storage ids and no proper "mount config to storage id" mapping, we can't reliably delete storage caches.

Deferring to 9.0

CC @Xenopathic

@PVince81 PVince81 modified the milestones: 9.0-next, 8.2-current Sep 1, 2015
@PVince81
Copy link
Contributor Author

PVince81 commented Oct 1, 2015

The underlying code has changed, need to use a different (and hopefully better) approach

@PVince81 PVince81 closed this Oct 1, 2015
@PVince81 PVince81 deleted the delete-storage branch October 1, 2015 10:36
@MorrisJobke MorrisJobke removed this from the 9.0-next milestone Oct 5, 2015
@enoch85
Copy link
Member

enoch85 commented Jul 2, 2016

The underlying code has changed, need to use a different (and hopefully better) approach

@PVince81 Is this done yet?

@PVince81
Copy link
Contributor Author

PVince81 commented Jul 4, 2016

@enoch85 this is done only for the cases where it is technically possible: as long as you don't use the $user variable in any of that parameters, it will remove the oc_storages and oc_filecache entries of the deleted external storage.

@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 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.

6 participants