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

Add Edit Locally support to macOS virtual files module #6803

Merged
merged 30 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5a3dccd
Add starter EditLocallyVerificationJob
claucambra May 23, 2024
d8f5996
Send and respond to SimpleApiJob to verify edit locally request with …
claucambra May 23, 2024
1422df7
Ensure verification job gets deleted after finish
claucambra May 23, 2024
a5713fc
Use verification job within edit locally job
claucambra May 23, 2024
e38368d
Move token and relpath validity checks to edit locally verification job
claucambra May 23, 2024
a5c7bfe
Ensure we emit error during edit locally job setup issues
claucambra May 27, 2024
7761182
Directly use accountstate in editlocallyjob
claucambra May 27, 2024
1659b21
Add a EditLocallyVerificationJobPtr type
claucambra May 27, 2024
83beaf8
Move verification job handling and scheduling out of editlocallyjob, …
claucambra May 27, 2024
35f988d
Move show error display features from editlocallyjob to editlocallyma…
claucambra May 27, 2024
00cd258
Remove token related tasks from editlocallyjob
claucambra May 27, 2024
263563d
Display token verification failure through GUI in edit locally manager
claucambra May 27, 2024
1080389
Show edit locally loading dialog with filename immediately in editloc…
claucambra May 27, 2024
5df96b8
Do not automatically delete edit locally token verification job upon …
claucambra May 27, 2024
834e096
Add starter FileProviderEditLocally class
claucambra May 28, 2024
7ee296a
Add FileProviderEditLocallyPtr type
claucambra May 28, 2024
9642a03
Add completion signals to fileprovidereditlocallyjob
claucambra May 28, 2024
5ce3cdc
Try to use FileProviderEditLocallyJob before attempting standard Edit…
claucambra May 28, 2024
eb5fa21
Add showError to FileProviderEditLocally
claucambra May 28, 2024
98a0d81
Begin setting up dialog for file provider edit locally job
claucambra May 28, 2024
231d30e
Fetch ocId for a given relative path
claucambra May 28, 2024
26c08e4
Expose domain manager in Mac::FileProvider
claucambra May 28, 2024
a5a9a02
Allow retrieval of domain associated with account in FileProviderDoma…
claucambra May 28, 2024
47c09f9
Anticipate null reply in idGetError of FileProviderEditLocallyJob
claucambra May 28, 2024
7adce87
Remove redundant includes in editlocallymanager
claucambra May 28, 2024
1538280
Implement opening of a file with a given ocId in File Provider files
claucambra May 28, 2024
e345cd4
Connect receipt of a file's ocId with file opening procedure in FileP…
claucambra May 28, 2024
52e29fe
Improve logging in editlocallymanager
claucambra May 28, 2024
8147e83
Start the FileProviderEditLocallyJob in editlocallymanager
claucambra May 28, 2024
27bc838
Fix crash upon removing edit locally file provider jobs in edit local…
claucambra May 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/gui/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ set(client_SRCS
editlocallyjob.cpp
editlocallymanager.h
editlocallymanager.cpp
editlocallyverificationjob.h
editlocallyverificationjob.cpp
filetagmodel.h
filetagmodel.cpp
folder.h
Expand Down Expand Up @@ -288,6 +290,9 @@ IF( APPLE )
macOS/fileproviderdomainmanager_mac.mm
macOS/fileproviderdomainsyncstatus.h
macOS/fileproviderdomainsyncstatus_mac.mm
macOS/fileprovidereditlocallyjob.h
macOS/fileprovidereditlocallyjob.cpp
macOS/fileprovidereditlocallyjob_mac.mm
macOS/fileprovideritemmetadata.h
macOS/fileprovideritemmetadata.cpp
macOS/fileprovideritemmetadata_mac.mm
Expand Down
4 changes: 2 additions & 2 deletions src/gui/application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ void Application::handleEditLocallyFromOptions()
return;
}

EditLocallyManager::instance()->editLocally(_editFileLocallyUrl);
EditLocallyManager::instance()->handleRequest(_editFileLocallyUrl);
_editFileLocallyUrl.clear();
}

