Run updateDataDirectory after Update
#14357
Conversation
|
Refer to this link for build results (access rights to CI server needed): |
|
@LukasReschke while at it - can we remove the apache specific check above? In case the upgrade is executed using occ this code block will never be executed - refs #14280 |
LukasReschke
force-pushed
the
fix/1373
branch
from
6c2c188
to
9223ee6
Compare
|
Done. Added as incompatible change in https://github.com/owncloud/core/wiki/ownCloud-8.1-Features as well since this can cause problems if somebody has a custom |
|
Refer to this link for build results (access rights to CI server needed): |
|
👍 |
|
Please review @th3fallen @bantu @nickvergessen THX |
| \OC_Setup::updateHtaccess(); | ||
| \OC_Setup::protectDataDirectory(); | ||
| } catch (\Exception $e) { | ||
| throw new \Exception($e->getMessage()); |
There was a problem hiding this comment.
This seems like a rather weird construction. If you know why we're doing this, add a comment.
There was a problem hiding this comment.
Blind guess:
To convert the \OC\HintException into a normal Exception with a normal string message
There was a problem hiding this comment.
That assumption is correct. Let me add a comment in regard to this behaviour.
There was a problem hiding this comment.
Consider leaving this exception uncaught. Whoever catches the new exception calls $e->getMessage() anyway? Not sure.
There was a problem hiding this comment.
The problem is that in the update situation we except a regular exception and in the install situation our code expects a HintException and will show it to the user.
If we don't catch it the behaviour is not anymore the one that we wanted.
There was a problem hiding this comment.
Yes. But we'll handle that differently later in the code (such as index.php)
There was a problem hiding this comment.
This is ... broken ... but let's move on.
There was a problem hiding this comment.
To reiterate: Code further up in the call stack throws a HintException which can be properly handled by code further down in the stack. However, for some reason, we want the code further down to handle this as a more generic exception of type \Exception. Technical debt ticket?
There was a problem hiding this comment.
In fact, the conversion from HintException to \Exception is only necessary because the code further down is capable of handling HintException. If it would not handle HintException, but only the more generic \Exception, everything would be fine.
|
This conflicts now. |
|
@LukasReschke can I ask you for a rebase? THX |
|
Nothing is funnier than a rebase. |
|
Refer to this link for build results (access rights to CI server needed): |
|
The inspection completed: 1 updated code elements |
|
👍 |
Run `updateDataDirectory` after Update
Fixes #13731
fixes #14280
cc @DeepDiver1975