Core: [10.5.0 RC2] Manual locking feature not exposed via capabilities endpoint

[michaelstingl]

Steps to reproduce

docker run --rm -d \
  --name owncloud \
  -p 18080:8080 \
  -e OWNCLOUD_APPS_INSTALL=oauth2 \
  -e ADMIN_USERNAME=admin \
  -e ADMIN_PASSWORD=admin \
  owncloud/server:10.5.0-rc2

nothing related to locking

% curl -u admin:admin 'http://localhost:18080/ocs/v1.php/cloud/capabilities?format=json' | jq | grep lock
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1792  100  1792    0     0   3756      0 --:--:-- --:--:-- --:--:--  3748

Expected behaviour

Clients check capabilities endpoint anyway, so they would enable a future manual locking feature based on this data

Actual behaviour

nothing related to locking found

[phil-davis]

Please remember to add a test scenario for the new capability to https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiCapabilities/capabilities.feature

[jvillafanez]

I don't think we have a nice solution for this... The main problem is that the LockManager isn't public and it isn't exposed outside of the storage layer. We also need to decide where to place this capability.

Some dirty options, assuming we want this capability be part of the dav app (which is where it will be used):

• Get only the explicitly configured values from the app config
  
  
  
  • Implicit default values (if the admin didn't set anything) won't be shown
  
  
  • We'd need to use -1 if the value hasn't been set.
  
  
• Get the configured values with defaults
  
  
  
  • We'd need to use the LockManager, which isn't public, in the dav app.
  
  
  • Wrong values (negative timeouts) could be returned. We don't want to have the logic duplicated so there won't be logic in this case.
  
  
• Create new methods in the LockManager class to get the timeouts and use them in the dav capabilities
  
  
  
  • We'd need to use the LockManager, which isn't public, in the dav app.
  
  
• Create a ILockManager in the public namespace to be able to use it in the dav app
  
  
  
  • Not sure if this was planned
  
  
• Create a ILockManagerSettings to store the settings of the LockManager.
  
  
  
  • This would be publicly accessible
  
  
  • Could include additional logic / validation.
  
  

Last option could also be reused to provide an endpoint for the settings page in order to provide server-side validation.

[DeepDiver1975]

``` /**

• @author Christopher Schäpers [e-mail]
• @author Joas Schilling [e-mail]
• @author Roeland Jago Douma [e-mail]
• @author Tom Needham [e-mail]
  *
• @copyright Copyright (c) 2018, ownCloud GmbH
• @license AGPL-3.0
  *
• This code is free software: you can redistribute it and/or modify
• it under the terms of the GNU Affero General Public License, version 3,
• as published by the Free Software Foundation.
  *
• This program is distributed in the hope that it will be useful,
• but WITHOUT ANY WARRANTY; without even the implied warranty of
• MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• GNU Affero General Public License for more details.
  *
• You should have received a copy of the GNU Affero General Public License, version 3,
• along with this program. If not, see http://www.gnu.org/licenses/
  *
  */

namespace OCAFiles;

use OCP\Capabilities\ICapability;
use OCP\IConfig;

/**

• Class Capabilities
  *
• @package OCAFiles
  /
  class Capabilities implements ICapability {
  /* @var IConfig */
  protected $config;

/**
 * Capabilities constructor.
 *
 * @param IConfig $config
 */
public function __construct(IConfig $config) {
    $this->config = $config;
}

/**
 * Return this classes capabilities
 *
 * @return array
 */
public function getCapabilities() {
    return [
        'checksums' => [
            'supportedTypes' => ['SHA1'],
            'preferredUploadType' => 'SHA1'
        ],
        'files' => [
            'privateLinks' => true,
            'privateLinksDetailsParam' => true,
            'bigfilechunking' => true,
            'blacklisted_files' => $this->config->getSystemValue('blacklisted_files', ['.htaccess']),
            'enable_lock_file_action' => (boolean)($this->config->getAppValue('files', 'enable_lock_file_action', false)),
        ],
    ];
}

}

[phil-davis]

Just a note that in release-10.5.0 the enable_lock_file_action check is changed to:

$enableLockFileAction = (boolean)($config->getAppValue('files', 'enable_lock_file_action', 'no') === 'yes');

This is still to be merged back to core master. That is a bit unfortunate if someone starts working on exposing that capability somewhere.

[phil-davis]

At the moment, enable_lock_file_action is just a boolean to switch on the file icon icon in the webUI. "enable_lock_file_action_on_webui_client" is its current behavior. PR #37720 is adding a checkbox on the webUI so that this can be switch on/off on the admin webUI, as well as using the occ command.

The description of that checkbox is currently "Enable manual file locking on the web UI".

So it needs to be decided if we want this setting to:
1) control just the webUI locking (and have other settings to control lock behavior on other clients), or;
2) control the UI locking behavior on all clients (if this setting is enabled, then any new enough client - Android, iOS, desktop and webUI... - would display appropriate UI for manual locking.

The answer to this question then determines the description and documentation of the capability.

[micbar]

@pmaier1 @michaelstingl

Assuming that you agree I propose the following

Add two capabilities

• 1) file_locking_support -> general capability
• 2) file_locking_enable_file_action - > depending on the setting

[michaelstingl]

1. file_locking_enable_file_action - > depending on the setting

clients would show manual locking feature depending on this capability

[phil-davis]

There is the existing enable_lock_file_action which is not currently exposed as a capability, and is only used by the webUI browser-based client. Do we leave that the way it is?
(and so it effectively means specifically just enable_lock_file_action_on_web_ui )

Edit: I see that @DeepDiver1975 is exposing the internal setting enable_lock_file_action with the capability name file_locking_enable_file_action - so that deals with my question.