Expand Down Expand Up @@ -1125,7 +1125,7 @@ bool Application::event(QEvent *event)
// On macOS, Qt does not handle receiving a custom URI as it does on other systems (as an application argument).
// Instead, it sends out a QFileOpenEvent. We therefore need custom handling for our URI handling on macOS.
qCInfo(lcApplication) << "macOS: Opening local file for editing: " << openEvent->url();
EditLocallyManager::instance()->editLocally(openEvent->url());
EditLocallyManager::instance()->handleRequest(openEvent->url());
} else {
const auto errorParsingLocalFileEditingUrl = QStringLiteral("The supplied url for local file editing '%1' is invalid!").arg(openEvent->url().toString());
qCInfo(lcApplication) << errorParsingLocalFileEditingUrl;
Expand Down
2 changes: 1 addition & 1 deletion src/gui/cocoainitializer_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ - (void)handleURLEvent:(NSAppleEventDescriptor *)event withReplyEvent:(NSAppleEv
{
NSURL* url = [NSURL URLWithString:[[event paramDescriptorForKeyword:keyDirectObject] stringValue]];
const auto qtUrl = QUrl::fromNSURL(url);
OCC::EditLocallyManager::instance()->editLocally(qtUrl);
OCC::EditLocallyManager::instance()->handleRequest(qtUrl);
}

@end
Expand Down
228 changes: 41 additions & 187 deletions src/gui/editlocallyjob.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/gui/editlocallyjob.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/gui/editlocallyjob.cpp

File src/gui/editlocallyjob.cpp does not conform to Custom style guidelines. (lines 31, 32, 45, 54, 130, 271, 272, 281, 310)
* Copyright (C) by Claudio Cambra <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -28,139 +28,41 @@

Q_LOGGING_CATEGORY(lcEditLocallyJob, "nextcloud.gui.editlocallyjob", QtInfoMsg)

EditLocallyJob::EditLocallyJob(const QString &userId,
const QString &relPath,
const QString &token,
QObject *parent)
EditLocallyJob::EditLocallyJob(const AccountStatePtr &accountState,
const QString &relPath,
QObject *parent)
: QObject{parent}
, _userId(userId)
, _accountState(accountState)
, _relPath(relPath)
, _token(token)
{
connect(this, &EditLocallyJob::callShowError, this, &EditLocallyJob::showError, Qt::QueuedConnection);
}

void EditLocallyJob::startSetup()
{
if (_token.isEmpty() || _relPath.isEmpty() || _userId.isEmpty()) {
if (_relPath.isEmpty() || !_accountState) {
qCWarning(lcEditLocallyJob) << "Could not start setup."
<< "token:" << _token
<< "relPath:" << _relPath
<< "userId" << _userId;
return;
}

// Show the loading dialog but don't show the filename until we have
// verified the token
Systray::instance()->createEditFileLocallyLoadingDialog({});

// We check the input data locally first, without modifying any state or
// showing any potentially misleading data to the user
if (!isTokenValid(_token)) {
qCWarning(lcEditLocallyJob) << "Edit locally request is missing a valid token, will not open file. "
<< "Token received was:" << _token;
showError(tr("Invalid token received."), tr("Please try again."));
return;
}

if (!isRelPathValid(_relPath)) {
qCWarning(lcEditLocallyJob) << "Provided relPath was:" << _relPath << "which is not canonical.";
showError(tr("Invalid file path was provided."), tr("Please try again."));
return;
}

_accountState = AccountManager::instance()->accountFromUserId(_userId);

if (!_accountState) {
qCWarning(lcEditLocallyJob) << "Could not find an account " << _userId << " to edit file " << _relPath << " locally.";
showError(tr("Could not find an account for local editing."), tr("Please try again."));
return;
}

// We now ask the server to verify the token, before we again modify any
// state or look at local files
startTokenRemoteCheck();
}

void EditLocallyJob::startTokenRemoteCheck()
{
if (!_accountState || _relPath.isEmpty() || _token.isEmpty()) {
qCWarning(lcEditLocallyJob) << "Could not start token check."
<< "accountState:" << _accountState
<< "relPath:" << _relPath
<< "token:" << _token;

showError(tr("Could not start editing locally."),
tr("An error occurred trying to verify the request to edit locally."));
return;
}

const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(_token)); // Sanitise the token
const auto encodedRelPath = QUrl::toPercentEncoding(_relPath); // Sanitise the relPath

const auto checkTokenJob = new SimpleApiJob(_accountState->account(),
QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken));

QUrlQuery params;
params.addQueryItem(QStringLiteral("path"), prefixSlashToPath(encodedRelPath));
checkTokenJob->addQueryParams(params);
checkTokenJob->setVerb(SimpleApiJob::Verb::Post);
connect(checkTokenJob, &SimpleApiJob::resultReceived, this, &EditLocallyJob::remoteTokenCheckResultReceived);

checkTokenJob->start();
}

void EditLocallyJob::remoteTokenCheckResultReceived(const int statusCode)
{
qCInfo(lcEditLocallyJob) << "token check result" << statusCode;

constexpr auto HTTP_OK_CODE = 200;
_tokenVerified = statusCode == HTTP_OK_CODE;

if (!_tokenVerified) {
showError(tr("Could not validate the request to open a file from server."), tr("Please try again."));
return;
}

findAfolderAndConstructPaths();
}

void EditLocallyJob::proceedWithSetup()
{
if (!_tokenVerified) {
qCWarning(lcEditLocallyJob) << "Could not proceed with setup as token is not verified.";
showError(tr("Could not validate the request to open a file from server."), tr("Please try again."));
<< "relPath:" << _relPath
<< "accountState:" << _accountState;
showError(tr("Could not start editing locally."), tr("An error occurred during setup."));
return;
}

const auto relPathSplit = _relPath.split(QLatin1Char('/'));
if (relPathSplit.isEmpty()) {
showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), _relPath);
showError(tr("Could not find a file for local editing."
"Make sure its path is valid and it is synced locally."), _relPath);
return;
}

_fileName = relPathSplit.last();
_folderForFile = findFolderForFile(_relPath, _userId);

if (!_folderForFile) {
showError(tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), _relPath);
return;
}

if (!isFileParentItemValid()) {
showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), _relPath);
return;
}

_localFilePath = _folderForFile->path() + _relativePathToRemoteRoot;

Systray::instance()->destroyEditFileLocallyLoadingDialog();
startEditLocally();
Systray::instance()->createEditFileLocallyLoadingDialog(_fileName);
findAfolderAndConstructPaths();
}

void EditLocallyJob::findAfolderAndConstructPaths()
{
_folderForFile = findFolderForFile(_relPath, _userId);
_folderForFile = findFolderForFile(_relPath, _accountState->account()->userIdAtHostWithPort());

if (!_folderForFile) {
showError(tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), _relPath);
Expand Down Expand Up @@ -223,6 +125,25 @@
job->start();
}

void EditLocallyJob::proceedWithSetup()
{
_folderForFile = findFolderForFile(_relPath, _accountState->account()->userIdAtHostWithPort());

if (!_folderForFile) {
showError(tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), _relPath);
return;
}

if (!isFileParentItemValid()) {
showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), _relPath);
return;
}

_localFilePath = _folderForFile->path() + _relativePathToRemoteRoot;

startEditLocally();
}

bool EditLocallyJob::checkIfFileParentSyncIsNeeded()
{
if (_relPathParent == QLatin1String("/")) {
Expand Down Expand Up @@ -336,63 +257,29 @@
return QStringLiteral("/");
}

bool EditLocallyJob::isTokenValid(const QString &token)
{
if (token.isEmpty()) {
return false;
}

// Token is an alphanumeric string 128 chars long.
// Ensure that is what we received and what we are sending to the server.
static const QRegularExpression tokenRegex("^[a-zA-Z0-9]{128}$");
const auto regexMatch = tokenRegex.match(token);

return regexMatch.hasMatch();
}

bool EditLocallyJob::isRelPathValid(const QString &relPath)
{
if (relPath.isEmpty()) {
return false;
}

// We want to check that the path is canonical and not relative
// (i.e. that it doesn't contain ../../) but we always receive
// a relative path, so let's make it absolute by prepending a
// slash
const auto slashPrefixedPath = prefixSlashToPath(relPath);

// Let's check that the filepath is canonical, and that the request
// contains no funny behaviour regarding paths
const auto cleanedPath = QDir::cleanPath(slashPrefixedPath);

if (cleanedPath != slashPrefixedPath) {
return false;
}

return true;
}

OCC::Folder *EditLocallyJob::findFolderForFile(const QString &relPath, const QString &userId)
{
if (relPath.isEmpty()) {
return nullptr;
}

const auto folderMap = FolderMan::instance()->map();

const auto relPathSplit = relPath.split(QLatin1Char('/'));

// a file is on the first level of remote root, so, we just need a proper folder that points to a remote root
if (relPathSplit.size() == 1) {
const auto foundIt = std::find_if(std::begin(folderMap), std::end(folderMap), [&userId](const OCC::Folder *folder) {
return folder->remotePath() == QStringLiteral("/") && folder->accountState()->account()->userIdAtHostWithPort() == userId;
const auto foundIt = std::find_if(std::begin(folderMap),
std::end(folderMap),
[&userId](const OCC::Folder *folder) {
const auto folderUserId = folder->accountState()->account()->userIdAtHostWithPort();
return folder->remotePath() == QStringLiteral("/") && folderUserId == userId;
});

return foundIt != std::end(folderMap) ? foundIt.value() : nullptr;
}

const auto relPathWithSlash = relPath.startsWith(QStringLiteral("/")) ? relPath : QStringLiteral("/") + relPath;
const auto relPathWithSlash =
relPath.startsWith(QStringLiteral("/")) ? relPath : QStringLiteral("/") + relPath;

for (const auto &folder : folderMap) {
// make sure we properly handle folders with non-root(nested) remote paths
Expand Down Expand Up @@ -420,42 +307,11 @@

void EditLocallyJob::showError(const QString &message, const QString &informativeText)
{
Systray::instance()->destroyEditFileLocallyLoadingDialog();
showErrorNotification(message, informativeText);
// to make sure the error is not missed, show a message box in addition
showErrorMessageBox(message, informativeText);
Systray::instance()->destroyEditFileLocallyLoadingDialog();
EditLocallyManager::showError(message, informativeText);
Q_EMIT error(message, informativeText);
}

void EditLocallyJob::showErrorNotification(const QString &message, const QString &informativeText) const
{
if (!_accountState || !_accountState->account()) {
return;
}

const auto folderMap = FolderMan::instance()->map();
const auto foundFolder = std::find_if(folderMap.cbegin(), folderMap.cend(), [this](const auto &folder) {
return _accountState->account()->davUrl() == folder->remoteUrl();
});

if (foundFolder != folderMap.cend()) {
emit (*foundFolder)->syncEngine().addErrorToGui(SyncFileItem::SoftError, message, informativeText, OCC::ErrorCategory::GenericError);
}
}

void EditLocallyJob::showErrorMessageBox(const QString &message, const QString &informativeText) const
{
const auto messageBox = new QMessageBox;
messageBox->setAttribute(Qt::WA_DeleteOnClose);
messageBox->setText(message);
messageBox->setInformativeText(informativeText);
messageBox->setIcon(QMessageBox::Warning);
messageBox->addButton(QMessageBox::StandardButton::Ok);
messageBox->show();
messageBox->activateWindow();
messageBox->raise();
}

void EditLocallyJob::startEditLocally()
{
if (_fileName.isEmpty() || _localFilePath.isEmpty() || !_folderForFile) {
Expand All @@ -468,8 +324,6 @@
return;
}

Systray::instance()->createEditFileLocallyLoadingDialog(_fileName);

if (_folderForFile->isSyncRunning()) {
// in case sync is already running - terminate it and start a new one
_syncTerminatedConnection = connect(_folderForFile, &Folder::syncFinished, this, [this]() {
Expand Down
13 changes: 1 addition & 12 deletions src/gui/editlocallyjob.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/gui/editlocallyjob.h

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/gui/editlocallyjob.h

File src/gui/editlocallyjob.h does not conform to Custom style guidelines. (lines 35)
* Copyright (C) by Claudio Cambra <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand All @@ -14,7 +14,7 @@

#pragma once

#include <QObject>

Check failure on line 17 in src/gui/editlocallyjob.h

View workflow job for this annotation

GitHub Actions / build

src/gui/editlocallyjob.h:17:10 [clang-diagnostic-error]

'QObject' file not found

#include "accountstate.h"
#include "syncfileitem.h"
Expand All @@ -32,13 +32,10 @@
Q_OBJECT

public:
explicit EditLocallyJob(const QString &userId,
explicit EditLocallyJob(const AccountStatePtr &accountState,
const QString &relPath,
const QString &token,
QObject *parent = nullptr);

[[nodiscard]] static bool isTokenValid(const QString &token);
[[nodiscard]] static bool isRelPathValid(const QString &relPath);
[[nodiscard]] static OCC::Folder *findFolderForFile(const QString &relPath, const QString &userId);
[[nodiscard]] static QString prefixSlashToPath(const QString &path);

Expand All @@ -55,15 +52,11 @@
void fetchRemoteFileParentInfo();
void startSyncBeforeOpening();

void startTokenRemoteCheck();
void proceedWithSetup();
void findAfolderAndConstructPaths();

void showError(const QString &message, const QString &informativeText);
void showErrorNotification(const QString &message, const QString &informativeText) const;
void showErrorMessageBox(const QString &message, const QString &informativeText) const;

void remoteTokenCheckResultReceived(const int statusCode);
void slotItemDiscovered(const OCC::SyncFileItemPtr &item);
void slotItemCompleted(const OCC::SyncFileItemPtr &item);

Expand Down Expand Up @@ -92,16 +85,12 @@

[[nodiscard]] bool isFileParentItemValid() const;

bool _tokenVerified = false;

bool _shouldScheduleFolderSyncAfterFileIsOpened = false;

AccountStatePtr _accountState;
QString _userId;
QString _relPath; // full remote path for a file (as on the server)
QString _relativePathToRemoteRoot; // (relative path - Folder::remotePath()) for folders pointing to a non-root remote path e.g. '/subfolder' instead of '/'
QString _relPathParent; // a folder where the file resides ('/' if it is in the first level of a remote root, or e.g. a '/subfolder/a/b/c if it resides in a nested folder)
QString _token;
SyncFileItemPtr _fileParentItem;

QString _fileName;
Expand Down
Loading
Loading