We need to ensure that the app holding the objectstore implementation is loaded #26299

Merged
merged 1 commit into from Oct 28, 2016

Projects

None yet

5 participants

@DeepDiver1975
Member
DeepDiver1975 commented Oct 7, 2016 edited

Description

In some cases the app is not yet loaded before loading the objectstore class.
This fixes this issue.

Related Issue

fixes owncloud/objectstore#38

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
@DeepDiver1975 DeepDiver1975 We need to ensure that the app holding the objectstore implementation…
… is loaded - fixes owncloud/objectstore#38
7bb28ae
@DeepDiver1975 DeepDiver1975 added this to the 9.2 milestone Oct 7, 2016
@mention-bot

@DeepDiver1975, thanks for your PR! By analyzing the history of the files in this pull request, we identified @LukasReschke, @PVince81 and @icewind1991 to be potential reviewers.

@felixboehm
Contributor

Code makes sense, can't test right now, missing infrastructure.
@butonic

+ $name = $config['class'];
+ if (strpos($name, 'OCA\\') === 0 && substr_count($name, '\\') >= 2) {
+ $segments = explode('\\', $name);
+ OC_App::loadApp(strtolower($segments[1]));
@PVince81
PVince81 Oct 10, 2016 Collaborator

Shouldn't this happen automatically already ? Maybe load all apps of type "filesystem" here instead ?

@PVince81
PVince81 Oct 10, 2016 Collaborator

(side note: we might want to introduce a new app type "storage" or "storage provider" to accomodate for future separate files_external storage apps and maybe home storage providers)

@DeepDiver1975
DeepDiver1975 Oct 10, 2016 Member

Especially during installation and upgrade apps are not loaded

@PVince81
PVince81 Oct 18, 2016 edited Collaborator

Ok. And is this kind of app name finding magic based on class name copied from somewhere else ? Is it reliable ? (I guess it is for the few implementations we know)

@butonic
Member
butonic commented Oct 27, 2016

finally got to test this, nice! 👍

@PVince81
Collaborator

if #26299 (comment) is reliable and isn't a newly invented concept then I'm fine

@butonic
Member
butonic commented Oct 27, 2016

#26299 (diff) is a workaround for the objectstore app. when we finally implement an explicit way of setting up storages (like an fstab) we can get rid of it.

@PVince81 PVince81 merged commit 20fd465 into master Oct 28, 2016

4 of 5 checks passed

VersionEye Some dependencies have no license.
Details
Scrutinizer 1 updated code elements
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@PVince81 PVince81 deleted the load-primary-storage-apps branch Oct 28, 2016
@DeepDiver1975
Member

backporting ...

@PVince81
Collaborator
PVince81 commented Dec 7, 2016

Backports were merged, removing label

@butonic
Member
butonic commented Dec 16, 2016

There might be a problem when the objectstore version increases: https://github.com/owncloud/core/blob/master/lib/private/legacy/app.php#L151 self updating doesnt work then ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment