diff --git a/src/framework/cloud/cloudtypes.h b/src/framework/cloud/cloudtypes.h index 61b3079d91183..2e40802a92b27 100644 --- a/src/framework/cloud/cloudtypes.h +++ b/src/framework/cloud/cloudtypes.h @@ -22,6 +22,7 @@ #ifndef MUSE_CLOUD_CLOUDTYPES_H #define MUSE_CLOUD_CLOUDTYPES_H +#include #include #include @@ -30,6 +31,8 @@ #include "types/id.h" +#include "musescorecom/converttypes.h" + namespace muse::cloud { static const QString MUSESCORE_COM_CLOUD_CODE = "musescorecom"; static const QString AUDIO_COM_CLOUD_CODE = "audiocom"; @@ -126,6 +129,19 @@ struct ScoreInfo { } }; +struct ScoreConversionInfo { + int id = 0; + ConvertType type = ConvertType::Omr; + ConvertStatus status = ConvertStatus::Unknown; + + bool operator==(const ScoreConversionInfo& other) const + { + return id == other.id + && type == other.type + && status == other.status; + } +}; + struct ScoresList { struct Item { int id = 0; @@ -135,6 +151,7 @@ struct ScoresList { QString thumbnailUrl; Visibility visibility = Visibility::Private; int viewCount = 0; + std::optional conversion; //! set if the score originated from a conversion }; std::vector items; diff --git a/src/framework/cloud/internal/abstractcloudservice.cpp b/src/framework/cloud/internal/abstractcloudservice.cpp index 374b32d8871ee..a6af98efbd030 100644 --- a/src/framework/cloud/internal/abstractcloudservice.cpp +++ b/src/framework/cloud/internal/abstractcloudservice.cpp @@ -358,14 +358,21 @@ Promise AbstractCloudService::checkCloudIsAvailableAsync() const } Progress progressVal = progress.val; + std::shared_ptr finished = std::make_shared(false); QTimer* timer = new QTimer(); timer->setSingleShot(true); - QObject::connect(timer, &QTimer::timeout, [progressVal]() mutable { - progressVal.cancel(); + QObject::connect(timer, &QTimer::timeout, [progressVal, finished]() mutable { + if (!*finished) { + progressVal.cancel(); + } }); - progressVal.finished().onReceive(this, [resolve, timer](const ProgressResult& res) { + progressVal.finished().onReceive(this, [resolve, timer, finished](const ProgressResult& res) { + if (*finished) { + return; + } + *finished = true; timer->stop(); timer->deleteLater(); (void)resolve(res.ret); diff --git a/src/framework/cloud/musescorecom/converttypes.h b/src/framework/cloud/musescorecom/converttypes.h index d28383873d631..4026fe78ed72f 100644 --- a/src/framework/cloud/musescorecom/converttypes.h +++ b/src/framework/cloud/musescorecom/converttypes.h @@ -22,8 +22,9 @@ #pragma once +#include +#include #include -#include #include #include @@ -31,14 +32,17 @@ #include #include -#include "global/logstream.h" +#include "global/types/bytearray.h" #include "global/types/flags.h" -#include "io/path.h" +#include "global/io/path.h" +#include "global/logstream.h" namespace muse::cloud { enum class ConvertType { - Omr, - Audio2Score + Omr = 0, + Audio2Score, + + Last = Audio2Score }; enum class ConvertStatus { @@ -87,13 +91,12 @@ enum class ConvertErrorCode { TooComplex, DontRecognizeNotes, GeneralFailure, + BadParams, }; //! NOTE: key for ConvertErrorCode stored in Ret::data static const std::string CONVERT_ERROR_CODE_KEY("errorCode"); -static const qint64 MAX_CONVERT_FILE_SIZE_BYTES = 1024LL * 1024 * 1024; // 1 GB - enum class LinkSource { NoSources = 0x0, YouTube = 0x1, @@ -141,67 +144,40 @@ struct ConvertConfig { Audio2ScoreConfig audio2score; }; -struct OmrConvertInput { - muse::io::paths_t paths; +struct ConvertFileData { + muse::ByteArray data; + muse::io::path_t fileName; // basename, with extension }; +using ConvertFileDataList = std::vector; -struct Audio2ScoreConvertInput { - std::variant data; // paths or link +struct ConvertUploadData { + ConvertType type = ConvertType::Omr; + ConvertFileDataList files; + QUrl link; // Audio2Score only + QString filename; // desired name for the converted score, no extension }; - -using ConvertInput = std::variant; - -inline ConvertType convertTypeOf(const ConvertInput& input) -{ - return std::holds_alternative(input) ? ConvertType::Omr : ConvertType::Audio2Score; -} - -inline muse::io::paths_t convertPathsOf(const ConvertInput& input) -{ - if (const OmrConvertInput* omr = std::get_if(&input)) { - return omr->paths; - } - - const muse::io::paths_t* paths = std::get_if(&std::get(input).data); - return paths ? *paths : muse::io::paths_t(); -} - -inline QString convertLinkOf(const ConvertInput& input) -{ - const Audio2ScoreConvertInput* a2s = std::get_if(&input); - if (!a2s) { - return QString(); - } - - const QString* link = std::get_if(&a2s->data); - return link ? *link : QString(); -} +using ConvertUploadDataPtr = std::shared_ptr; struct ConvertResult { int id = 0; ConvertType type = ConvertType::Omr; - ConvertStatus status = ConvertStatus::Processing; + ConvertStatus status = ConvertStatus::Unknown; }; struct ConvertQueueItem { int id = 0; ConvertType type = ConvertType::Omr; - ConvertStatus status = ConvertStatus::Processing; + ConvertStatus status = ConvertStatus::Unknown; QString filename; QString link; //! audio2score only - int scoreId = 0; - QDateTime createdAt; - QDateTime updatedAt; + std::optional scoreId; //! set once the score is ready (AwaitingReview/Done) + QDateTime dataCreated; + QDateTime dataUpdated; ConvertErrorCode errorCode = ConvertErrorCode::Unknown; }; using ConvertQueueList = std::vector; -struct SignedMsczUrl { - QUrl url; - int expiresInSeconds = 0; -}; - //! NOTE: must be in sync with the musescore.com API enum class ReviewRating { Bad = 0, @@ -220,8 +196,8 @@ inline muse::logger::Stream& operator<<(muse::logger::Stream& s, const muse::clo << ", link: \"" << item.link << "\"" << ", type: " << muse::cloud::convertTypeToString(item.type) << ", status: " << muse::cloud::convertStatusToString(item.status) - << ", scoreId: " << item.scoreId - << ", createdAt: " << dateTimeToString(item.createdAt) - << ", updatedAt: " << dateTimeToString(item.updatedAt); + << ", scoreId: " << (item.scoreId ? QString::number(*item.scoreId) : QString("none")) + << ", dataCreated: " << dateTimeToString(item.dataCreated) + << ", dataUpdated: " << dateTimeToString(item.dataUpdated); return s; } diff --git a/src/framework/cloud/musescorecom/imusescorecomconvertservice.h b/src/framework/cloud/musescorecom/imusescorecomconvertservice.h index 83ef1014b8e08..2819ae5202c46 100644 --- a/src/framework/cloud/musescorecom/imusescorecomconvertservice.h +++ b/src/framework/cloud/musescorecom/imusescorecomconvertservice.h @@ -29,9 +29,6 @@ #include "converttypes.h" -class QIODevice; -using DevicePtr = std::shared_ptr; - namespace muse::cloud { /// fetchConfig() can be called at any time (no authenticated user required) to get the /// upload limits (max file size, page/image counts, allowed types) for client-side validation @@ -39,14 +36,13 @@ namespace muse::cloud { /// /// Expected call order for a conversion (OMR or Audio2Score): /// 1. upload() to submit the file(s) and start processing -/// 2. Poll fetchQueue() and watch the item's status -/// 3. As soon as the status is AwaitingReview or Done, the MSCZ is already -/// available: call fetchMsczUrl() then downloadConvertedScore() to get the score -/// 4. Rating the recognition quality (submitReview(), once AwaitingReview) is optional -/// and does not gate the download above; submitReviewComment() may attach a -/// comment afterwards, once the review has been submitted -/// 5. Keep polling fetchQueue() until the status is Failed, or the item disappears +/// 2. Poll fetchQueue() and watch the item's status; once it's AwaitingReview or Done, its +/// scoreId identifies the resulting score, already available via IMuseScoreComService +/// 3. Rating the recognition quality (submitReview(), once AwaitingReview) is optional; +/// submitReviewComment() may attach a comment afterwards, once the review has been submitted +/// 4. Keep polling fetchQueue() until the status is Failed, or the item disappears /// from the queue (which should be treated the same as Done) +/// 5. deleteConversion() may be called at any point to remove an item from the queue class IMuseScoreComConvertService : MODULE_CONTEXT_INTERFACE { INTERFACE_ID(IMuseScoreComConvertService) @@ -56,15 +52,15 @@ class IMuseScoreComConvertService : MODULE_CONTEXT_INTERFACE virtual async::Promise > fetchConfig() = 0; - virtual ProgressPtr upload(const ConvertInput& input) = 0; - virtual ProgressPtr downloadConvertedScore(const SignedMsczUrl& urlInfo, DevicePtr scoreData) = 0; + virtual ProgressPtr upload(const ConvertUploadDataPtr& data) = 0; virtual async::Promise > fetchQueue() = 0; - virtual async::Promise > fetchMsczUrl(ConvertType type, int id) = 0; virtual async::Promise > submitReview(ConvertType type, int id, ReviewRating review, const QString& comment = QString()) = 0; virtual async::Promise > submitReviewComment(ConvertType type, int id, const QString& comment) = 0; + + virtual async::Promise deleteConversion(ConvertType type, int id) = 0; }; using IMuseScoreComConvertServicePtr = std::shared_ptr; } diff --git a/src/framework/cloud/musescorecom/musescorecomservice.cpp b/src/framework/cloud/musescorecom/musescorecomservice.cpp index f61555066b6ea..0befcc40ade4a 100644 --- a/src/framework/cloud/musescorecom/musescorecomservice.cpp +++ b/src/framework/cloud/musescorecom/musescorecomservice.cpp @@ -23,8 +23,6 @@ #include "musescorecomservice.h" #include -#include -#include #include #include #include @@ -46,11 +44,12 @@ using namespace muse::async; static const QString MUSESCORECOM_CLOUD_TITLE("MuseScore.com"); static const QString MUSESCORECOM_CLOUD_URL("https://musescore.com"); static const QString MUSESCORECOM_API_ROOT_URL("https://desktop.musescore.com/editor/v1"); +static const QString MUSESCORECOM_API_ROOT_URL_V2("https://desktop.musescore.com/editor/v2"); static const QUrl MUSESCORECOM_SCORE_MANAGER_URL(MUSESCORECOM_CLOUD_URL + "/my-scores"); static const QUrl MUSESCORECOM_USER_INFO_API_URL(MUSESCORECOM_API_ROOT_URL + "/me"); static const QUrl MUSESCORECOM_SCORE_INFO_API_URL(MUSESCORECOM_API_ROOT_URL + "/score/info"); -static const QUrl MUSESCORECOM_SCORES_LIST_API_URL(MUSESCORECOM_API_ROOT_URL + "/collection/scores"); +static const QUrl MUSESCORECOM_SCORES_LIST_API_URL(MUSESCORECOM_API_ROOT_URL_V2 + "/collection/scores"); static const QUrl MUSESCORECOM_SCORE_DOWNLOAD_API_URL(MUSESCORECOM_API_ROOT_URL + "/score/download"); static const QUrl MUSESCORECOM_SCORE_DOWNLOAD_SHARED_API_URL(MUSESCORECOM_API_ROOT_URL + "/score/download-shared"); static const QUrl MUSESCORECOM_UPLOAD_SCORE_API_URL(MUSESCORECOM_API_ROOT_URL + "/score/upload"); @@ -58,8 +57,9 @@ static const QUrl MUSESCORECOM_UPLOAD_AUDIO_API_URL(MUSESCORECOM_API_ROOT_URL + static const QUrl MUSESCORECOM_CONVERT_CONFIG_URL("https://musescore.com/static/musescore/studio/upload-config.json"); static const QUrl MUSESCORECOM_CONVERT_UPLOAD_API_URL(MUSESCORECOM_API_ROOT_URL + "/score/convert/convert"); +//! NOTE: same path as the upload endpoint, DELETE instead of POST +static const QUrl MUSESCORECOM_CONVERT_DELETE_API_URL = MUSESCORECOM_CONVERT_UPLOAD_API_URL; static const QUrl MUSESCORECOM_CONVERT_QUEUE_API_URL(MUSESCORECOM_API_ROOT_URL + "/score/convert/queue"); -static const QUrl MUSESCORECOM_CONVERT_MSCZ_API_URL(MUSESCORECOM_API_ROOT_URL + "/score/convert/mscz"); static const QUrl MUSESCORECOM_CONVERT_REVIEW_API_URL(MUSESCORECOM_API_ROOT_URL + "/score/convert/review"); static const QUrl MUSESCORECOM_CONVERT_COMMENT_API_URL(MUSESCORECOM_API_ROOT_URL + "/score/convert/comment"); @@ -96,6 +96,98 @@ static RetVal parseMuseScoreComAccountInfo(const QByteArray& data) return RetVal::make_ok(info); } +static QString convertTypeToApiString(ConvertType type) +{ + switch (type) { + case ConvertType::Omr: return "omr"; + case ConvertType::Audio2Score: return "audio2score"; + } + + return QString(); +} + +static ConvertType convertTypeFromApiString(const QString& str) +{ + if (str == "omr") { + return ConvertType::Omr; + } + + if (str == "audio2score") { + return ConvertType::Audio2Score; + } + + LOGW() << "Unknown convert type: \"" << str << "\", falling back to Omr"; + return ConvertType::Omr; +} + +static ConvertStatus convertStatusFromApiString(const QString& str) +{ + if (str == "processing") { + return ConvertStatus::Processing; + } else if (str == "awaiting_review") { + return ConvertStatus::AwaitingReview; + } else if (str == "done") { + return ConvertStatus::Done; + } else if (str == "failed") { + return ConvertStatus::Failed; + } + + return ConvertStatus::Unknown; +} + +static ConvertErrorCode convertErrorCodeFromApiString(const QString& str) +{ + if (str == "unsupported_format") { + return ConvertErrorCode::UnsupportedFormat; + } else if (str == "file_too_large") { + return ConvertErrorCode::FileTooLarge; + } else if (str == "too_many_files") { + return ConvertErrorCode::TooManyFiles; + } else if (str == "file_or_link_required") { + return ConvertErrorCode::FileOrLinkRequired; + } else if (str == "invalid_link") { + return ConvertErrorCode::InvalidLink; + } else if (str == "rate_limited") { + return ConvertErrorCode::RateLimited; + } else if (str == "mscz_not_ready") { + return ConvertErrorCode::MsczNotReady; + } else if (str == "no_need_review") { + return ConvertErrorCode::NoNeedReview; + } else if (str == "review_required") { + return ConvertErrorCode::ReviewRequired; + } else if (str == "comment_required") { + return ConvertErrorCode::CommentRequired; + } else if (str == "mu_status_various_file_issues") { + return ConvertErrorCode::VariousFileIssues; + } else if (str == "mu_status_too_complex") { + return ConvertErrorCode::TooComplex; + } else if (str == "mu_status_dont_recognize_notes") { + return ConvertErrorCode::DontRecognizeNotes; + } else if (str == "mu_status_general_failure") { + return ConvertErrorCode::GeneralFailure; + } else if (str == "bad_params") { + return ConvertErrorCode::BadParams; + } + + return ConvertErrorCode::Unknown; +} + +static std::optional parseScoreConversionInfo(const QJsonObject& itemObj) +{ + if (!itemObj.value("conversion").isObject()) { + return std::nullopt; + } + + QJsonObject conversionObj = itemObj.value("conversion").toObject(); + + ScoreConversionInfo result; + result.id = conversionObj.value("id").toInt(); + result.type = convertTypeFromApiString(conversionObj.value("type").toString()); + result.status = convertStatusFromApiString(conversionObj.value("status").toString()); + + return result; +} + static RetVal parseScoreList(const QByteArray& data, int batchNumber) { QJsonParseError err; @@ -134,6 +226,7 @@ static RetVal parseScoreList(const QByteArray& data, int batchNumber item.thumbnailUrl = itemObj.value("thumbnails").toObject().value("small").toString(); item.visibility = static_cast(itemObj.value("privacy").toInt()); item.viewCount = itemObj.value("view_count").toInt(); + item.conversion = parseScoreConversionInfo(itemObj); result.items.push_back(item); } @@ -259,80 +352,6 @@ static QHttpMultiPartPtr makeMultiPartForAudioUpload(QIODevice* audioData, const return multiPart; } -static QString convertTypeToApiString(ConvertType type) -{ - switch (type) { - case ConvertType::Omr: return "omr"; - case ConvertType::Audio2Score: return "audio2score"; - } - - return QString(); -} - -static ConvertType convertTypeFromApiString(const QString& str) -{ - if (str == "omr") { - return ConvertType::Omr; - } - - if (str == "audio2score") { - return ConvertType::Audio2Score; - } - - LOGW() << "Unknown convert type: \"" << str << "\", falling back to Omr"; - return ConvertType::Omr; -} - -static ConvertStatus convertStatusFromApiString(const QString& str) -{ - if (str == "processing") { - return ConvertStatus::Processing; - } else if (str == "awaiting_review") { - return ConvertStatus::AwaitingReview; - } else if (str == "done") { - return ConvertStatus::Done; - } else if (str == "failed") { - return ConvertStatus::Failed; - } - - return ConvertStatus::Unknown; -} - -static ConvertErrorCode convertErrorCodeFromApiString(const QString& str) -{ - if (str == "unsupported_format") { - return ConvertErrorCode::UnsupportedFormat; - } else if (str == "file_too_large") { - return ConvertErrorCode::FileTooLarge; - } else if (str == "too_many_files") { - return ConvertErrorCode::TooManyFiles; - } else if (str == "file_or_link_required") { - return ConvertErrorCode::FileOrLinkRequired; - } else if (str == "invalid_link") { - return ConvertErrorCode::InvalidLink; - } else if (str == "rate_limited") { - return ConvertErrorCode::RateLimited; - } else if (str == "mscz_not_ready") { - return ConvertErrorCode::MsczNotReady; - } else if (str == "no_need_review") { - return ConvertErrorCode::NoNeedReview; - } else if (str == "review_required") { - return ConvertErrorCode::ReviewRequired; - } else if (str == "comment_required") { - return ConvertErrorCode::CommentRequired; - } else if (str == "mu_status_various_file_issues") { - return ConvertErrorCode::VariousFileIssues; - } else if (str == "mu_status_too_complex") { - return ConvertErrorCode::TooComplex; - } else if (str == "mu_status_dont_recognize_notes") { - return ConvertErrorCode::DontRecognizeNotes; - } else if (str == "mu_status_general_failure") { - return ConvertErrorCode::GeneralFailure; - } - - return ConvertErrorCode::Unknown; -} - static void appendServerErrorCode(Ret& ret, const QByteArray& data) { QJsonParseError err; @@ -389,9 +408,12 @@ static RetVal parseConvertQueueList(const QByteArray& data) item.status = convertStatusFromApiString(itemObj.value("status").toString()); item.filename = itemObj.value("filename").toString(); item.link = itemObj.value("link").toString(); - item.scoreId = itemObj.value("score_id").toInt(); - item.createdAt = QDateTime::fromSecsSinceEpoch(itemObj.value("created_at").toInteger()); - item.updatedAt = QDateTime::fromSecsSinceEpoch(itemObj.value("updated_at").toInteger()); + const QJsonValue scoreIdVal = itemObj.value("score_id"); + if (scoreIdVal.isDouble()) { + item.scoreId = scoreIdVal.toInt(); + } + item.dataCreated = QDateTime::fromSecsSinceEpoch(itemObj.value("date_created").toInteger()); + item.dataUpdated = QDateTime::fromSecsSinceEpoch(itemObj.value("date_updated").toInteger()); item.errorCode = convertErrorCodeFromApiString(itemObj.value("error_code").toString()); result.push_back(item); @@ -400,23 +422,6 @@ static RetVal parseConvertQueueList(const QByteArray& data) return RetVal::make_ok(result); } -static RetVal parseSignedMsczUrl(const QByteArray& data) -{ - QJsonParseError err; - QJsonDocument doc = QJsonDocument::fromJson(data, &err); - if (err.error != QJsonParseError::NoError || !doc.isObject()) { - return RetVal::make_ret((int)Ret::Code::InternalError, err.errorString().toStdString()); - } - - QJsonObject obj = doc.object(); - - SignedMsczUrl result; - result.url = QUrl(obj.value("url").toString()); - result.expiresInSeconds = obj.value("expires_in").toInt(); - - return RetVal::make_ok(result); -} - static LinkSource linkSourceFromApiString(const QString& str) { if (str.compare("youtube", Qt::CaseInsensitive) == 0) { @@ -493,9 +498,8 @@ static QString sanitizeContentDispositionFilename(const QString& fileName) return sanitized; } -using ConvertFileList = std::vector >; - -static QHttpMultiPartPtr makeMultiPartForConvertUpload(ConvertType type, const ConvertFileList& files, const QString& link) +static QHttpMultiPartPtr makeMultiPartForConvertUpload(ConvertType type, const ConvertFileDataList& files, const QUrl& link, + const QString& filename) { auto multiPart = std::make_shared(QHttpMultiPart::FormDataType); @@ -504,23 +508,31 @@ static QHttpMultiPartPtr makeMultiPartForConvertUpload(ConvertType type, const C typePart.setBody(convertTypeToApiString(type).toUtf8()); multiPart->append(typePart); + if (!filename.isEmpty()) { + QHttpPart filenamePart; + filenamePart.setHeader(QNetworkRequest::ContentDispositionHeader, QVariant("form-data; name=\"filename\"")); + filenamePart.setBody(filename.toUtf8()); + multiPart->append(filenamePart); + } + if (!link.isEmpty()) { QHttpPart linkPart; linkPart.setHeader(QNetworkRequest::ContentDispositionHeader, QVariant("form-data; name=\"link\"")); - linkPart.setBody(link.toUtf8()); + linkPart.setBody(link.toString().toUtf8()); multiPart->append(linkPart); } QMimeDatabase mimeDb; - for (const std::shared_ptr& file : files) { - const QString fileName = file->fileName(); - const QString baseName = QFileInfo(fileName).fileName(); + for (const ConvertFileData& file : files) { + const QString fileName = file.fileName.toQString(); QHttpPart filePart; filePart.setHeader(QNetworkRequest::ContentTypeHeader, QVariant(mimeDb.mimeTypeForFile(fileName).name())); QString contentDisposition - = QString("form-data; name=\"files[]\"; filename=\"%1\"").arg(sanitizeContentDispositionFilename(baseName)); + = QString("form-data; name=\"files[]\"; filename=\"%1\"").arg(sanitizeContentDispositionFilename(fileName)); filePart.setHeader(QNetworkRequest::ContentDispositionHeader, QVariant(contentDisposition)); - filePart.setBodyDevice(file.get()); + // NOTE: safe to avoid copying the bytes since the caller keeps ConvertUploadData alive + // (via a shared_ptr) for the whole async upload, not just this synchronous setup + filePart.setBody(file.data.toQByteArrayNoCopy()); multiPart->append(filePart); } @@ -1023,13 +1035,13 @@ Promise > MuseScoreComService::fetchConfig() }); } -ProgressPtr MuseScoreComService::upload(const ConvertInput& input) +ProgressPtr MuseScoreComService::upload(const ConvertUploadDataPtr& data) { ProgressPtr progress = std::make_shared(); progress->start(); - executeAsyncRequest([this, input, progress]() { - return doUpload(input, progress); + executeAsyncRequest([this, data, progress]() { + return doUpload(data, progress); }).onResolve(this, [progress](const Ret& ret) { if (progress->isStarted()) { progress->finish(ret); @@ -1039,36 +1051,17 @@ ProgressPtr MuseScoreComService::upload(const ConvertInput& input) return progress; } -Promise MuseScoreComService::doUpload(const ConvertInput& input, ProgressPtr progress) +Promise MuseScoreComService::doUpload(const ConvertUploadDataPtr& data, ProgressPtr progress) { TRACEFUNC; - return make_promise([this, input, progress](auto resolve, auto) { + return make_promise([this, data, progress](auto resolve, auto) { RetVal uploadUrl = prepareUrlForRequest(MUSESCORECOM_CONVERT_UPLOAD_API_URL); if (!uploadUrl.ret) { return resolve(uploadUrl.ret); } - const ConvertType type = convertTypeOf(input); - const QString link = convertLinkOf(input); - - ConvertFileList files; - for (const io::path_t& path : convertPathsOf(input)) { - auto file = std::make_shared(path.toQString()); - if (!file->open(QIODevice::ReadOnly)) { - return resolve(make_ret(Err::InvalidData)); - } - - if (file->size() > MAX_CONVERT_FILE_SIZE_BYTES) { - Ret ret = make_ret(Err::Status422_ValidationFailed); - ret.setData(CONVERT_ERROR_CODE_KEY, ConvertErrorCode::FileTooLarge); - return resolve(ret); - } - - files.push_back(file); - } - - auto multiPart = makeMultiPartForConvertUpload(type, files, link); + auto multiPart = makeMultiPartForConvertUpload(data->type, data->files, data->link, data->filename); auto receivedData = std::make_shared(); RetVal uploadProgress = m_networkManager->post(uploadUrl.val, multiPart, receivedData, headers()); @@ -1080,9 +1073,7 @@ Promise MuseScoreComService::doUpload(const ConvertInput& input, ProgressPt progress->progress(current, total, msg); }); - //! NOTE: files must stay alive (and open) until the request finishes, - //! since multiPart's file parts hold raw pointers into them - uploadProgress.val.finished().onReceive(this, [this, files, receivedData, resolve, progress](const ProgressResult& res) { + uploadProgress.val.finished().onReceive(this, [this, data, receivedData, resolve, progress](const ProgressResult& res) { if (!res.ret) { printServerReply(*receivedData); Ret ret = uploadingDownloadingRetFromRawRet(res.ret); @@ -1110,37 +1101,6 @@ Promise MuseScoreComService::doUpload(const ConvertInput& input, ProgressPt }); } -ProgressPtr MuseScoreComService::downloadConvertedScore(const SignedMsczUrl& urlInfo, DevicePtr scoreData) -{ - TRACEFUNC; - - ProgressPtr progress = std::make_shared(); - progress->start(); - - IF_ASSERT_FAILED(urlInfo.url.isValid()) { - progress->finish(make_ret(Err::InvalidData)); - return progress; - } - - //! NOTE: urlInfo.url is already a signed URL, so it must be - //! requested as-is, without going through prepareUrlForRequest - RetVal getProgress = m_networkManager->get(urlInfo.url, scoreData, headers()); - if (!getProgress.ret) { - progress->finish(getProgress.ret); - return progress; - } - - getProgress.val.progressChanged().onReceive(this, [progress](int64_t current, int64_t total, const std::string& msg) { - progress->progress(current, total, msg); - }); - - getProgress.val.finished().onReceive(this, [this, progress](const ProgressResult& res) { - progress->finish(uploadingDownloadingRetFromRawRet(res.ret)); - }); - - return progress; -} - Promise > MuseScoreComService::fetchQueue() { return Promise >([this](auto resolve, auto) { @@ -1171,22 +1131,19 @@ Promise > MuseScoreComService::fetchQueue() }); } -Promise > MuseScoreComService::fetchMsczUrl(ConvertType type, int id) +Promise > MuseScoreComService::submitReview(ConvertType type, int id, ReviewRating review, const QString& comment) { - return Promise >([this, type, id](auto resolve, auto) { - QVariantMap params; - params["id"] = id; - params["type"] = convertTypeToApiString(type); - - RetVal msczUrl = prepareUrlForRequest(MUSESCORECOM_CONVERT_MSCZ_API_URL, params); - if (!msczUrl.ret) { - return resolve(RetVal::make_ret(msczUrl.ret)); + return Promise >([this, type, id, review, comment](auto resolve, auto) { + RetVal url = prepareUrlForRequest(MUSESCORECOM_CONVERT_REVIEW_API_URL); + if (!url.ret) { + return resolve(RetVal::make_ret(url.ret)); } + auto multiPart = makeMultiPartForReview(type, id, review, comment); auto receivedData = std::make_shared(); - RetVal progress = m_networkManager->get(msczUrl.val, receivedData, headers()); + RetVal progress = m_networkManager->post(url.val, multiPart, receivedData, headers()); if (!progress.ret) { - return resolve(RetVal::make_ret(progress.ret)); + return resolve(RetVal::make_ret(progress.ret)); } progress.val.finished().onReceive(this, [this, receivedData, resolve](const ProgressResult& res) { @@ -1194,30 +1151,33 @@ Promise > MuseScoreComService::fetchMsczUrl(ConvertType ty printServerReply(*receivedData); Ret ret = uploadingDownloadingRetFromRawRet(res.ret); appendServerErrorCode(ret, receivedData->data()); - (void)resolve(RetVal::make_ret(ret)); + (void)resolve(RetVal::make_ret(ret)); return; } - (void)resolve(parseSignedMsczUrl(receivedData->data())); + (void)resolve(parseConvertResult(receivedData->data())); }); - return Promise >::dummy_result(); + return Promise >::dummy_result(); }); } -Promise > MuseScoreComService::submitReview(ConvertType type, int id, ReviewRating review, const QString& comment) +Promise MuseScoreComService::deleteConversion(ConvertType type, int id) { - return Promise >([this, type, id, review, comment](auto resolve, auto) { - RetVal url = prepareUrlForRequest(MUSESCORECOM_CONVERT_REVIEW_API_URL); + return Promise([this, type, id](auto resolve, auto) { + QVariantMap params; + params["type"] = convertTypeToApiString(type); + params["id"] = id; + + RetVal url = prepareUrlForRequest(MUSESCORECOM_CONVERT_DELETE_API_URL, params); if (!url.ret) { - return resolve(RetVal::make_ret(url.ret)); + return resolve(url.ret); } - auto multiPart = makeMultiPartForReview(type, id, review, comment); auto receivedData = std::make_shared(); - RetVal progress = m_networkManager->post(url.val, multiPart, receivedData, headers()); + RetVal progress = m_networkManager->del(url.val, receivedData, headers()); if (!progress.ret) { - return resolve(RetVal::make_ret(progress.ret)); + return resolve(progress.ret); } progress.val.finished().onReceive(this, [this, receivedData, resolve](const ProgressResult& res) { @@ -1225,14 +1185,14 @@ Promise > MuseScoreComService::submitReview(ConvertType ty printServerReply(*receivedData); Ret ret = uploadingDownloadingRetFromRawRet(res.ret); appendServerErrorCode(ret, receivedData->data()); - (void)resolve(RetVal::make_ret(ret)); + (void)resolve(ret); return; } - (void)resolve(parseConvertResult(receivedData->data())); + (void)resolve(make_ok()); }); - return Promise >::dummy_result(); + return Promise::dummy_result(); }); } diff --git a/src/framework/cloud/musescorecom/musescorecomservice.h b/src/framework/cloud/musescorecom/musescorecomservice.h index defd95d412635..fec4ef9d8fbbd 100644 --- a/src/framework/cloud/musescorecom/musescorecomservice.h +++ b/src/framework/cloud/musescorecom/musescorecomservice.h @@ -67,16 +67,16 @@ class MuseScoreComService : public IMuseScoreComService, public IMuseScoreComCon // IMuseScoreComConvertService async::Promise > fetchConfig() override; - ProgressPtr upload(const ConvertInput& input) override; - ProgressPtr downloadConvertedScore(const SignedMsczUrl& urlInfo, DevicePtr scoreData) override; + ProgressPtr upload(const ConvertUploadDataPtr& data) override; async::Promise > fetchQueue() override; - async::Promise > fetchMsczUrl(ConvertType type, int id) override; async::Promise > submitReview(ConvertType type, int id, ReviewRating review, const QString& comment = QString()) override; async::Promise > submitReviewComment(ConvertType type, int id, const QString& comment) override; + async::Promise deleteConversion(ConvertType type, int id) override; + private: ServerConfig serverConfig() const override; @@ -96,6 +96,6 @@ class MuseScoreComService : public IMuseScoreComService, public IMuseScoreComCon async::Promise doUploadAudio(DevicePtr audioData, const QString& audioFormat, const QUrl& sourceUrl, ProgressPtr progress); - async::Promise doUpload(const ConvertInput& input, ProgressPtr progress); + async::Promise doUpload(const ConvertUploadDataPtr& data, ProgressPtr progress); }; } diff --git a/src/framework/cloud/tests/mocks/musescorecomconvertservicemock.h b/src/framework/cloud/tests/mocks/musescorecomconvertservicemock.h index 3c945716da477..90346f7e9f1cd 100644 --- a/src/framework/cloud/tests/mocks/musescorecomconvertservicemock.h +++ b/src/framework/cloud/tests/mocks/musescorecomconvertservicemock.h @@ -31,13 +31,13 @@ class MuseScoreComConvertServiceMock : public IMuseScoreComConvertService public: MOCK_METHOD(async::Promise >, fetchConfig, (), (override)); - MOCK_METHOD(ProgressPtr, upload, (const ConvertInput&), (override)); - MOCK_METHOD(ProgressPtr, downloadConvertedScore, (const SignedMsczUrl&, DevicePtr), (override)); + MOCK_METHOD(ProgressPtr, upload, (const ConvertUploadDataPtr&), (override)); MOCK_METHOD(async::Promise >, fetchQueue, (), (override)); - MOCK_METHOD(async::Promise >, fetchMsczUrl, (ConvertType, int), (override)); MOCK_METHOD(async::Promise >, submitReview, (ConvertType, int, ReviewRating, const QString&), (override)); MOCK_METHOD(async::Promise >, submitReviewComment, (ConvertType, int, const QString&), (override)); + + MOCK_METHOD(async::Promise, deleteConversion, (ConvertType, int), (override)); }; } diff --git a/src/framework/multiwindows/tests/mocks/multiwindowsprovidermock.h b/src/framework/multiwindows/tests/mocks/multiwindowsprovidermock.h new file mode 100644 index 0000000000000..08ad15803ce4f --- /dev/null +++ b/src/framework/multiwindows/tests/mocks/multiwindowsprovidermock.h @@ -0,0 +1,58 @@ +/* + * SPDX-License-Identifier: GPL-3.0-only + * MuseScore-Studio-CLA-applies + * + * MuseScore Studio + * Music Composition & Notation + * + * Copyright (C) 2026 MuseScore Limited and others + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +#pragma once + +#include + +#include "multiwindows/imultiwindowsprovider.h" + +namespace muse::mi { +class MultiWindowsProviderMock : public IMultiWindowsProvider +{ +public: + MOCK_METHOD(int, windowCount, (), (const, override)); + + MOCK_METHOD(bool, isProjectAlreadyOpened, (const io::path_t&), (const, override)); + MOCK_METHOD(void, activateWindowWithProject, (const io::path_t&), (override)); + MOCK_METHOD(bool, isHasWindowWithoutProject, (), (const, override)); + MOCK_METHOD(void, activateWindowWithoutProject, (const QStringList&), (override)); + MOCK_METHOD(bool, openNewWindow, (const QStringList&), (override)); + + MOCK_METHOD(bool, isPreferencesAlreadyOpened, (), (const, override)); + MOCK_METHOD(void, activateWindowWithOpenedPreferences, (), (const, override)); + MOCK_METHOD(void, settingsBeginTransaction, (), (override)); + MOCK_METHOD(void, settingsCommitTransaction, (), (override)); + MOCK_METHOD(void, settingsRollbackTransaction, (), (override)); + MOCK_METHOD(void, settingsReset, (), (override)); + MOCK_METHOD(void, settingsSetValue, (const std::string&, const Val&), (override)); + + MOCK_METHOD(bool, lockResource, (const std::string&), (override)); + MOCK_METHOD(bool, unlockResource, (const std::string&), (override)); + MOCK_METHOD(void, notifyAboutResourceChanged, (const std::string&), (override)); + MOCK_METHOD(async::Channel, resourceChanged, (), (override)); + + MOCK_METHOD(void, notifyAboutWindowWasQuited, (), (override)); + MOCK_METHOD(void, quitForAll, (), (override)); + MOCK_METHOD(void, quitAllAndRestartLast, (), (override)); + MOCK_METHOD(void, quitAllAndRunInstallation, (const io::path_t&), (override)); +}; +} diff --git a/src/project/iconvertfiletoscorescenario.h b/src/project/iconvertfiletoscorescenario.h index 8af74d6d483fb..c770567cb1908 100644 --- a/src/project/iconvertfiletoscorescenario.h +++ b/src/project/iconvertfiletoscorescenario.h @@ -35,7 +35,7 @@ class QUrl; namespace mu::project { struct ConvertSelection { ConvertInput input; - muse::String convertedFileName; + muse::String convertedScoreName; }; class IConvertFileToScoreScenario : MODULE_CONTEXT_INTERFACE @@ -62,8 +62,9 @@ class IConvertFileToScoreScenario : MODULE_CONTEXT_INTERFACE //! opens the picker with them pre-selected virtual void convertFiles(const muse::io::paths_t& paths = {}) = 0; - //! Emits the result once the server-side conversion completes - virtual muse::async::Channel convertFinished() const = 0; + //! Emits the result once the server-side conversion completes; on success, the converted score + //! is already available in the user's MuseScore.com account, described by the WatchedScore + virtual muse::async::Channel convertFinished() const = 0; }; using IConvertFileToScoreScenarioPtr = std::shared_ptr; diff --git a/src/project/iconvertfiletoscoreservice.h b/src/project/iconvertfiletoscoreservice.h index 90dd3c557f8c2..0b7379edb83e7 100644 --- a/src/project/iconvertfiletoscoreservice.h +++ b/src/project/iconvertfiletoscoreservice.h @@ -53,14 +53,13 @@ class IConvertFileToScoreService : MODULE_CONTEXT_INTERFACE virtual muse::Ret validateLink(const QUrl& link) const = 0; //! Sends the conversion request to the server - virtual muse::Ret startConvert(const ConvertInput& input, const muse::String& convertedFileName) = 0; + virtual muse::Ret startConvert(const ConvertInput& input, const muse::String& convertedScoreName) = 0; - //! Emits the result once the server-side conversion completes - virtual muse::async::Channel convertFinished() const = 0; + //! Emits the final result of a conversion (upload or processing failure, or success with the WatchedScore) + virtual muse::async::Channel convertFinished() const = 0; - //! Names of the files currently being converted server-side (queued, processing, or downloading) - virtual muse::StringList fileNamesBeingConverted() const = 0; - virtual muse::async::Notification fileNamesBeingConvertedChanged() const = 0; + //! All pending/reviewable conversions from the server's convert queue + virtual muse::ValNt watchedScores() const = 0; //! Emitted whenever checking the conversion status fails virtual muse::async::Channel pollingFailed() const = 0; @@ -68,10 +67,13 @@ class IConvertFileToScoreService : MODULE_CONTEXT_INTERFACE //! Resumes polling for any still-pending items - e.g. in response to the user pressing "Retry" virtual void retryPolling() = 0; - //! Emitted once a converted score has been downloaded and is awaiting a quality review - virtual muse::async::Channel reviewRequested() const = 0; - virtual void submitReview(ConvertType type, int itemId, ReviewRating rating, const QString& comment = QString()) = 0; - virtual void submitReviewComment(ConvertType type, int itemId, const QString& comment) = 0; + //! Emitted once a converted score is ready and awaiting a quality review + virtual muse::async::Channel reviewRequested() const = 0; + virtual void submitReview(int scoreId, ReviewRating rating, const QString& comment = QString()) = 0; + virtual void submitReviewComment(int scoreId, const QString& comment) = 0; + + //! Deletes a watched conversion, both server-side and from watchedScores() + virtual void deleteConversion(ConvertType type, int convertId) = 0; }; using IConvertFileToScoreServicePtr = std::shared_ptr; diff --git a/src/project/internal/convertfiletoscorescenario.cpp b/src/project/internal/convertfiletoscorescenario.cpp index d8e970425d363..d5f5c7d6da660 100644 --- a/src/project/internal/convertfiletoscorescenario.cpp +++ b/src/project/internal/convertfiletoscorescenario.cpp @@ -21,7 +21,6 @@ */ #include "convertfiletoscorescenario.h" -#include #include #include @@ -39,6 +38,9 @@ using namespace muse::cloud; //! NOTE: gives the user a moment to land on the score before prompting for a review static constexpr int REVIEW_PROMPT_DELAY_MS = 10000; +//! NOTE: attempt 4 is ~5 minutes into retrying +static constexpr int RETRY_TOAST_ATTEMPT_THRESHOLD = 4; + static ConvertSelection toConvertSelection(const Val& val) { const QVariantMap map = val.toQVariant().toMap(); @@ -53,10 +55,10 @@ static ConvertSelection toConvertSelection(const Val& val) } ConvertSelection selection; - selection.convertedFileName = map.value("convertedFileName").toString(); + selection.convertedScoreName = map.value("convertedScoreName").toString(); if (type == ConvertType::Audio2Score && !link.isEmpty()) { - selection.input = Audio2ScoreConvertInput { link }; + selection.input = Audio2ScoreConvertInput { QUrl(link) }; } else if (type == ConvertType::Audio2Score) { selection.input = Audio2ScoreConvertInput { paths }; } else { @@ -75,21 +77,32 @@ void ConvertFileToScoreScenario::init() { TRACEFUNC; - service()->convertFinished().onReceive(this, [this](const Ret& ret, const io::path_t& path) { + service()->convertFinished().onReceive(this, [this](const Ret& ret, const WatchedScore& watched) { if (ret) { - showScoreReadyNotification(path); + showScoreReadyNotification(watched); } else { showConvertFailedNotification(ret); } - m_convertFinished.send(ret, path); + m_convertFinished.send(ret, watched); }); - service()->reviewRequested().onReceive(this, [this](ConvertType type, int queueId, const io::path_t& path) { - m_pendingReviews[path] = { type, queueId }; + service()->reviewRequested().onReceive(this, [this](int scoreId) { + m_pendingReviews[configuration()->cloudProjectPath(scoreId)] = scoreId; checkPendingReview(); }); + service()->pollingFailed().onReceive(this, [this](const PollingFailure& failure) { + if (failure.attempt == 1) { + m_retryToastShown = false; + } + + if (!m_retryToastShown && failure.attempt >= RETRY_TOAST_ATTEMPT_THRESHOLD) { + m_retryToastShown = true; + showPollingFailureNotification(); + } + }); + globalContext()->currentProjectChanged().onNotify(this, [this]() { checkPendingReview(); }); @@ -107,11 +120,10 @@ void ConvertFileToScoreScenario::checkPendingReview() return; } - const ConvertType type = it->second.first; - const int queueId = it->second.second; + const int scoreId = it->second; const io::path_t path = it->first; - QTimer::singleShot(REVIEW_PROMPT_DELAY_MS, this, [this, type, queueId, path]() { + QTimer::singleShot(REVIEW_PROMPT_DELAY_MS, this, [this, scoreId, path]() { INotationProjectPtr currentProject = globalContext()->currentProject(); if (!currentProject || currentProject->path() != path) { return; @@ -121,7 +133,7 @@ void ConvertFileToScoreScenario::checkPendingReview() return; } - askReviewRating(type, queueId); + askReviewRating(scoreId); }); } @@ -166,7 +178,7 @@ void ConvertFileToScoreScenario::convertFiles(const io::paths_t& paths) if (paths.empty()) { selectFilesToConvert() .onResolve(this, [this](const ConvertSelection& selection) { - startConvert(selection.input, selection.convertedFileName); + startConvert(selection.input, selection.convertedScoreName); }); return; } @@ -180,7 +192,7 @@ void ConvertFileToScoreScenario::convertFiles(const io::paths_t& paths) }); } -async::Channel ConvertFileToScoreScenario::convertFinished() const +async::Channel ConvertFileToScoreScenario::convertFinished() const { return m_convertFinished; } @@ -277,14 +289,14 @@ void ConvertFileToScoreScenario::confirmConvert(const io::paths_t& paths, Conver selectFilesToConvert(paths, type) .onResolve(this, [this](const ConvertSelection& selection) { - startConvert(selection.input, selection.convertedFileName); + startConvert(selection.input, selection.convertedScoreName); }); }); } -Ret ConvertFileToScoreScenario::startConvert(const ConvertInput& input, const muse::String& convertedFileName) +Ret ConvertFileToScoreScenario::startConvert(const ConvertInput& input, const muse::String& convertedScoreName) { - Ret ret = service()->startConvert(input, convertedFileName); + Ret ret = service()->startConvert(input, convertedScoreName); if (!ret) { showUnknownError(); return ret; @@ -439,22 +451,23 @@ void ConvertFileToScoreScenario::showFileProcessingDialog() }); } -void ConvertFileToScoreScenario::showScoreReadyNotification(const io::path_t& path) +void ConvertFileToScoreScenario::showScoreReadyNotification(const WatchedScore& watched) { constexpr int openScoreBtn = int(toast::ToastActionCode::Custom) + 1; + const int scoreId = watched.scoreId ? *watched.scoreId : 0; - QString scoreName = QFileInfo(path.toQString()).completeBaseName(); std::string msg = muse::qtrc("project/convert", "‘%1’ has finished processing and is ready to open.") - .arg(scoreName).toStdString(); + .arg(watched.name.toQString()).toStdString(); toastService()->show(muse::trc("project/convert", "Your score is ready!"), msg, muse::ui::IconCode::Code::TICK_FILLED, true, { { muse::trc("global", "Dismiss"), toast::ToastActionCode::Dismiss }, { muse::trc("project/convert", "Open score"), openScoreBtn, /*accent*/ true }, - }).onResolve(this, [this, path, openScoreBtn](const toast::ToastResult& result) { + }).onResolve(this, [this, scoreId, openScoreBtn](const toast::ToastResult& result) { if (result.isCode(openScoreBtn)) { - dispatcher()->dispatch("file-open", actions::ActionData::make_arg1(path.toQUrl())); + const QUrl url(QString("musescore://open-score/%1").arg(scoreId)); + dispatcher()->dispatch("file-open", actions::ActionData::make_arg1(url)); } }); } @@ -481,7 +494,14 @@ void ConvertFileToScoreScenario::showConvertFailedNotification(const Ret& ret) }); } -void ConvertFileToScoreScenario::askReviewRating(ConvertType type, int queueId) +void ConvertFileToScoreScenario::showPollingFailureNotification() +{ + toastService()->showWarning( + muse::trc("project/convert", "We’re having trouble connecting to the internet."), + muse::trc("project/convert", "We’ll keep trying intermittently.")); +} + +void ConvertFileToScoreScenario::askReviewRating(int scoreId) { static constexpr int goodBtn = int(toast::ToastActionCode::Custom) + 1; static constexpr int badBtn = int(toast::ToastActionCode::Custom) + 2; @@ -495,8 +515,8 @@ void ConvertFileToScoreScenario::askReviewRating(ConvertType type, int queueId) { muse::trc("project/convert", "Good"), goodBtn, /*accent*/ true, muse::ui::IconCode::Code::LIKE }, //: Button to rate the quality of a converted score as bad { muse::trc("project/convert", "Bad"), badBtn, /*accent*/ false, muse::ui::IconCode::Code::DISLIKE }, - }).onResolve(this, [this, queueId, type](const toast::ToastResult& result) { + }).onResolve(this, [this, scoreId](const toast::ToastResult& result) { ReviewRating rating = result.isCode(goodBtn) ? ReviewRating::Good : ReviewRating::Bad; - service()->submitReview(type, queueId, rating); + service()->submitReview(scoreId, rating); }); } diff --git a/src/project/internal/convertfiletoscorescenario.h b/src/project/internal/convertfiletoscorescenario.h index 56f5de1b71689..58a6e03c93791 100644 --- a/src/project/internal/convertfiletoscorescenario.h +++ b/src/project/internal/convertfiletoscorescenario.h @@ -66,7 +66,7 @@ class ConvertFileToScoreScenario : public QObject, public IConvertFileToScoreSce muse::Ret validateLink(const QUrl& link) override; void convertFiles(const muse::io::paths_t& paths = {}) override; - muse::async::Channel convertFinished() const override; + muse::async::Channel convertFinished() const override; private: muse::async::Promise checkConvertIsAllowed(); @@ -76,7 +76,7 @@ class ConvertFileToScoreScenario : public QObject, public IConvertFileToScoreSce void confirmConvert(const muse::io::paths_t& paths, ConvertType type); - muse::Ret startConvert(const ConvertInput& input, const muse::String& convertedFileName); + muse::Ret startConvert(const ConvertInput& input, const muse::String& convertedScoreName); void showValidationError(const muse::Ret& ret); @@ -92,13 +92,16 @@ class ConvertFileToScoreScenario : public QObject, public IConvertFileToScoreSce void showTooManyImagesError(int maxImages); void showFileProcessingDialog(); - void showScoreReadyNotification(const muse::io::path_t& path); + void showScoreReadyNotification(const WatchedScore& watched); void showConvertFailedNotification(const muse::Ret& ret); + void showPollingFailureNotification(); - void askReviewRating(ConvertType type, int queueId); + void askReviewRating(int scoreId); void checkPendingReview(); - muse::async::Channel m_convertFinished; - std::map > m_pendingReviews; + muse::async::Channel m_convertFinished; + std::map m_pendingReviews; + + bool m_retryToastShown = false; }; } diff --git a/src/project/internal/convertfiletoscoreservice.cpp b/src/project/internal/convertfiletoscoreservice.cpp index f5f4ed3fcd8de..739a578e2df6c 100644 --- a/src/project/internal/convertfiletoscoreservice.cpp +++ b/src/project/internal/convertfiletoscoreservice.cpp @@ -22,9 +22,11 @@ #include "convertfiletoscoreservice.h" #include +#include +#include #include +#include -#include #include #include "project/types/filecategory.h" @@ -32,6 +34,7 @@ #include "network/networkerrors.h" #include "cloud/clouderrors.h" +#include "multiwindows/resourcelockguard.h" #include "global/serialization/json.h" #include "global/types/bytearray.h" @@ -106,18 +109,16 @@ static std::string errorCodeToString(ConvertErrorCode code) case ConvertErrorCode::TooComplex: return "The file is too large or there is a problem with access to this file"; case ConvertErrorCode::DontRecognizeNotes: return "Invalid file, could not recognize notes"; case ConvertErrorCode::GeneralFailure: return "Something went wrong"; + case ConvertErrorCode::BadParams: return "Invalid conversion parameters"; } return std::string(); } -static std::string convertLogId(ConvertType type, int itemId) -{ - return std::to_string(itemId) + " (type: " + convertTypeToString(type) + ")"; -} +static const std::string WATCHED_CONVERTS_RESOURCE_NAME("WATCHED_CONVERTS"); -static std::string convertLogId(const muse::String& convertedFileName, ConvertType type, int itemId) +static std::string convertIdAndType(ConvertType type, int itemId) { - return "\"" + convertedFileName.toStdString() + "\" (conversion " + convertLogId(type, itemId) + ")"; + return "id: " + std::to_string(itemId) + ", type: " + convertTypeToString(type); } void ConvertFileToScoreService::init() @@ -149,24 +150,36 @@ void ConvertFileToScoreService::init() m_config = config.val; } }); + + multiwindowsProvider()->resourceChanged().onReceive(this, [this](const std::string& resourceName) { + if (resourceName == WATCHED_CONVERTS_RESOURCE_NAME && !m_isSaving) { + loadWatchedScores(); + + if (!m_watchedScores.empty() && !m_timer.isActive()) { + m_timer.start(); + } + + m_watchedScoresChanged.notify(); + } + }); } void ConvertFileToScoreService::resumeConvert() { - loadWatchedItems(); + loadWatchedScores(); - if (m_watchedItems.empty()) { + if (m_watchedScores.empty()) { return; } - LOGI() << "Resuming " << m_watchedItems.size() << " pending conversion(s)"; + LOGI() << "Resuming watching " << m_watchedScores.size() << " pending conversion(s)"; m_timer.start(); - m_fileNamesBeingConvertedChanged.notify(); + m_watchedScoresChanged.notify(); - for (const WatchedItem& item : m_watchedItems) { - if (item.convertStatus == ConvertStatus::AwaitingReview && !item.downloadedScorePath.empty()) { - m_reviewRequested.send(item.type, item.id, item.downloadedScorePath); + for (const WatchedScore& watched : m_watchedScores) { + if (watched.conversion.status == ConvertStatus::AwaitingReview && watched.scoreId) { + m_reviewRequested.send(*watched.scoreId); } } @@ -276,71 +289,70 @@ Ret ConvertFileToScoreService::validateLink(const QUrl& link) const return make_ret(Err::ConvertUnsupportedLink); } -Ret ConvertFileToScoreService::startConvert(const ConvertInput& input, const muse::String& convertedFileName) +Ret ConvertFileToScoreService::startConvert(const ConvertInput& input, const muse::String& convertedScoreName) { - IF_ASSERT_FAILED(!convertPathsOf(input).empty() || !convertLinkOf(input).isEmpty()) { + const io::paths_t paths = convertPathsOf(input); + const QUrl link = convertLinkOf(input); + + IF_ASSERT_FAILED(!paths.empty() || link.isValid()) { return make_ret(Err::ConvertValidationFailed); } - IF_ASSERT_FAILED(io::isAllowedFileName(io::path_t(convertedFileName))) { + IF_ASSERT_FAILED(io::isAllowedFileName(io::path_t(convertedScoreName))) { return make_ret(Err::ConvertValidationFailed); } + ConvertFileDataList files; + files.reserve(paths.size()); + for (const io::path_t& path : paths) { + RetVal fileData = fileSystem()->readFile(path); + if (!fileData.ret) { + return fileData.ret; + } + + files.push_back(ConvertFileData { std::move(fileData.val), io::filename(path) }); + } + const ConvertType type = convertTypeOf(input); - ProgressPtr progress = museScoreComService()->convert()->upload(input); + auto data = std::make_shared(ConvertUploadData { type, std::move(files), link, + convertedScoreName.toQString() }); + ProgressPtr progress = museScoreComService()->convert()->upload(data); - progress->progressChanged().onReceive(this, [convertedFileName](int64_t current, int64_t total, const std::string&) { - LOGI() << "Uploading for convert \"" << convertedFileName << "\": " << current << "/" << total; + progress->progressChanged().onReceive(this, [convertedScoreName](int64_t current, int64_t total, const std::string&) { + LOGI() << "Uploading for convert \"" << convertedScoreName << "\": " << current << "/" << total; }); - progress->finished().onReceive(this, [this, type, convertedFileName](const ProgressResult& res) { + progress->finished().onReceive(this, [this, type, convertedScoreName](const ProgressResult& res) { if (!res.ret) { - LOGE() << "Could not upload files for \"" << convertedFileName << "\" (type: " + LOGE() << "Could not upload files for \"" << convertedScoreName << "\" (type: " << convertTypeToString(type) << "): " << res.ret.toString(); Ret ret = res.ret; - ret.setData(CONVERT_FAILED_FILE_NAME_KEY, convertedFileName); + ret.setData(CONVERT_FAILED_FILE_NAME_KEY, convertedScoreName); finishConvert(ret); return; } const int itemId = res.val.toMap()["id"].toInt(); - watch(type, itemId, convertedFileName); + watch(type, itemId, convertedScoreName); }); return make_ok(); } -async::Channel ConvertFileToScoreService::convertFinished() const +async::Channel ConvertFileToScoreService::convertFinished() const { return m_convertFinished; } -bool ConvertFileToScoreService::isPending(const WatchedItem& item) +ValNt ConvertFileToScoreService::watchedScores() const { - //! NOTE: an AwaitingReview item that's already downloaded is just waiting on the user - //! to submit a review - not "being converted" anymore - return item.convertStatus != ConvertStatus::AwaitingReview || item.downloadedScorePath.empty(); -} - -muse::StringList ConvertFileToScoreService::fileNamesBeingConverted() const -{ - muse::StringList result; - result.reserve(m_watchedItems.size()); - - for (const WatchedItem& item : m_watchedItems) { - if (isPending(item)) { - result.push_back(item.convertedFileName); - } - } + ValNt result; + result.val = m_watchedScores; + result.notification = m_watchedScoresChanged; return result; } -async::Notification ConvertFileToScoreService::fileNamesBeingConvertedChanged() const -{ - return m_fileNamesBeingConvertedChanged; -} - async::Channel ConvertFileToScoreService::pollingFailed() const { return m_pollingFailed; @@ -353,155 +365,88 @@ void ConvertFileToScoreService::retryPolling() poll(); } -async::Channel ConvertFileToScoreService::reviewRequested() const +async::Channel ConvertFileToScoreService::reviewRequested() const { return m_reviewRequested; } -void ConvertFileToScoreService::submitReview(ConvertType type, int itemId, ReviewRating rating, const QString& comment) +void ConvertFileToScoreService::submitReview(int scoreId, ReviewRating rating, const QString& comment) { IF_ASSERT_FAILED(rating == ReviewRating::Bad || comment.isEmpty()) { return; } - museScoreComService()->convert()->submitReview(type, itemId, rating, comment) - .onResolve(this, [type, itemId](const RetVal& submitRes) { - if (!submitRes.ret) { - LOGE() << "Could not submit the review for conversion " << convertLogId(type, itemId) << ": " << submitRes.ret.toString(); - } - }); -} + const WatchedScore* watched = findWatchedScoreByScoreId(scoreId); + IF_ASSERT_FAILED(watched) { + return; + } -void ConvertFileToScoreService::submitReviewComment(ConvertType type, int itemId, const QString& comment) -{ - museScoreComService()->convert()->submitReviewComment(type, itemId, comment) - .onResolve(this, [type, itemId](const RetVal& submitRes) { + const ConvertType type = watched->conversion.type; + const int convertId = watched->conversion.id; + + museScoreComService()->convert()->submitReview(type, convertId, rating, comment) + .onResolve(this, [type, convertId](const RetVal& submitRes) { if (!submitRes.ret) { - LOGE() << "Could not submit the comment for conversion " << convertLogId(type, itemId) << ": " << submitRes.ret.toString(); + LOGE() << "Could not submit the review for conversion (" << convertIdAndType(type, + convertId) << "): " << submitRes.ret.toString(); } }); } -void ConvertFileToScoreService::watch(ConvertType type, int itemId, const muse::String& convertedFileName) +void ConvertFileToScoreService::submitReviewComment(int scoreId, const QString& comment) { - LOGI() << "Watching conversion " << convertLogId(convertedFileName, type, itemId); - - m_watchedItems.push_back(WatchedItem { itemId, type, convertedFileName }); - saveWatchedItems(); - m_fileNamesBeingConvertedChanged.notify(); - - if (!m_timer.isActive()) { - m_timer.start(); - } - - poll(); -} - -void ConvertFileToScoreService::poll() -{ - if (m_watchedItems.empty()) { - m_timer.stop(); - return; - } - - if (m_pollInProgress) { + const WatchedScore* watched = findWatchedScoreByScoreId(scoreId); + IF_ASSERT_FAILED(watched) { return; } - m_pollInProgress = true; - - museScoreComService()->convert()->fetchQueue().onResolve(this, [this](const RetVal& result) { - m_pollInProgress = false; + const ConvertType type = watched->conversion.type; + const int convertId = watched->conversion.id; - if (!result.ret) { - handlePollFailure(result.ret); - return; + museScoreComService()->convert()->submitReviewComment(type, convertId, comment) + .onResolve(this, [type, convertId](const RetVal& submitRes) { + if (!submitRes.ret) { + LOGE() << "Could not submit the comment for conversion (" << convertIdAndType(type, + convertId) << "): " << submitRes.ret.toString(); } - - resetPollState(); - updateWatchedItems(result.val); }); } -void ConvertFileToScoreService::resetPollState() +void ConvertFileToScoreService::deleteConversion(ConvertType type, int convertId) { - m_pollFailureCount = 0; - m_pollIntervalMs = MIN_RETRY_INTERVAL_MS; - m_timer.setInterval(MIN_RETRY_INTERVAL_MS); -} - -void ConvertFileToScoreService::handlePollFailure(const Ret& ret) -{ - if (isRetryableError(ret) && ++m_pollFailureCount < MAX_POLL_RETRY_ATTEMPTS) { - //! NOTE: the first retry is likely just a stale pooled connection the server closed - //! (e.g. HTTP/2 GOAWAY) - don't back off yet, retry at the normal interval - if (m_pollFailureCount > 1) { - m_pollIntervalMs = std::min(m_pollIntervalMs * 2, MAX_RETRY_INTERVAL_MS); - m_timer.setInterval(m_pollIntervalMs); + museScoreComService()->convert()->deleteConversion(type, convertId) + .onResolve(this, [this, type, convertId](const Ret& ret) { + if (!ret) { + LOGE() << "Could not delete conversion (" << convertIdAndType(type, convertId) << "): " << ret.toString(); + return; } - const secs_t intervalSecs(m_pollIntervalMs / 1000.0); - LOGW() << "Could not check the conversion status, retrying in " << intervalSecs.raw() - << "s (attempt " << m_pollFailureCount << "/" << MAX_POLL_RETRY_ATTEMPTS - << "): " << ret.toString(); - m_pollingFailed.send(PollingFailure { ret, m_pollFailureCount, MAX_POLL_RETRY_ATTEMPTS, intervalSecs, false }); - return; - } - - giveUpPolling(ret); -} - -void ConvertFileToScoreService::giveUpPolling(const Ret& ret) -{ - LOGE() << "Could not check the conversion status, stopping polling for now, " - << m_watchedItems.size() << " pending conversion(s) remain watched: " << ret.toString(); - - const int count = m_pollFailureCount; - - m_timer.stop(); - resetPollState(); - - m_pollingFailed.send(PollingFailure { ret, count, MAX_POLL_RETRY_ATTEMPTS, secs_t(0), true }); -} -void ConvertFileToScoreService::updateWatchedItems(const ConvertQueueList& queue) -{ - const std::vector previousWatchedItems = m_watchedItems; - - for (auto it = m_watchedItems.begin(); it != m_watchedItems.end();) { - WatchedItem& item = *it; - - auto found = std::find_if(queue.begin(), queue.end(), [&item](const ConvertQueueItem& queueItem) { - return queueItem.id == item.id && queueItem.type == item.type; + const auto it = std::find_if(m_watchedScores.begin(), m_watchedScores.end(), [type, convertId](const WatchedScore& watched) { + return watched.conversion.type == type && watched.conversion.id == convertId; }); - //! NOTE: normally a Done item still reports its status while queued, but it may - //! be dropped from the queue automatically at some point afterwards - treat that as Done too - const ConvertStatus status = found != queue.end() ? found->status : ConvertStatus::Done; - const ConvertErrorCode errorCode = found != queue.end() ? found->errorCode : ConvertErrorCode::Unknown; - - handleItem(item, status, errorCode); - - if (status == ConvertStatus::Failed - || (status == ConvertStatus::Done && !item.downloadedScorePath.empty())) { - it = m_watchedItems.erase(it); - } else { - ++it; + if (it == m_watchedScores.end()) { + return; } - } - if (m_watchedItems != previousWatchedItems) { - saveWatchedItems(); - m_fileNamesBeingConvertedChanged.notify(); - } + m_watchedScores.erase(it); + saveWatchedScores(); + m_watchedScoresChanged.notify(); + }); } -void ConvertFileToScoreService::loadWatchedItems() +void ConvertFileToScoreService::loadWatchedScores() { TRACEFUNC; - m_watchedItems.clear(); + m_watchedScores.clear(); + + RetVal data; + { + muse::mi::ReadResourceLockGuard resource_guard(multiwindowsProvider(), WATCHED_CONVERTS_RESOURCE_NAME); + data = fileSystem()->readFile(configuration()->watchedConvertsJsonPath()); + } - RetVal data = fileSystem()->readFile(configuration()->pendingConvertsJsonPath()); if (!data.ret || data.val.empty()) { if (!data.ret && data.ret.code() != static_cast(io::Err::FSNotExist)) { LOGE() << "Could not read the pending conversions file: " << data.ret; @@ -519,303 +464,290 @@ void ConvertFileToScoreService::loadWatchedItems() } const JsonArray array = json.rootArray(); - m_watchedItems.reserve(array.size()); + m_watchedScores.reserve(array.size()); for (size_t i = 0; i < array.size(); ++i) { const JsonObject obj = array.at(i).toObject(); - const int itemId = obj.value("id").toInt(); - const ConvertType type = static_cast(obj.value("type").toInt()); - const muse::String convertedFileName = muse::String::fromStdString(obj.value("convertedFileName").toStdString()); - const ConvertStatus convertStatus = static_cast(obj.value("convertStatus").toInt()); - const io::path_t downloadedScorePath = obj.value("downloadedScorePath").toStdString(); + const int typeInt = obj.value("type").toInt(); + if (typeInt < 0 || typeInt > static_cast(ConvertType::Last)) { + LOGW() << "Skipping conversion with unknown type: " << typeInt; + continue; + } - m_watchedItems.push_back(WatchedItem { itemId, type, convertedFileName, convertStatus, false, downloadedScorePath }); + WatchedScore& watched = m_watchedScores.emplace_back(); + watched.conversion.id = obj.value("id").toInt(); + watched.conversion.type = static_cast(typeInt); + watched.conversion.status = static_cast(obj.value("status").toInt()); + watched.scoreId = obj.contains("scoreId") ? std::optional(obj.value("scoreId").toInt()) : std::nullopt; + watched.startedLocally = obj.value("startedLocally").toBool(); + watched.name = muse::String::fromStdString(obj.value("convertedScoreName").toStdString()); } } -void ConvertFileToScoreService::saveWatchedItems() +void ConvertFileToScoreService::saveWatchedScores() { TRACEFUNC; JsonArray array; - for (const WatchedItem& item : m_watchedItems) { + for (const WatchedScore& watched : m_watchedScores) { JsonObject obj; - obj["id"] = item.id; - obj["type"] = static_cast(item.type); - obj["convertStatus"] = static_cast(item.convertStatus); + obj["id"] = watched.conversion.id; + obj["type"] = static_cast(watched.conversion.type); + obj["status"] = static_cast(watched.conversion.status); + obj["startedLocally"] = watched.startedLocally; - if (!item.convertedFileName.isEmpty()) { - obj["convertedFileName"] = item.convertedFileName.toStdString(); + if (!watched.name.isEmpty()) { + obj["convertedScoreName"] = watched.name.toStdString(); } - if (!item.downloadedScorePath.empty()) { - obj["downloadedScorePath"] = item.downloadedScorePath.toStdString(); + if (watched.scoreId) { + obj["scoreId"] = *watched.scoreId; } array << obj; } JsonDocument json(array); - Ret ret = fileSystem()->writeFile(configuration()->pendingConvertsJsonPath(), json.toJson()); - if (!ret) { - LOGE() << "Could not save the pending conversions list: " << ret.toString(); + + m_isSaving = true; + { + muse::mi::WriteResourceLockGuard resource_guard(multiwindowsProvider(), WATCHED_CONVERTS_RESOURCE_NAME); + Ret ret = fileSystem()->writeFile(configuration()->watchedConvertsJsonPath(), json.toJson()); + if (!ret) { + LOGE() << "Could not save the pending conversions list: " << ret.toString(); + } } + m_isSaving = false; } -std::vector::iterator ConvertFileToScoreService::findWatchedItem(ConvertType type, int itemId) +void ConvertFileToScoreService::watch(ConvertType type, int itemId, const muse::String& convertedScoreName) { - return std::find_if(m_watchedItems.begin(), m_watchedItems.end(), [type, itemId](const WatchedItem& item) { - return item.type == type && item.id == itemId; + LOGI() << "Start watching conversion of \"" << convertedScoreName << "\" (" << convertIdAndType(type, itemId) << ")"; + + const auto it = std::find_if(m_watchedScores.begin(), m_watchedScores.end(), [type, itemId](const WatchedScore& watched) { + return watched.conversion.type == type && watched.conversion.id == itemId; }); -} + WatchedScore& watched = it != m_watchedScores.end() ? *it : m_watchedScores.emplace_back(); + watched.conversion.id = itemId; + watched.conversion.type = type; + watched.conversion.status = ConvertStatus::Processing; + watched.startedLocally = true; + watched.name = convertedScoreName; -void ConvertFileToScoreService::eraseWatchedItem(ConvertType type, int itemId) -{ - auto it = findWatchedItem(type, itemId); - if (it != m_watchedItems.end()) { - m_watchedItems.erase(it); + saveWatchedScores(); + m_watchedScoresChanged.notify(); + + if (!m_timer.isActive()) { + m_timer.start(); } + + poll(); } -void ConvertFileToScoreService::handleItem(WatchedItem& item, ConvertStatus status, ConvertErrorCode errorCode) +void ConvertFileToScoreService::poll() { - const bool statusChanged = item.convertStatus != status; - if (statusChanged) { - LOGI() << "Conversion status changed: " << convertLogId(item.convertedFileName, item.type, item.id) - << " -> " << convertStatusToString(status); + if (m_watchedScores.empty()) { + m_timer.stop(); + return; } - item.convertStatus = status; - switch (status) { - case ConvertStatus::Processing: - case ConvertStatus::Unknown: - break; - case ConvertStatus::AwaitingReview: - //! NOTE: the MSCZ is already available at this point; the review rating doesn't gate the download - downloadIfNotAlready(item); - break; - case ConvertStatus::Done: - downloadIfNotAlready(item); - break; - case ConvertStatus::Failed: { - if (!statusChanged) { + if (m_pollInProgress) { + return; + } + + m_pollInProgress = true; + + museScoreComService()->convert()->fetchQueue().onResolve(this, [this](const RetVal& result) { + m_pollInProgress = false; + + if (!result.ret) { + handlePollFailure(result.ret); return; } - Ret ret = make_ret(Err::ConvertProcessingFailed); - ret.setText("Conversion failed for \"" + item.convertedFileName.toStdString() + "\": " + errorCodeToString(errorCode)); - ret.setData(CONVERT_FAILED_FILE_NAME_KEY, item.convertedFileName); - - LOGE() << ret.toString(); + resetPollState(); + updateWatchedScores(result.val); + }); +} - finishConvert(ret); - break; - } - } +void ConvertFileToScoreService::resetPollState() +{ + m_pollFailureCount = 0; + m_pollIntervalMs = MIN_RETRY_INTERVAL_MS; + m_timer.setInterval(MIN_RETRY_INTERVAL_MS); } -void ConvertFileToScoreService::downloadIfNotAlready(WatchedItem& item) +void ConvertFileToScoreService::handlePollFailure(const Ret& ret) { - if (item.isDownloading || !item.downloadedScorePath.empty()) { + if (isRetryableError(ret) && ++m_pollFailureCount < MAX_POLL_RETRY_ATTEMPTS) { + //! NOTE: the first retry is likely just a stale pooled connection the server closed + //! (e.g. HTTP/2 GOAWAY) - don't back off yet, retry at the normal interval + if (m_pollFailureCount > 1) { + m_pollIntervalMs = std::min(m_pollIntervalMs * 2, MAX_RETRY_INTERVAL_MS); + m_timer.setInterval(m_pollIntervalMs); + } + const secs_t intervalSecs(m_pollIntervalMs / 1000.0); + LOGW() << "Could not check the conversion status, retrying in " << intervalSecs.raw() + << "s (attempt " << m_pollFailureCount << "/" << MAX_POLL_RETRY_ATTEMPTS + << "): " << ret.toString(); + m_pollingFailed.send(PollingFailure { ret, m_pollFailureCount, MAX_POLL_RETRY_ATTEMPTS, intervalSecs, false }); return; } - item.isDownloading = true; - fetchScoreUrlAndDownload(item.type, item.id, item.convertedFileName); + giveUpPolling(ret); } -void ConvertFileToScoreService::fetchScoreUrlAndDownload(ConvertType type, int itemId, const muse::String& convertedFileName) +void ConvertFileToScoreService::giveUpPolling(const Ret& ret) { - museScoreComService()->convert()->fetchMsczUrl(type, itemId) - .onResolve(this, [this, type, itemId, convertedFileName](const RetVal& urlInfo) { - if (!urlInfo.ret) { - if (isRetryableError(urlInfo.ret)) { - LOGW() << "Could not fetch the converted score " << convertLogId(convertedFileName, type, itemId) - << ", will retry on next poll: " << urlInfo.ret.toString(); - clearDownloading(type, itemId); - return; - } + LOGE() << "Could not check the conversion status, stopping polling for now, " + << m_watchedScores.size() << " pending conversion(s) remain watched: " << ret.toString(); - Ret ret = urlInfo.ret; - ret.setText("Could not fetch the converted score: " + ret.text()); - failConvert(ret, type, itemId, convertedFileName); - return; - } + const int count = m_pollFailureCount; - if (urlInfo.val.expiresInSeconds <= 0) { - Ret ret = make_ret(Err::DownloadLinkExpired, std::string("The download link has already expired")); - ret.setData(CONVERT_FAILED_FILE_NAME_KEY, convertedFileName); - LOGW() << "Could not download the converted score " << convertLogId(convertedFileName, type, itemId) - << ": " << ret.toString(); - eraseWatchedItem(type, itemId); - saveWatchedItems(); - m_fileNamesBeingConvertedChanged.notify(); - finishConvert(ret); - return; - } + m_timer.stop(); + resetPollState(); - downloadScoreAndFinish(type, itemId, convertedFileName, urlInfo.val); - }); + m_pollingFailed.send(PollingFailure { ret, count, MAX_POLL_RETRY_ATTEMPTS, secs_t(0), true }); } -void ConvertFileToScoreService::downloadScoreAndFinish(ConvertType type, int itemId, const muse::String& convertedFileName, - const SignedMsczUrl& urlInfo) +void ConvertFileToScoreService::updateWatchedScores(const ConvertQueueList& queue) { - auto scoreData = std::make_shared(); - ProgressPtr progress = museScoreComService()->convert()->downloadConvertedScore(urlInfo, scoreData); + TRACEFUNC; - progress->finished().onReceive(this, [this, type, itemId, convertedFileName, scoreData](const ProgressResult& res) { - if (!res.ret) { - if (isRetryableError(res.ret)) { - LOGW() << "Could not download the converted score " << convertLogId(convertedFileName, type, itemId) - << ", will retry on next poll: " << res.ret.toString(); - clearDownloading(type, itemId); - return; - } + constexpr size_t CONVERT_TYPE_COUNT = static_cast(ConvertType::Last) + 1; + std::array, CONVERT_TYPE_COUNT> oldByTypeAndId; + for (size_t i = 0; i < m_watchedScores.size(); ++i) { + const WatchedScore& watched = m_watchedScores[i]; + oldByTypeAndId[static_cast(watched.conversion.type)][watched.conversion.id] = i; + } - Ret ret = res.ret; - ret.setText("Could not download the converted score: " + ret.text()); - failConvert(ret, type, itemId, convertedFileName); - return; - } + std::vector seen(m_watchedScores.size(), false); - if (findWatchedItem(type, itemId) == m_watchedItems.end()) { - //! NOTE: the item was already removed (e.g. reported as Failed) while this download - //! was in progress - discard the result rather than reporting a contradictory outcome - LOGW() << "Conversion " << convertLogId(convertedFileName, type, itemId) - << " was already removed while its download was in progress, discarding the result"; - return; - } + //! NOTE: the queue is the source of truth - rebuild m_watchedScores from it every time, + //! since it may also contain conversions started outside MuseScore + std::vector newWatchedScores; + newWatchedScores.reserve(queue.size()); - writeConvertedScore(convertedFileName, scoreData, [this, type, itemId, convertedFileName](const RetVal& writeResult) { - onWriteFinished(type, itemId, convertedFileName, writeResult); - }); - }); -} + for (const ConvertQueueItem& queueItem : queue) { + const std::unordered_map& oldIndexById = oldByTypeAndId[static_cast(queueItem.type)]; + const auto it = oldIndexById.find(queueItem.id); -void ConvertFileToScoreService::writeConvertedScore(const muse::String& convertedFileName, const std::shared_ptr& scoreData, - std::function&)> onFinished) -{ - const io::path_t dir = configuration()->convertedScoresPath(); + if (it != oldIndexById.end()) { + //! NOTE: already watched - update it (name, status, scoreId) + seen[it->second] = true; + WatchedScore watched = m_watchedScores.at(it->second); + if (!queueItem.filename.isEmpty()) { + watched.name = queueItem.filename; + } + watched.scoreId = queueItem.scoreId; + updateStatus(watched, queueItem.status, queueItem.errorCode); - makePathWithRetry(dir, 0, [this, dir, convertedFileName, scoreData, onFinished](const Ret& ret) { - if (!ret) { - onFinished(RetVal::make_ret(ret)); - return; + if (watched.conversion.status != ConvertStatus::Done) { + newWatchedScores.push_back(watched); + } + continue; } - const io::path_t baseName = io::escapeFileName(io::path_t(convertedFileName)); - const std::string addition = configuration()->uniqueFileNameAddition(baseName, dir, "mscz"); - const io::path_t path = dir.appendingComponent(baseName + addition).appendingSuffix("mscz"); + if (queueItem.status != ConvertStatus::Processing && queueItem.status != ConvertStatus::AwaitingReview) { + //! NOTE: not previously watched, and already terminal - nothing to watch for anymore + continue; + } - writeFileWithRetry(path, scoreData, 0, [onFinished, path](const Ret& ret) { - onFinished(ret ? RetVal::make_ok(path) : RetVal::make_ret(ret)); - }); - }); -} + LOGI() << "Found new external conversion (" << convertIdAndType(queueItem.type, queueItem.id) << ")"; -void ConvertFileToScoreService::onWriteFinished(ConvertType type, int itemId, const muse::String& convertedFileName, - const RetVal& writeResult) -{ - if (!writeResult.ret) { - failConvert(writeResult.ret, type, itemId, convertedFileName); - return; - } + WatchedScore watched; + watched.conversion.id = queueItem.id; + watched.conversion.type = queueItem.type; + watched.name = queueItem.filename; + watched.scoreId = queueItem.scoreId; - //! NOTE: re-lookup rather than reusing an iterator from before the (possibly retried) write - - //! the vector may have changed while the write was in progress - auto watched = findWatchedItem(type, itemId); - if (watched == m_watchedItems.end()) { - LOGW() << "Conversion " << convertLogId(convertedFileName, type, itemId) - << " was already removed while its write was in progress, discarding the result"; - return; + updateStatus(watched, queueItem.status, queueItem.errorCode); + + if (watched.conversion.status != ConvertStatus::Done) { + newWatchedScores.push_back(watched); + } } - watched->isDownloading = false; - watched->downloadedScorePath = writeResult.val; - const bool requestReview = watched->convertStatus == ConvertStatus::AwaitingReview; + for (size_t i = 0; i < m_watchedScores.size(); ++i) { + if (seen.at(i)) { + continue; + } - //! NOTE: keep watching an AwaitingReview item even after it's downloaded, so the - //! review can be re-requested on resume if the app closes before it's submitted - if (watched->convertStatus == ConvertStatus::Done) { - m_watchedItems.erase(watched); - } + WatchedScore& dropped = m_watchedScores.at(i); - saveWatchedItems(); + if (dropped.scoreId) { + LOGI() << "Conversion of \"" << dropped.name << "\" (" << convertIdAndType(dropped.conversion.type, dropped.conversion.id) + << ") was dropped from the queue, recovering as Done with scoreId " << *dropped.scoreId; - m_fileNamesBeingConvertedChanged.notify(); - finishConvert(make_ok(), writeResult.val); + //! NOTE: always terminal, so never added back to newWatchedScores + updateStatus(dropped, ConvertStatus::Done, ConvertErrorCode::Unknown); + } else { + LOGW() << "Conversion of \"" << dropped.name << "\" (" << convertIdAndType(dropped.conversion.type, dropped.conversion.id) + << ") was dropped from the queue without ever reporting a scoreId"; + } + } - if (requestReview) { - m_reviewRequested.send(type, itemId, writeResult.val); + if (m_watchedScores != newWatchedScores) { + m_watchedScores = std::move(newWatchedScores); + saveWatchedScores(); + m_watchedScoresChanged.notify(); } } -void ConvertFileToScoreService::makePathWithRetry(const io::path_t& dir, int attempt, std::function onFinished) +void ConvertFileToScoreService::updateStatus(WatchedScore& watched, ConvertStatus newStatus, ConvertErrorCode errorCode) { - Ret ret = fileSystem()->makePath(dir); - if (ret) { - onFinished(ret); - return; - } - - if (attempt + 1 == MAX_FS_RETRY_ATTEMPTS) { - LOGE() << "Could not create the directory for converted scores \"" << dir << "\", giving up: " << ret.toString(); - onFinished(ret); + const ConvertStatus previousStatus = watched.conversion.status; + if (previousStatus == newStatus) { return; } - LOGW() << "Could not create the directory for converted scores \"" << dir - << "\", retrying (attempt " << (attempt + 1) << "/" << MAX_FS_RETRY_ATTEMPTS << "): " << ret.toString(); + LOGI() << "Conversion of \"" << watched.name << "\" (" << convertIdAndType(watched.conversion.type, watched.conversion.id) << ")" + << " status changed: " << convertStatusToString(previousStatus) << " -> " << convertStatusToString(newStatus); - QTimer::singleShot(FS_RETRY_INTERVAL_MS, this, [this, dir, attempt, onFinished]() { - makePathWithRetry(dir, attempt + 1, onFinished); - }); -} + const bool wasDone = previousStatus == ConvertStatus::Done + || previousStatus == ConvertStatus::AwaitingReview; -void ConvertFileToScoreService::writeFileWithRetry(const io::path_t& path, const std::shared_ptr& scoreData, int attempt, - std::function onFinished) -{ - //! NOTE: a no-copy view - scoreData is kept alive via capture for as long as retries are needed - const ByteArray byteArray = ByteArray::fromQByteArrayNoCopy(scoreData->data()); + watched.conversion.status = newStatus; - Ret ret = fileSystem()->writeFile(path, byteArray); - if (ret) { - onFinished(ret); - return; - } - - if (attempt + 1 == MAX_FS_RETRY_ATTEMPTS) { - LOGE() << "Could not save the converted score \"" << path << "\", giving up: " << ret.toString(); - onFinished(ret); - return; - } + switch (newStatus) { + case ConvertStatus::Processing: + case ConvertStatus::Unknown: + break; + case ConvertStatus::AwaitingReview: + case ConvertStatus::Done: { + if (!wasDone && watched.scoreId && watched.startedLocally) { + finishConvert(make_ok(), watched); + } - LOGW() << "Could not save the converted score \"" << path - << "\", retrying (attempt " << (attempt + 1) << "/" << MAX_FS_RETRY_ATTEMPTS << "): " << ret.toString(); + if (newStatus == ConvertStatus::AwaitingReview && watched.scoreId) { + m_reviewRequested.send(*watched.scoreId); + } + } break; + case ConvertStatus::Failed: { + Ret ret = make_ret(Err::ConvertProcessingFailed); + ret.setText("Conversion failed for \"" + watched.name.toStdString() + "\": " + errorCodeToString(errorCode)); + ret.setData(CONVERT_FAILED_FILE_NAME_KEY, watched.name); - QTimer::singleShot(FS_RETRY_INTERVAL_MS, this, [this, path, scoreData, attempt, onFinished]() { - writeFileWithRetry(path, scoreData, attempt + 1, onFinished); - }); -} + LOGE() << ret.toString(); -void ConvertFileToScoreService::clearDownloading(ConvertType type, int itemId) -{ - auto it = findWatchedItem(type, itemId); - if (it != m_watchedItems.end()) { - it->isDownloading = false; + if (watched.startedLocally) { + finishConvert(ret); + } + break; + } } } -void ConvertFileToScoreService::finishConvert(const Ret& ret, const io::path_t& path) +void ConvertFileToScoreService::finishConvert(const Ret& ret, const WatchedScore& watched) { - m_convertFinished.send(ret, path); + m_convertFinished.send(ret, watched); } -void ConvertFileToScoreService::failConvert(Ret ret, ConvertType type, int itemId, const muse::String& convertedFileName) +WatchedScore* ConvertFileToScoreService::findWatchedScoreByScoreId(int scoreId) { - LOGE() << ret.toString() << " " << convertLogId(convertedFileName, type, itemId); - ret.setData(CONVERT_FAILED_FILE_NAME_KEY, convertedFileName); - clearDownloading(type, itemId); - finishConvert(ret); + auto it = std::find_if(m_watchedScores.begin(), m_watchedScores.end(), [scoreId](const WatchedScore& watched) { + return watched.scoreId == scoreId; + }); + + return it != m_watchedScores.end() ? &*it : nullptr; } diff --git a/src/project/internal/convertfiletoscoreservice.h b/src/project/internal/convertfiletoscoreservice.h index 8e46dc8c7f198..0da0c41acc63a 100644 --- a/src/project/internal/convertfiletoscoreservice.h +++ b/src/project/internal/convertfiletoscoreservice.h @@ -21,24 +21,22 @@ */ #pragma once -#include -#include +#include #include #include #include +#include "project/iconvertfiletoscoreservice.h" + #include "async/asyncable.h" #include "modularity/ioc.h" -#include "io/ifilesystem.h" #include "cloud/musescorecom/imusescorecomservice.h" - -#include "project/iconvertfiletoscoreservice.h" +#include "io/ifilesystem.h" +#include "multiwindows/imultiwindowsprovider.h" #include "project/iprojectconfiguration.h" -class QBuffer; - namespace mu::project { class ConvertFileToScoreService : public QObject, public IConvertFileToScoreService, public muse::async::Asyncable, public muse::Contextable { @@ -48,6 +46,7 @@ class ConvertFileToScoreService : public QObject, public IConvertFileToScoreServ muse::ContextInject museScoreComService = { this }; muse::GlobalInject fileSystem; muse::GlobalInject configuration; + muse::GlobalInject multiwindowsProvider; explicit ConvertFileToScoreService(const muse::modularity::ContextPtr& iocCtx, QObject* parent = nullptr) : QObject(parent), muse::Contextable(iocCtx) {} @@ -61,87 +60,53 @@ class ConvertFileToScoreService : public QObject, public IConvertFileToScoreServ muse::RetVal validateFiles(const muse::io::paths_t& paths) const override; muse::Ret validateLink(const QUrl& link) const override; - muse::Ret startConvert(const ConvertInput& input, const muse::String& convertedFileName) override; - muse::async::Channel convertFinished() const override; + muse::Ret startConvert(const ConvertInput& input, const muse::String& convertedScoreName) override; + muse::async::Channel convertFinished() const override; - muse::StringList fileNamesBeingConverted() const override; - muse::async::Notification fileNamesBeingConvertedChanged() const override; + muse::ValNt watchedScores() const override; muse::async::Channel pollingFailed() const override; void retryPolling() override; - muse::async::Channel reviewRequested() const override; - void submitReview(ConvertType type, int itemId, ReviewRating rating, const QString& comment = QString()) override; - void submitReviewComment(ConvertType type, int itemId, const QString& comment) override; + muse::async::Channel reviewRequested() const override; + void submitReview(int scoreId, ReviewRating rating, const QString& comment = QString()) override; + void submitReviewComment(int scoreId, const QString& comment) override; + + void deleteConversion(ConvertType type, int convertId) override; private: static constexpr int MIN_RETRY_INTERVAL_MS = 60000; static constexpr int MAX_RETRY_INTERVAL_MS = 10 * 60000; static constexpr int MAX_POLL_RETRY_ATTEMPTS = 5; // gives up after ~15 minutes - static constexpr int MAX_FS_RETRY_ATTEMPTS = 5; - static constexpr int FS_RETRY_INTERVAL_MS = 100; - - struct WatchedItem { - int id = 0; - ConvertType type = ConvertType::Omr; - muse::String convertedFileName; - muse::cloud::ConvertStatus convertStatus = muse::cloud::ConvertStatus::Unknown; - bool isDownloading = false; - muse::io::path_t downloadedScorePath; - - bool operator==(const WatchedItem& other) const - { - return id == other.id - && type == other.type - && convertedFileName == other.convertedFileName - && convertStatus == other.convertStatus - && isDownloading == other.isDownloading - && downloadedScorePath == other.downloadedScorePath; - } - }; - - void watch(ConvertType type, int itemId, const muse::String& convertedFileName); + void loadWatchedScores(); + void saveWatchedScores(); + + void watch(ConvertType type, int itemId, const muse::String& convertedScoreName); void poll(); void resetPollState(); void handlePollFailure(const muse::Ret& ret); void giveUpPolling(const muse::Ret& ret); - void updateWatchedItems(const muse::cloud::ConvertQueueList& queue); - - void loadWatchedItems(); - void saveWatchedItems(); - - std::vector::iterator findWatchedItem(ConvertType type, int itemId); - void eraseWatchedItem(ConvertType type, int itemId); - - void handleItem(WatchedItem& item, muse::cloud::ConvertStatus status, muse::cloud::ConvertErrorCode errorCode); - static bool isPending(const WatchedItem& item); - - void downloadIfNotAlready(WatchedItem& item); - void fetchScoreUrlAndDownload(ConvertType type, int itemId, const muse::String& convertedFileName); - void downloadScoreAndFinish(ConvertType type, int itemId, const muse::String& convertedFileName, - const muse::cloud::SignedMsczUrl& urlInfo); - void writeConvertedScore(const muse::String& convertedFileName, const std::shared_ptr& scoreData, - std::function&)> onFinished); - void onWriteFinished(ConvertType type, int itemId, const muse::String& convertedFileName, - const muse::RetVal& writeResult); - void makePathWithRetry(const muse::io::path_t& dir, int attempt, std::function onFinished); - void writeFileWithRetry(const muse::io::path_t& path, const std::shared_ptr& scoreData, int attempt, - std::function onFinished); - void clearDownloading(ConvertType type, int itemId); - void finishConvert(const muse::Ret& ret, const muse::io::path_t& path = muse::io::path_t()); - void failConvert(muse::Ret ret, ConvertType type, int itemId, const muse::String& convertedFileName); + void updateWatchedScores(const muse::cloud::ConvertQueueList& queue); + + void updateStatus(WatchedScore& watched, muse::cloud::ConvertStatus status, muse::cloud::ConvertErrorCode errorCode); + + void finishConvert(const muse::Ret& ret, const WatchedScore& watched = WatchedScore()); + + WatchedScore* findWatchedScoreByScoreId(int scoreId); ConvertConfig m_config; QTimer m_timer; int m_pollIntervalMs = MIN_RETRY_INTERVAL_MS; int m_pollFailureCount = 0; - std::vector m_watchedItems; + std::vector m_watchedScores; bool m_pollInProgress = false; - muse::async::Channel m_convertFinished; - muse::async::Channel m_reviewRequested; - muse::async::Notification m_fileNamesBeingConvertedChanged; + bool m_isSaving = false; + muse::async::Channel m_pollingFailed; + muse::async::Notification m_watchedScoresChanged; + muse::async::Channel m_convertFinished; + muse::async::Channel m_reviewRequested; }; } diff --git a/src/project/internal/projectconfiguration.cpp b/src/project/internal/projectconfiguration.cpp index af09743c3f40e..eec8d9431b846 100644 --- a/src/project/internal/projectconfiguration.cpp +++ b/src/project/internal/projectconfiguration.cpp @@ -795,12 +795,7 @@ void ProjectConfiguration::setShowConvertFileProcessingDialog(bool show) settings()->setSharedValue(SHOW_CONVERT_FILE_PROCESSING_DIALOG, Val(show)); } -muse::io::path_t ProjectConfiguration::convertedScoresPath() const +muse::io::path_t ProjectConfiguration::watchedConvertsJsonPath() const { - return globalConfiguration()->userAppDataPath() + "/converted_scores"; -} - -muse::io::path_t ProjectConfiguration::pendingConvertsJsonPath() const -{ - return globalConfiguration()->userAppDataPath().appendingComponent("pending_converts.json"); + return globalConfiguration()->userAppDataPath().appendingComponent("watched_converts.json"); } diff --git a/src/project/internal/projectconfiguration.h b/src/project/internal/projectconfiguration.h index c08da50856ccc..5d7798f05a4e9 100644 --- a/src/project/internal/projectconfiguration.h +++ b/src/project/internal/projectconfiguration.h @@ -174,17 +174,16 @@ class ProjectConfiguration : public IProjectConfiguration, public muse::Contexta bool showConvertFileProcessingDialog() const override; void setShowConvertFileProcessingDialog(bool show) override; - muse::io::path_t convertedScoresPath() const override; - muse::io::path_t pendingConvertsJsonPath() const override; - - std::string uniqueFileNameAddition(const muse::io::path_t& filename, const muse::io::path_t& folderPath, - const std::string& suffix = std::string()) const override; + muse::io::path_t watchedConvertsJsonPath() const override; private: muse::io::path_t appTemplatesPath() const; muse::io::path_t legacyCloudProjectsPath() const; muse::io::path_t cloudProjectsPath() const; + std::string uniqueFileNameAddition(const muse::io::path_t& filename, const muse::io::path_t& folderPath, + const std::string& suffix = std::string()) const; + muse::async::Channel m_userTemplatesPathChanged; muse::async::Channel m_userScoresPathChanged; diff --git a/src/project/iprojectconfiguration.h b/src/project/iprojectconfiguration.h index 35bfb466955c4..742e8956308dd 100644 --- a/src/project/iprojectconfiguration.h +++ b/src/project/iprojectconfiguration.h @@ -180,11 +180,7 @@ class IProjectConfiguration : MODULE_GLOBAL_INTERFACE virtual bool showConvertFileProcessingDialog() const = 0; virtual void setShowConvertFileProcessingDialog(bool show) = 0; - virtual muse::io::path_t convertedScoresPath() const = 0; - virtual muse::io::path_t pendingConvertsJsonPath() const = 0; - - virtual std::string uniqueFileNameAddition(const muse::io::path_t& filename, const muse::io::path_t& folderPath, - const std::string& suffix = std::string()) const = 0; + virtual muse::io::path_t watchedConvertsJsonPath() const = 0; }; } diff --git a/src/project/projecterrors.h b/src/project/projecterrors.h index ece45a99b273b..cff03a8c351f2 100644 --- a/src/project/projecterrors.h +++ b/src/project/projecterrors.h @@ -54,7 +54,6 @@ enum class Err { ConvertTooManyImages, ConvertUnsupportedLink, ConvertProcessingFailed, - DownloadLinkExpired, }; //! NOTE: key for the converted file name stored in Ret::data diff --git a/src/project/qml/MuseScore/Project/ConvertFileToScoreDialog.qml b/src/project/qml/MuseScore/Project/ConvertFileToScoreDialog.qml index 8073adbe2cac5..ee89bab956a6b 100644 --- a/src/project/qml/MuseScore/Project/ConvertFileToScoreDialog.qml +++ b/src/project/qml/MuseScore/Project/ConvertFileToScoreDialog.qml @@ -72,9 +72,9 @@ StyledDialogView { } } - function finish(type, paths, link, convertedFileName) { + function finish(type, paths, link, convertedScoreName) { root.skipCloseConfirmation = true - root.ret = { errcode: 0, value: { type: type, paths: paths, link: link, convertedFileName: convertedFileName } } + root.ret = { errcode: 0, value: { type: type, paths: paths, link: link, convertedScoreName: convertedScoreName } } root.hide() } @@ -194,8 +194,8 @@ StyledDialogView { onBackRequested: convertModel.confirmGoingBack() - onConvertRequested: function(paths, convertedFileName) { - root.finish(convertModel.convertType, paths, "", convertedFileName) + onConvertRequested: function(paths, convertedScoreName) { + root.finish(convertModel.convertType, paths, "", convertedScoreName) } onSelectMoreFilesRequested: function(existingPaths) { @@ -220,13 +220,13 @@ StyledDialogView { onBackRequested: convertModel.confirmGoingBack() - onConvertRequested: function(link, convertedFileName) { + onConvertRequested: function(link, convertedScoreName) { if (!convertModel.validateLink(link)) { return } convertModel.selectedLink = link - root.finish(convertModel.convertType, [], link, convertedFileName) + root.finish(convertModel.convertType, [], link, convertedScoreName) } } } diff --git a/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/LinkEntryPage.qml b/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/LinkEntryPage.qml index f197c8d5fd3fc..f5d712a062fe7 100644 --- a/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/LinkEntryPage.qml +++ b/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/LinkEntryPage.qml @@ -42,7 +42,7 @@ Item { signal cancelRequested() signal backRequested() - signal convertRequested(string link, string convertedFileName) + signal convertRequested(string link, string convertedScoreName) function focusOnDefault() { linkInputField.navigation.requestActive() diff --git a/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/SaveAsField.qml b/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/SaveAsField.qml index 39ea13dbd2a96..7e93defb39c55 100644 --- a/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/SaveAsField.qml +++ b/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/SaveAsField.qml @@ -51,6 +51,8 @@ Column { width: parent.width + maximumLength: 255 + navigation.panel: root.navigationPanel navigation.order: root.navigationOrder navigation.accessible.name: label.text + " " + input.currentText diff --git a/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/SelectedFilesPage.qml b/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/SelectedFilesPage.qml index 3f700a21d6d73..2402330683647 100644 --- a/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/SelectedFilesPage.qml +++ b/src/project/qml/MuseScore/Project/internal/ConvertFileToScore/SelectedFilesPage.qml @@ -43,7 +43,7 @@ Item { signal cancelRequested() signal backRequested() signal selectMoreFilesRequested(var existingPaths) - signal convertRequested(var paths, string convertedFileName) + signal convertRequested(var paths, string convertedScoreName) function focusOnDefault() { saveAsField.focusOnInput() diff --git a/src/project/qml/MuseScore/Project/internal/ScoresPage/recentscoresmodel.cpp b/src/project/qml/MuseScore/Project/internal/ScoresPage/recentscoresmodel.cpp index 5c3e5661c59fe..1faf6d8852520 100644 --- a/src/project/qml/MuseScore/Project/internal/ScoresPage/recentscoresmodel.cpp +++ b/src/project/qml/MuseScore/Project/internal/ScoresPage/recentscoresmodel.cpp @@ -45,15 +45,9 @@ void RecentScoresModel::load() updateRecentScores(); }); - convertFileToScoreService()->fileNamesBeingConvertedChanged().onNotify(this, [this]() { + convertFileToScoreService()->watchedScores().notification.onNotify(this, [this]() { updateRecentScores(); }); - - convertFileToScoreService()->convertFinished().onReceive(this, [this](const Ret& ret, const io::path_t& path) { - if (ret) { - recentFilesController()->prependRecentFile(RecentFile(path)); - } - }); } void RecentScoresModel::setRecentScores(const std::vector& items) @@ -70,10 +64,10 @@ void RecentScoresModel::setRecentScores(const std::vector& items) void RecentScoresModel::updateRecentScores() { const RecentFilesList& recentScores = recentFilesController()->recentFilesList(); - const StringList processingFiles = convertFileToScoreService()->fileNamesBeingConverted(); + const WatchedScoreList watchedScores = convertFileToScoreService()->watchedScores().val; std::vector items; - items.reserve(recentScores.size() + processingFiles.size() + 2); + items.reserve(recentScores.size() + watchedScores.size() + 2); QVariantMap addItem; addItem[NAME_KEY] = muse::qtrc("project", "New score"); @@ -83,9 +77,13 @@ void RecentScoresModel::updateRecentScores() addItem[IS_CLOUD_KEY] = false; items.push_back(addItem); - for (const String& fileName : processingFiles) { + for (const WatchedScore& watchedScore : watchedScores) { + if (watchedScore.conversion.status != ConvertStatus::Processing) { + continue; + } + QVariantMap obj; - obj[NAME_KEY] = fileName.toQString(); + obj[NAME_KEY] = watchedScore.name.toQString(); obj[IS_CREATE_NEW_KEY] = false; obj[IS_NO_RESULTS_FOUND_KEY] = false; obj[IS_PROCESSING_KEY] = true; diff --git a/src/project/tests/convertfiletoscorescenario_tests.cpp b/src/project/tests/convertfiletoscorescenario_tests.cpp index b492654219880..e4d255effc7b2 100644 --- a/src/project/tests/convertfiletoscorescenario_tests.cpp +++ b/src/project/tests/convertfiletoscorescenario_tests.cpp @@ -144,7 +144,7 @@ QVariantMap pickedOmrFileSelection() return { { "type", int(ConvertType::Omr) }, { "paths", QStringList { "/some/file.xyz" } }, - { "convertedFileName", QString("file") } + { "convertedScoreName", QString("file") } }; } } @@ -251,13 +251,16 @@ class Project_ConvertFileToScoreScenarioTest : public ::testing::Test TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Success_ShowsScoreReadyNotificationAndForwards) { // [GIVEN] The service's channels, wired up via init() - async::Channel convertFinished; - async::Channel reviewRequested; + async::Channel convertFinished; + async::Channel reviewRequested; ON_CALL(*m_service, convertFinished()).WillByDefault(Return(convertFinished)); ON_CALL(*m_service, reviewRequested()).WillByDefault(Return(reviewRequested)); m_scenario->init(); - const io::path_t path = "/some/path/My Score.xyz"; + WatchedScore watched; + watched.scoreId = 555; + watched.name = u"My Score"; + constexpr int openScoreBtn = int(toast::ToastActionCode::Custom) + 1; const std::string title = muse::trc("project/convert", "Your score is ready!"); @@ -275,24 +278,60 @@ TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Success_ShowsScoreReadyNotif bool forwarded = false; Ret forwardedRet; - m_scenario->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { + WatchedScore forwardedWatched; + m_scenario->convertFinished().onReceive(nullptr, [&](const Ret& ret, const WatchedScore& w) { forwarded = true; forwardedRet = ret; + forwardedWatched = w; }); // [WHEN] The service reports a successful conversion - convertFinished.send(make_ok(), path); + convertFinished.send(make_ok(), watched); - // [THEN] The result is forwarded to the scenario's own convertFinished channel + // [THEN] The result, including the WatchedScore payload, is forwarded to the scenario's own convertFinished channel EXPECT_TRUE(forwarded); EXPECT_TRUE(forwardedRet); + ASSERT_TRUE(forwardedWatched.scoreId.has_value()); + EXPECT_EQ(*forwardedWatched.scoreId, 555); + EXPECT_EQ(forwardedWatched.name, u"My Score"); +} + +TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Success_OpenScoreButton_DispatchesOpenScoreUrl) +{ + // [GIVEN] The service's channels, wired up via init() + async::Channel convertFinished; + async::Channel reviewRequested; + ON_CALL(*m_service, convertFinished()).WillByDefault(Return(convertFinished)); + ON_CALL(*m_service, reviewRequested()).WillByDefault(Return(reviewRequested)); + m_scenario->init(); + + WatchedScore watched; + watched.scoreId = 555; + watched.name = u"My Score"; + + // [GIVEN] The user clicks "Open score" on the ready notification + constexpr int openScoreBtn = int(toast::ToastActionCode::Custom) + 1; + ON_CALL(*m_toastService, show(_, _, _, _, _)) + .WillByDefault(Invoke([](auto&&...) { + return resolvedToastResultPromise(toast::ToastResult(openScoreBtn)); + })); + + // [THEN] The score is opened via the cloud open-score URL, not a local path + EXPECT_CALL(*m_dispatcher, dispatch(actions::ActionCode("file-open"), Truly([](const actions::ActionData& data) { + return data.arg(0) == QUrl("musescore://open-score/555"); + }))) + .Times(1); + + // [WHEN] The service reports a successful conversion + convertFinished.send(make_ok(), watched); + pumpEvents(); } TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Failure_ShowsConvertFailedNotificationAndForwards) { // [GIVEN] The service's channels, wired up via init() - async::Channel convertFinished; - async::Channel reviewRequested; + async::Channel convertFinished; + async::Channel reviewRequested; ON_CALL(*m_service, convertFinished()).WillByDefault(Return(convertFinished)); ON_CALL(*m_service, reviewRequested()).WillByDefault(Return(reviewRequested)); m_scenario->init(); @@ -315,13 +354,13 @@ TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Failure_ShowsConvertFailedNo bool forwarded = false; Ret forwardedRet; - m_scenario->convertFinished().onReceive(nullptr, [&](const Ret& r, const io::path_t&) { + m_scenario->convertFinished().onReceive(nullptr, [&](const Ret& r, const WatchedScore&) { forwarded = true; forwardedRet = r; }); // [WHEN] The service reports a failed conversion - convertFinished.send(ret, io::path_t()); + convertFinished.send(ret, WatchedScore()); // [THEN] The failure is still forwarded to the scenario's own convertFinished channel EXPECT_TRUE(forwarded); @@ -331,8 +370,8 @@ TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Failure_ShowsConvertFailedNo TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Failure_TryAgain_RestartsConvert) { // [GIVEN] The service's channels, wired up via init() - async::Channel convertFinished; - async::Channel reviewRequested; + async::Channel convertFinished; + async::Channel reviewRequested; ON_CALL(*m_service, convertFinished()).WillByDefault(Return(convertFinished)); ON_CALL(*m_service, reviewRequested()).WillByDefault(Return(reviewRequested)); m_scenario->init(); @@ -347,7 +386,7 @@ TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Failure_TryAgain_RestartsCon const QVariantMap selectionMap { { "type", int(ConvertType::Omr) }, { "paths", QStringList { "/some/file.xyz" } }, - { "convertedFileName", QString("file") } + { "convertedScoreName", QString("file") } }; ON_CALL(*m_interactive, open(UriQuery("musescore://project/convert/selectfiles"))) .WillByDefault(Invoke([selectionMap](auto&&...) { @@ -363,7 +402,7 @@ TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Failure_TryAgain_RestartsCon // [WHEN] The service reports a failed conversion Ret ret = make_ret(Err::ConvertProcessingFailed); ret.setData(CONVERT_FAILED_FILE_NAME_KEY, muse::String(u"My Score")); - convertFinished.send(ret, io::path_t()); + convertFinished.send(ret, WatchedScore()); pumpEvents(); } @@ -371,8 +410,8 @@ TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Failure_TryAgain_RestartsCon TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Failure_Dismiss_DoesNotRestartConvert) { // [GIVEN] The service's channels, wired up via init() - async::Channel convertFinished; - async::Channel reviewRequested; + async::Channel convertFinished; + async::Channel reviewRequested; ON_CALL(*m_service, convertFinished()).WillByDefault(Return(convertFinished)); ON_CALL(*m_service, reviewRequested()).WillByDefault(Return(reviewRequested)); m_scenario->init(); @@ -388,71 +427,40 @@ TEST_F(Project_ConvertFileToScoreScenarioTest, Init_Failure_Dismiss_DoesNotResta EXPECT_CALL(*m_service, startConvert(_, _)).Times(0); // [WHEN] The service reports a failed conversion - convertFinished.send(make_ret(Err::ConvertProcessingFailed), io::path_t()); + convertFinished.send(make_ret(Err::ConvertProcessingFailed), WatchedScore()); pumpEvents(); } -TEST_F(Project_ConvertFileToScoreScenarioTest, DISABLED_Init_ReviewRequested_Good_SubmitsGoodRating) -{ - // [GIVEN] The service's channels, wired up via init() - async::Channel convertFinished; - async::Channel reviewRequested; - ON_CALL(*m_service, convertFinished()).WillByDefault(Return(convertFinished)); - ON_CALL(*m_service, reviewRequested()).WillByDefault(Return(reviewRequested)); - m_scenario->init(); - - constexpr int goodBtn = int(toast::ToastActionCode::Custom) + 1; - constexpr int badBtn = int(toast::ToastActionCode::Custom) + 2; - - const std::string title = muse::trc("project/convert", "How does your score look?"); - const std::string text = muse::trc("project/convert", - "We’re always improving our score conversion accuracy. Let us know how we did with this one."); - - // [THEN] The review rating toast is shown, and the user's pick of "Good" is submitted - EXPECT_CALL(*m_toastService, - show(title, text, muse::ui::IconCode::Code::NONE, true, ToastActionCodesAre({ goodBtn, badBtn }))) - .WillOnce(Invoke([goodBtn](auto&&...) { - return resolvedToastResultPromise(toast::ToastResult(goodBtn)); - })); - - EXPECT_CALL(*m_service, submitReview(ConvertType::Omr, 42, ReviewRating::Good, QString())) - .Times(1); - - // [WHEN] The service requests a review for a finished conversion - reviewRequested.send(ConvertType::Omr, 42, io::path_t("/some/path/My Score.mscz")); - - pumpEvents(); -} +// ================================================== +// init() -- pollingFailed() +// ================================================== -TEST_F(Project_ConvertFileToScoreScenarioTest, DISABLED_Init_ReviewRequested_Bad_SubmitsBadRating) +TEST_F(Project_ConvertFileToScoreScenarioTest, Init_PollingFailed_ShowsToastOnceAfterThreshold) { // [GIVEN] The service's channels, wired up via init() - async::Channel convertFinished; - async::Channel reviewRequested; + async::Channel convertFinished; + async::Channel reviewRequested; + async::Channel pollingFailed; ON_CALL(*m_service, convertFinished()).WillByDefault(Return(convertFinished)); ON_CALL(*m_service, reviewRequested()).WillByDefault(Return(reviewRequested)); + ON_CALL(*m_service, pollingFailed()).WillByDefault(Return(pollingFailed)); m_scenario->init(); - constexpr int goodBtn = int(toast::ToastActionCode::Custom) + 1; - constexpr int badBtn = int(toast::ToastActionCode::Custom) + 2; + const std::string title = muse::trc("project/convert", "We’re having trouble connecting to the internet."); + const std::string text = muse::trc("project/convert", "We’ll keep trying intermittently."); - const std::string title = muse::trc("project/convert", "How does your score look?"); - const std::string text = muse::trc("project/convert", - "We’re always improving our score conversion accuracy. Let us know how we did with this one."); - - // [THEN] The review rating toast is shown, and the user's pick of "Bad" is submitted - EXPECT_CALL(*m_toastService, - show(title, text, muse::ui::IconCode::Code::NONE, true, ToastActionCodesAre({ goodBtn, badBtn }))) - .WillOnce(Invoke([badBtn](auto&&...) { - return resolvedToastResultPromise(toast::ToastResult(badBtn)); - })); - - EXPECT_CALL(*m_service, submitReview(ConvertType::Audio2Score, 7, ReviewRating::Bad, QString())) - .Times(1); + // [THEN] The connectivity toast is shown exactly once + EXPECT_CALL(*m_toastService, showWarning(title, text)).Times(1); - // [WHEN] The service requests a review for a finished conversion - reviewRequested.send(ConvertType::Audio2Score, 7, io::path_t("/some/path/My Score.mscz")); + // [WHEN] Polling fails below the attempt threshold (4), then reaches and passes it, and + // eventually gives up + pollingFailed.send(PollingFailure { Ret(), 1, 5, secs_t(0), false }); + pollingFailed.send(PollingFailure { Ret(), 2, 5, secs_t(0), false }); + pollingFailed.send(PollingFailure { Ret(), 3, 5, secs_t(0), false }); + pollingFailed.send(PollingFailure { Ret(), 4, 5, secs_t(0), false }); + pollingFailed.send(PollingFailure { Ret(), 5, 5, secs_t(0), false }); + pollingFailed.send(PollingFailure { Ret(), 5, 5, secs_t(0), true }); pumpEvents(); } @@ -1068,7 +1076,7 @@ TEST_F(Project_ConvertFileToScoreScenarioTest, ConvertFiles_Proceeds_StartsOmrCo const QVariantMap selectionMap { { "type", int(ConvertType::Omr) }, { "paths", QStringList { "/some/path/file.xyz" } }, - { "convertedFileName", QString("file") } + { "convertedScoreName", QString("file") } }; EXPECT_CALL(*m_interactive, open(expectedQuery)) .WillOnce(Invoke([selectionMap](auto&&...) { @@ -1117,7 +1125,7 @@ TEST_F(Project_ConvertFileToScoreScenarioTest, ConvertFiles_Proceeds_StartsAudio const QVariantMap selectionMap { { "type", int(ConvertType::Audio2Score) }, { "paths", QStringList { "/some/path/song.xyz" } }, - { "convertedFileName", QString("song") } + { "convertedScoreName", QString("song") } }; EXPECT_CALL(*m_interactive, open(expectedQuery)) .WillOnce(Invoke([selectionMap](auto&&...) { diff --git a/src/project/tests/convertfiletoscoreservice_tests.cpp b/src/project/tests/convertfiletoscoreservice_tests.cpp index 58a4b72bb9302..35ee0c32c73fb 100644 --- a/src/project/tests/convertfiletoscoreservice_tests.cpp +++ b/src/project/tests/convertfiletoscoreservice_tests.cpp @@ -21,11 +21,10 @@ */ #include -#include -#include +#include #include +#include -#include #include #include "project/internal/convertfiletoscoreservice.h" @@ -40,12 +39,12 @@ #include "global/types/val.h" #include "global/types/bytearray.h" #include "global/serialization/json.h" -#include "global/io/ioretcodes.h" #include "mocks/projectconfigurationmock.h" #include "global/tests/mocks/filesystemmock.h" #include "cloud/tests/mocks/musescorecomservicemock.h" #include "cloud/tests/mocks/musescorecomconvertservicemock.h" +#include "multiwindows/tests/mocks/multiwindowsprovidermock.h" using namespace ::testing; using namespace mu::project; @@ -63,17 +62,6 @@ void pumpEvents(int iterations = 10) } } -//! NOTE: FS write/makePath retries are scheduled via QTimer::singleShot, which (unlike -//! pumpEvents() above) needs the real Qt event loop pumped and real time to actually pass -void waitUntil(const std::function& pred, int timeoutMs = 2000) -{ - const std::chrono::steady_clock::time_point deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(timeoutMs); - while (!pred() && std::chrono::steady_clock::now() < deadline) { - QCoreApplication::processEvents(); - std::this_thread::sleep_for(std::chrono::milliseconds(5)); - } -} - //! NOTE: keep the body queued, don't run it eagerly //! otherwise resolve() can fire before onResolve/onReject are attached template @@ -112,6 +100,21 @@ async::Promise pendingPromise() return async::Promise::dummy_result(); }); } + +bool uploadDataMatchesPaths(const ConvertUploadData& data, const io::paths_t& paths) +{ + if (data.files.size() != paths.size()) { + return false; + } + + for (size_t i = 0; i < paths.size(); ++i) { + if (data.files[i].fileName != io::filename(paths[i])) { + return false; + } + } + + return true; +} } namespace mu::project { @@ -126,16 +129,21 @@ class Project_ConvertFileToScoreServiceTest : public ::testing::Test m_convertService = std::make_shared >(); m_fileSystem = std::make_shared >(); m_configuration = std::make_shared >(); + m_multiWindowsProvider = std::make_shared >(); m_service->museScoreComService.set(m_museScoreComService); m_service->fileSystem.set(m_fileSystem); m_service->configuration.set(m_configuration); + m_service->multiwindowsProvider.set(m_multiWindowsProvider); ON_CALL(*m_museScoreComService, convert()) .WillByDefault(Return(m_convertService)); ON_CALL(*m_fileSystem, fileSize(_)) .WillByDefault(Return(RetVal::make_ok(1024))); + + ON_CALL(*m_fileSystem, readFile(_)) + .WillByDefault(Return(RetVal::make_ok(ByteArray()))); } void setConfig(const ConvertConfig& config) @@ -159,7 +167,7 @@ class Project_ConvertFileToScoreServiceTest : public ::testing::Test //! NOTE: uploads the given file, resolves the upload with queueId, and lets the resulting //! poll (mocked to return queueList) run to completion - void deliverQueueStatus(const ConvertQueueList& queueList, ConvertType type, int queueId, const QString& convertedFileName) + void deliverQueueStatus(const ConvertQueueList& queueList, ConvertType type, int queueId, const QString& convertedScoreName) { ON_CALL(*m_convertService, fetchQueue()) .WillByDefault(Invoke([queueList] { @@ -175,7 +183,7 @@ class Project_ConvertFileToScoreServiceTest : public ::testing::Test ? ConvertInput(OmrConvertInput { paths }) : ConvertInput(Audio2ScoreConvertInput { paths }); - m_service->startConvert(input, convertedFileName); + m_service->startConvert(input, convertedScoreName); uploadProgress->finish(ProgressResult::make_ok(Val(ValMap { { "id", Val(queueId) } }))); pumpEvents(); @@ -184,15 +192,15 @@ class Project_ConvertFileToScoreServiceTest : public ::testing::Test //! NOTE: uploads the given file and resolves with queueId, triggering a fresh poll //! (watch() always re-polls all watched items) without touching the fetchQueue mock, //! which the caller owns - lets a test drive N polls without waiting on the real QTimer - void uploadAndResolve(int queueId, const QString& convertedFileName, const io::paths_t& paths) + void uploadAndResolve(int queueId, const QString& convertedScoreName, const io::paths_t& paths) { auto uploadProgress = std::make_shared(); - EXPECT_CALL(*m_convertService, upload(Truly([paths](const ConvertInput& input) { - return convertPathsOf(input) == paths; + EXPECT_CALL(*m_convertService, upload(Truly([paths](const ConvertUploadDataPtr& data) { + return uploadDataMatchesPaths(*data, paths); }))) .WillOnce(Return(uploadProgress)); - m_service->startConvert(OmrConvertInput { paths }, convertedFileName); + m_service->startConvert(OmrConvertInput { paths }, convertedScoreName); uploadProgress->finish(ProgressResult::make_ok(Val(ValMap { { "id", Val(queueId) } }))); pumpEvents(); @@ -203,6 +211,7 @@ class Project_ConvertFileToScoreServiceTest : public ::testing::Test std::shared_ptr m_convertService; std::shared_ptr m_fileSystem; std::shared_ptr m_configuration; + std::shared_ptr m_multiWindowsProvider; }; } @@ -562,7 +571,7 @@ TEST_F(Project_ConvertFileToScoreServiceTest, StartConvert_UploadFails_ForwardsF bool received = false; Ret receivedRet; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { + m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const WatchedScore&) { received = true; receivedRet = ret; }); @@ -582,14 +591,14 @@ TEST_F(Project_ConvertFileToScoreServiceTest, StartConvert_UploadFails_ForwardsF TEST_F(Project_ConvertFileToScoreServiceTest, StartConvert_UploadSucceeds_PersistsWatchedItemAndPolls) { // [GIVEN] The upload succeeds with queue id TEST_QUEUE_ID - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); + ON_CALL(*m_configuration, watchedConvertsJsonPath()) + .WillByDefault(Return(io::path_t("/watched.json"))); auto uploadProgress = std::make_shared(); const io::paths_t paths { "/some/path/file.pdf" }; - EXPECT_CALL(*m_convertService, upload(Truly([&](const ConvertInput& input) { - return convertTypeOf(input) == ConvertType::Omr && convertPathsOf(input) == paths; + EXPECT_CALL(*m_convertService, upload(Truly([&](const ConvertUploadDataPtr& data) { + return data->type == ConvertType::Omr && uploadDataMatchesPaths(*data, paths); }))) .WillOnce(Return(uploadProgress)); @@ -601,7 +610,7 @@ TEST_F(Project_ConvertFileToScoreServiceTest, StartConvert_UploadSucceeds_Persis })); bool savedExpectedEntry = false; - EXPECT_CALL(*m_fileSystem, writeFile(io::path_t("/pending.json"), _)) + EXPECT_CALL(*m_fileSystem, writeFile(io::path_t("/watched.json"), _)) .WillOnce(Invoke([&](const io::path_t&, const ByteArray& data) { std::string err; JsonDocument json = JsonDocument::fromJson(data, &err); @@ -609,7 +618,7 @@ TEST_F(Project_ConvertFileToScoreServiceTest, StartConvert_UploadSucceeds_Persis JsonObject obj = json.rootArray().at(0).toObject(); savedExpectedEntry = obj.value("id").toInt() == TEST_QUEUE_ID && obj.value("type").toInt() == int(ConvertType::Omr) - && obj.value("convertedFileName").toStdString() == "My Score"; + && obj.value("convertedScoreName").toStdString() == "My Score"; } return make_ok(); })); @@ -626,8 +635,8 @@ TEST_F(Project_ConvertFileToScoreServiceTest, StartConvert_UploadSucceeds_Persis TEST_F(Project_ConvertFileToScoreServiceTest, StartConvert_UploadSucceeds_PersistsAudio2ScoreType) { // [GIVEN] The upload succeeds for an Audio2Score conversion - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); + ON_CALL(*m_configuration, watchedConvertsJsonPath()) + .WillByDefault(Return(io::path_t("/watched.json"))); auto uploadProgress = std::make_shared(); ON_CALL(*m_convertService, upload(_)) @@ -638,7 +647,7 @@ TEST_F(Project_ConvertFileToScoreServiceTest, StartConvert_UploadSucceeds_Persis })); bool savedExpectedType = false; - EXPECT_CALL(*m_fileSystem, writeFile(io::path_t("/pending.json"), _)) + EXPECT_CALL(*m_fileSystem, writeFile(io::path_t("/watched.json"), _)) .WillOnce(Invoke([&](const io::path_t&, const ByteArray& data) { std::string err; JsonDocument json = JsonDocument::fromJson(data, &err); @@ -659,19 +668,19 @@ TEST_F(Project_ConvertFileToScoreServiceTest, StartConvert_UploadSucceeds_Persis } // ================================================== -// fileNamesBeingConverted() / fileNamesBeingConvertedChanged() +// watchedScores() // ================================================== -TEST_F(Project_ConvertFileToScoreServiceTest, FileNamesBeingConverted_Initially_Empty) +TEST_F(Project_ConvertFileToScoreServiceTest, WatchedScores_Initially_Empty) { - EXPECT_TRUE(m_service->fileNamesBeingConverted().empty()); + EXPECT_TRUE(m_service->watchedScores().val.empty()); } -TEST_F(Project_ConvertFileToScoreServiceTest, FileNamesBeingConverted_AfterStartConvert_ContainsFileNameAndFiresChanged) +TEST_F(Project_ConvertFileToScoreServiceTest, WatchedScores_AfterStartConvert_ContainsScoreAndFiresChanged) { // [GIVEN] The upload succeeds, and polling is left pending (the item stays watched) - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); + ON_CALL(*m_configuration, watchedConvertsJsonPath()) + .WillByDefault(Return(io::path_t("/watched.json"))); ON_CALL(*m_fileSystem, writeFile(_, _)) .WillByDefault(Return(make_ok())); @@ -685,7 +694,7 @@ TEST_F(Project_ConvertFileToScoreServiceTest, FileNamesBeingConverted_AfterStart })); bool changed = false; - m_service->fileNamesBeingConvertedChanged().onNotify(nullptr, [&] { + m_service->watchedScores().notification.onNotify(nullptr, [&] { changed = true; }); @@ -693,61 +702,69 @@ TEST_F(Project_ConvertFileToScoreServiceTest, FileNamesBeingConverted_AfterStart m_service->startConvert(OmrConvertInput { paths }, u"My Score"); uploadProgress->finish(ProgressResult::make_ok(Val(ValMap { { "id", Val(TEST_QUEUE_ID) } }))); - // [THEN] The file being converted is reported, and the change is signaled + // [THEN] The score being converted is reported, and the change is signaled EXPECT_TRUE(changed); - ASSERT_EQ(m_service->fileNamesBeingConverted().size(), 1u); - EXPECT_EQ(m_service->fileNamesBeingConverted().front(), u"My Score"); + const WatchedScoreList watchedScores = m_service->watchedScores().val; + ASSERT_EQ(watchedScores.size(), 1u); + EXPECT_EQ(watchedScores.front().name, u"My Score"); } -TEST_F(Project_ConvertFileToScoreServiceTest, FileNamesBeingConverted_AfterSuccessfulDownload_NoLongerContainsFileName) +TEST_F(Project_ConvertFileToScoreServiceTest, WatchedScores_AfterDone_NoLongerContainsScore) { - // [GIVEN] The queue reports the conversion as done, and the download succeeds + // [GIVEN] The queue reports the conversion as done, with its scoreId ConvertQueueItem item; item.id = TEST_QUEUE_ID; item.type = ConvertType::Omr; item.status = ConvertStatus::Done; + item.scoreId = 555; - ON_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .WillByDefault(Invoke([] { - SignedMsczUrl url; - url.url = QUrl("https://link.xyz/score.mscz"); - url.expiresInSeconds = 60; - return resolvedPromise >(RetVal::make_ok(url)); - })); - - auto downloadProgress = std::make_shared(); - ON_CALL(*m_convertService, downloadConvertedScore(_, _)) - .WillByDefault(Return(downloadProgress)); - - ON_CALL(*m_fileSystem, makePath(_)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_fileSystem, writeFile(_, _)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_configuration, convertedScoresPath()) - .WillByDefault(Return(io::path_t("/scores"))); - ON_CALL(*m_configuration, uniqueFileNameAddition(_, _, _)) - .WillByDefault(Return(std::string())); + bool changed = false; + m_service->watchedScores().notification.onNotify(nullptr, [&] { + changed = true; + }); // [WHEN] Uploading and polling the status deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); - ASSERT_EQ(m_service->fileNamesBeingConverted().size(), 1u); + // [THEN] The item is reported ready and immediately erased, so it's no longer being converted + EXPECT_TRUE(changed); + EXPECT_TRUE(m_service->watchedScores().val.empty()); +} - bool changed = false; - m_service->fileNamesBeingConvertedChanged().onNotify(nullptr, [&] { - changed = true; - }); +TEST_F(Project_ConvertFileToScoreServiceTest, WatchedScores_ExternalProcessingItem_AddedToWatchedScores) +{ + // [GIVEN] The queue reports an item that was never started via startConvert() locally, + // alongside the one that was + const int externalId = TEST_QUEUE_ID + 1; - // [AND WHEN] The download completes - downloadProgress->finish(ProgressResult::make_ok(Val())); + ConvertQueueItem ownItem; + ownItem.id = TEST_QUEUE_ID; + ownItem.type = ConvertType::Omr; + ownItem.status = ConvertStatus::Processing; - // [THEN] The file is no longer reported as being converted - EXPECT_TRUE(changed); - EXPECT_TRUE(m_service->fileNamesBeingConverted().empty()); + ConvertQueueItem externalItem; + externalItem.id = externalId; + externalItem.type = ConvertType::Omr; + externalItem.status = ConvertStatus::Processing; + externalItem.filename = "Externally Started Score"; + + // [WHEN] Uploading and polling the status + deliverQueueStatus({ ownItem, externalItem }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); + + // [THEN] Both items are watched - the one we started, and the one discovered via the queue + const WatchedScoreList watchedScores = m_service->watchedScores().val; + ASSERT_EQ(watchedScores.size(), 2u); + + const auto externalIt = std::find_if(watchedScores.begin(), watchedScores.end(), [externalId](const WatchedScore& watched) { + return watched.conversion.id == externalId; + }); + ASSERT_NE(externalIt, watchedScores.end()); + EXPECT_EQ(externalIt->name, u"Externally Started Score"); + EXPECT_FALSE(externalIt->scoreId.has_value()); } // ================================================== -// resumeConvert() / loadWatchedItems() +// resumeConvert() // ================================================== TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_LoadsPersistedWatchedItem) @@ -756,15 +773,15 @@ TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_LoadsPersistedWatche JsonObject obj; obj["id"] = TEST_QUEUE_ID; obj["type"] = int(ConvertType::Audio2Score); - obj["convertedFileName"] = "My Score"; + obj["convertedScoreName"] = "My Score"; JsonArray array; array << obj; JsonDocument json(array); - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); - ON_CALL(*m_fileSystem, readFile(io::path_t("/pending.json"))) + ON_CALL(*m_configuration, watchedConvertsJsonPath()) + .WillByDefault(Return(io::path_t("/watched.json"))); + ON_CALL(*m_fileSystem, readFile(io::path_t("/watched.json"))) .WillByDefault(Return(RetVal::make_ok(json.toJson()))); ON_CALL(*m_convertService, fetchQueue()) @@ -773,7 +790,7 @@ TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_LoadsPersistedWatche })); bool changed = false; - m_service->fileNamesBeingConvertedChanged().onNotify(nullptr, [&] { + m_service->watchedScores().notification.onNotify(nullptr, [&] { changed = true; }); @@ -782,27 +799,28 @@ TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_LoadsPersistedWatche // [THEN] The persisted item is restored and reported as being converted, and polling resumes EXPECT_TRUE(changed); - ASSERT_EQ(m_service->fileNamesBeingConverted().size(), 1u); - EXPECT_EQ(m_service->fileNamesBeingConverted().front(), u"My Score"); + const WatchedScoreList watchedScores = m_service->watchedScores().val; + ASSERT_EQ(watchedScores.size(), 1u); + EXPECT_EQ(watchedScores.front().name, u"My Score"); } -TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_AwaitingReviewItemAlreadyDownloaded_SendsReviewRequested) +TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_AwaitingReviewItemWithScoreId_SendsReviewRequested) { - // [GIVEN] A persisted item that was already downloaded and awaiting review before the app closed + // [GIVEN] A persisted item that was already reported ready and awaiting review before the app closed JsonObject obj; obj["id"] = TEST_QUEUE_ID; obj["type"] = int(ConvertType::Omr); - obj["convertStatus"] = int(ConvertStatus::AwaitingReview); - obj["convertedFileName"] = "My Score"; - obj["downloadedScorePath"] = "/scores/My Score.mscz"; + obj["status"] = int(ConvertStatus::AwaitingReview); + obj["convertedScoreName"] = "My Score"; + obj["scoreId"] = 555; JsonArray array; array << obj; JsonDocument json(array); - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); - ON_CALL(*m_fileSystem, readFile(io::path_t("/pending.json"))) + ON_CALL(*m_configuration, watchedConvertsJsonPath()) + .WillByDefault(Return(io::path_t("/watched.json"))); + ON_CALL(*m_fileSystem, readFile(io::path_t("/watched.json"))) .WillByDefault(Return(RetVal::make_ok(json.toJson()))); ON_CALL(*m_convertService, fetchQueue()) @@ -810,46 +828,37 @@ TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_AwaitingReviewItemAl return pendingPromise >(); })); - // [THEN] No download is attempted - the score is already on disk - EXPECT_CALL(*m_convertService, fetchMsczUrl(_, _)).Times(0); - bool reviewRequested = false; - ConvertType reviewType = ConvertType::Audio2Score; - int reviewQueueId = 0; - io::path_t reviewPath; - m_service->reviewRequested().onReceive(nullptr, [&](ConvertType type, int queueId, const io::path_t& path) { + int reviewScoreId = 0; + m_service->reviewRequested().onReceive(nullptr, [&](int scoreId) { reviewRequested = true; - reviewType = type; - reviewQueueId = queueId; - reviewPath = path; + reviewScoreId = scoreId; }); // [WHEN] Resuming m_service->resumeConvert(); - // [THEN] The review is requested immediately, carrying the previously downloaded score's path + // [THEN] The review is requested immediately, carrying the previously reported scoreId ASSERT_TRUE(reviewRequested); - EXPECT_EQ(reviewType, ConvertType::Omr); - EXPECT_EQ(reviewQueueId, TEST_QUEUE_ID); - EXPECT_EQ(reviewPath, io::path_t("/scores/My Score.mscz")); + EXPECT_EQ(reviewScoreId, 555); } -TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_AwaitingReviewItemNotYetDownloaded_DoesNotSendReviewRequested) +TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_AwaitingReviewItemWithoutScoreId_DoesNotSendReviewRequested) { - // [GIVEN] A persisted item that was awaiting review, but never got a chance to download before the app closed + // [GIVEN] A persisted item that was awaiting review, but never got a chance to report a scoreId before the app closed JsonObject obj; obj["id"] = TEST_QUEUE_ID; obj["type"] = int(ConvertType::Omr); - obj["convertStatus"] = int(ConvertStatus::AwaitingReview); - obj["convertedFileName"] = "My Score"; + obj["status"] = int(ConvertStatus::AwaitingReview); + obj["convertedScoreName"] = "My Score"; JsonArray array; array << obj; JsonDocument json(array); - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); - ON_CALL(*m_fileSystem, readFile(io::path_t("/pending.json"))) + ON_CALL(*m_configuration, watchedConvertsJsonPath()) + .WillByDefault(Return(io::path_t("/watched.json"))); + ON_CALL(*m_fileSystem, readFile(io::path_t("/watched.json"))) .WillByDefault(Return(RetVal::make_ok(json.toJson()))); ON_CALL(*m_convertService, fetchQueue()) @@ -858,400 +867,174 @@ TEST_F(Project_ConvertFileToScoreServiceTest, ResumeConvert_AwaitingReviewItemNo })); bool reviewRequested = false; - m_service->reviewRequested().onReceive(nullptr, [&](ConvertType, int, const io::path_t&) { + m_service->reviewRequested().onReceive(nullptr, [&](int) { reviewRequested = true; }); // [WHEN] Resuming m_service->resumeConvert(); - // [THEN] No review is requested yet - there's no downloaded score to review + // [THEN] No review is requested yet - there's no scoreId to identify the score by EXPECT_FALSE(reviewRequested); } // ================================================== -// polling / download pipeline (via startConvert()) +// polling / score info fetch pipeline // ================================================== -TEST_F(Project_ConvertFileToScoreServiceTest, Poll_DoneStatus_DownloadsAndFinishesWithPath) +TEST_F(Project_ConvertFileToScoreServiceTest, Poll_DoneStatus_FinishesImmediatelyWithWatchedScore) { - // [GIVEN] The queue reports the conversion as done + // [GIVEN] The queue reports the conversion as done, with its scoreId ConvertQueueItem item; item.id = TEST_QUEUE_ID; item.type = ConvertType::Omr; item.status = ConvertStatus::Done; - - ON_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .WillByDefault(Invoke([] { - SignedMsczUrl url; - url.url = QUrl("https://link.xyz/score.mscz"); - url.expiresInSeconds = 60; - return resolvedPromise >(RetVal::make_ok(url)); - })); - - auto downloadProgress = std::make_shared(); - ON_CALL(*m_convertService, downloadConvertedScore(_, _)) - .WillByDefault(Return(downloadProgress)); - - ON_CALL(*m_fileSystem, makePath(_)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_fileSystem, writeFile(_, _)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_configuration, convertedScoresPath()) - .WillByDefault(Return(io::path_t("/scores"))); - ON_CALL(*m_configuration, uniqueFileNameAddition(_, _, _)) - .WillByDefault(Return(std::string())); + item.scoreId = 555; bool received = false; Ret receivedRet; - io::path_t receivedPath; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t& path) { + WatchedScore receivedWatched; + m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const WatchedScore& watched) { received = true; receivedRet = ret; - receivedPath = path; + receivedWatched = watched; }); // [WHEN] Uploading and polling the status deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); - // [AND WHEN] The download completes - downloadProgress->finish(ProgressResult::make_ok(Val())); - - // [THEN] The conversion finishes successfully, with the downloaded score's path - ASSERT_TRUE(received); - EXPECT_TRUE(receivedRet); - EXPECT_TRUE(receivedPath.hasSuffix("mscz")); - EXPECT_NE(receivedPath.toStdString().find("scores"), std::string::npos); -} - -TEST_F(Project_ConvertFileToScoreServiceTest, Download_WriteFileFailsThenSucceeds_RetriesAndFinishesSuccessfully) -{ - // [GIVEN] The queue reports the conversion as done - ConvertQueueItem item; - item.id = TEST_QUEUE_ID; - item.type = ConvertType::Omr; - item.status = ConvertStatus::Done; - - ON_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .WillByDefault(Invoke([] { - SignedMsczUrl url; - url.url = QUrl("https://link.xyz/score.mscz"); - url.expiresInSeconds = 60; - return resolvedPromise >(RetVal::make_ok(url)); - })); - - auto downloadProgress = std::make_shared(); - ON_CALL(*m_convertService, downloadConvertedScore(_, _)) - .WillByDefault(Return(downloadProgress)); - - ON_CALL(*m_fileSystem, makePath(_)) - .WillByDefault(Return(make_ok())); - - // [GIVEN] saveWatchedItems() persists to its own file, unrelated to the converted score itself - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); - EXPECT_CALL(*m_fileSystem, writeFile(io::path_t("/pending.json"), _)) - .Times(AnyNumber()) - .WillRepeatedly(Return(make_ok())); - - ON_CALL(*m_configuration, convertedScoresPath()) - .WillByDefault(Return(io::path_t("/scores"))); - ON_CALL(*m_configuration, uniqueFileNameAddition(_, _, _)) - .WillByDefault(Return(std::string())); - - // [GIVEN] Writing the file fails twice with a transient FS error, then succeeds - EXPECT_CALL(*m_fileSystem, writeFile(Truly([](const io::path_t& path) { return path.hasSuffix("mscz"); }), _)) - .Times(3) - .WillOnce(Return(make_ret(io::Err::FSWriteError))) - .WillOnce(Return(make_ret(io::Err::FSWriteError))) - .WillOnce(Return(make_ok())); - - bool received = false; - Ret receivedRet; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { - received = true; - receivedRet = ret; - }); - - // [WHEN] Uploading and polling the status, then letting the download complete - deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); - downloadProgress->finish(ProgressResult::make_ok(Val())); - - // [THEN] The transient FS failures are retried in place (via a QTimer, hence the wait), and the - // conversion still finishes successfully - waitUntil([&] { return received; }); + // [THEN] The conversion finishes successfully immediately - no separate fetch is needed, since + // the queue already carries everything needed to identify the resulting score ASSERT_TRUE(received); EXPECT_TRUE(receivedRet); + ASSERT_TRUE(receivedWatched.scoreId.has_value()); + EXPECT_EQ(*receivedWatched.scoreId, 555); + EXPECT_EQ(receivedWatched.name, u"My Score"); } -TEST_F(Project_ConvertFileToScoreServiceTest, Download_WriteFileFailsPermanently_FailsConversionAfterMaxRetries) +TEST_F(Project_ConvertFileToScoreServiceTest, Poll_AwaitingReviewWithoutScoreId_DoesNotReportYet) { - // [GIVEN] The queue reports the conversion as done - ConvertQueueItem item; - item.id = TEST_QUEUE_ID; - item.type = ConvertType::Omr; - item.status = ConvertStatus::Done; - - ON_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .WillByDefault(Invoke([] { - SignedMsczUrl url; - url.url = QUrl("https://link.xyz/score.mscz"); - url.expiresInSeconds = 60; - return resolvedPromise >(RetVal::make_ok(url)); - })); - - auto downloadProgress = std::make_shared(); - ON_CALL(*m_convertService, downloadConvertedScore(_, _)) - .WillByDefault(Return(downloadProgress)); - - ON_CALL(*m_fileSystem, makePath(_)) - .WillByDefault(Return(make_ok())); - - // [GIVEN] saveWatchedItems() persists to its own file, unrelated to the converted score itself - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); - EXPECT_CALL(*m_fileSystem, writeFile(io::path_t("/pending.json"), _)) - .Times(AnyNumber()) - .WillRepeatedly(Return(make_ok())); - - ON_CALL(*m_configuration, convertedScoresPath()) - .WillByDefault(Return(io::path_t("/scores"))); - ON_CALL(*m_configuration, uniqueFileNameAddition(_, _, _)) - .WillByDefault(Return(std::string())); - - // [GIVEN] Writing the file always fails. MAX_FS_RETRY_ATTEMPTS (see convertfiletoscoreservice.h) - // is 5, so the 5th attempt should be the last one - const int maxAttempts = 5; - EXPECT_CALL(*m_fileSystem, writeFile(Truly([](const io::path_t& path) { return path.hasSuffix("mscz"); }), _)) - .Times(maxAttempts) - .WillRepeatedly(Return(make_ret(io::Err::FSWriteError))); - - bool received = false; - Ret receivedRet; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { - received = true; - receivedRet = ret; - }); - - // [WHEN] Uploading and polling the status, then letting the download complete - deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); - downloadProgress->finish(ProgressResult::make_ok(Val())); - - // [THEN] The conversion is reported as failed once the retries (via QTimer, hence the wait) are exhausted - waitUntil([&] { return received; }); - ASSERT_TRUE(received); - EXPECT_FALSE(receivedRet); -} - -TEST_F(Project_ConvertFileToScoreServiceTest, Download_MakePathFailsPermanently_FailsConversionWithoutWritingFile) -{ - // [GIVEN] The queue reports the conversion as done - ConvertQueueItem item; - item.id = TEST_QUEUE_ID; - item.type = ConvertType::Omr; - item.status = ConvertStatus::Done; - - ON_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .WillByDefault(Invoke([] { - SignedMsczUrl url; - url.url = QUrl("https://link.xyz/score.mscz"); - url.expiresInSeconds = 60; - return resolvedPromise >(RetVal::make_ok(url)); - })); - - auto downloadProgress = std::make_shared(); - ON_CALL(*m_convertService, downloadConvertedScore(_, _)) - .WillByDefault(Return(downloadProgress)); - - // [GIVEN] Creating the destination directory always fails. MAX_FS_RETRY_ATTEMPTS (see - // convertfiletoscoreservice.h) is 5, so the 5th attempt should be the last one - const int maxAttempts = 5; - EXPECT_CALL(*m_fileSystem, makePath(_)) - .Times(maxAttempts) - .WillRepeatedly(Return(make_ret(io::Err::FSMakingError))); - - // [GIVEN] saveWatchedItems() persists to its own file, unrelated to the converted score itself - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); - EXPECT_CALL(*m_fileSystem, writeFile(io::path_t("/pending.json"), _)) - .Times(AnyNumber()) - .WillRepeatedly(Return(make_ok())); - - ON_CALL(*m_configuration, convertedScoresPath()) - .WillByDefault(Return(io::path_t("/scores"))); - - // [THEN] The file is never written, since the directory could never be created - EXPECT_CALL(*m_fileSystem, writeFile(Truly([](const io::path_t& path) { return path.hasSuffix("mscz"); }), _)).Times(0); - - bool received = false; - Ret receivedRet; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { - received = true; - receivedRet = ret; - }); - - // [WHEN] Uploading and polling the status, then letting the download complete - deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); - downloadProgress->finish(ProgressResult::make_ok(Val())); - - // [THEN] The conversion is reported as failed once the retries (via QTimer, hence the wait) are exhausted - waitUntil([&] { return received; }); - ASSERT_TRUE(received); - EXPECT_FALSE(receivedRet); -} - -TEST_F(Project_ConvertFileToScoreServiceTest, Poll_AwaitingReview_DownloadsButDoesNotRequestReviewYet) -{ - // [GIVEN] The queue reports the conversion as awaiting review + // [GIVEN] The queue reports the conversion as awaiting review, but hasn't assigned a scoreId yet ConvertQueueItem item; item.id = TEST_QUEUE_ID; item.type = ConvertType::Omr; item.status = ConvertStatus::AwaitingReview; - - // [THEN] The score is downloaded even though the rating hasn't been submitted yet - EXPECT_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .Times(1) - .WillOnce(Invoke([] { - return pendingPromise >(); - })); + item.scoreId = std::nullopt; // no scoreId bool reviewRequested = false; - m_service->reviewRequested().onReceive(nullptr, [&](ConvertType, int, const io::path_t&) { + bool convertFinished = false; + m_service->reviewRequested().onReceive(nullptr, [&](int) { reviewRequested = true; }); + m_service->convertFinished().onReceive(nullptr, [&](const Ret&, const WatchedScore&) { + convertFinished = true; + }); // [WHEN] Uploading and polling the status deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); - // [THEN] No review is requested yet - the score hasn't finished downloading + // [THEN] Neither signal fires yet EXPECT_FALSE(reviewRequested); + EXPECT_FALSE(convertFinished); } -TEST_F(Project_ConvertFileToScoreServiceTest, Poll_AwaitingReview_EmitsReviewRequestedOnlyAfterDownloadCompletes) +TEST_F(Project_ConvertFileToScoreServiceTest, Poll_AwaitingReviewWithScoreId_EmitsReviewRequestedAndConvertFinished) { - // [GIVEN] The queue reports the conversion as awaiting review + // [GIVEN] The queue reports the conversion as awaiting review, with its scoreId ConvertQueueItem item; item.id = TEST_QUEUE_ID; item.type = ConvertType::Omr; item.status = ConvertStatus::AwaitingReview; - - ON_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .WillByDefault(Invoke([] { - SignedMsczUrl url; - url.url = QUrl("https://link.xyz/score.mscz"); - url.expiresInSeconds = 60; - return resolvedPromise >(RetVal::make_ok(url)); - })); - - auto downloadProgress = std::make_shared(); - ON_CALL(*m_convertService, downloadConvertedScore(_, _)) - .WillByDefault(Return(downloadProgress)); - - ON_CALL(*m_fileSystem, makePath(_)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_fileSystem, writeFile(_, _)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_configuration, convertedScoresPath()) - .WillByDefault(Return(io::path_t("/scores"))); - ON_CALL(*m_configuration, uniqueFileNameAddition(_, _, _)) - .WillByDefault(Return(std::string())); + item.scoreId = 555; bool reviewRequested = false; - ConvertType reviewType = ConvertType::Audio2Score; - int reviewQueueId = 0; - io::path_t reviewPath; - m_service->reviewRequested().onReceive(nullptr, [&](ConvertType type, int queueId, const io::path_t& path) { + int reviewScoreId = 0; + m_service->reviewRequested().onReceive(nullptr, [&](int scoreId) { reviewRequested = true; - reviewType = type; - reviewQueueId = queueId; - reviewPath = path; + reviewScoreId = scoreId; }); - // [WHEN] Uploading and polling the status, then letting the download complete + bool convertFinished = false; + Ret convertFinishedRet; + WatchedScore convertFinishedWatched; + m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const WatchedScore& watched) { + convertFinished = true; + convertFinishedRet = ret; + convertFinishedWatched = watched; + }); + + // [WHEN] Uploading and polling the status deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); - downloadProgress->finish(ProgressResult::make_ok(Val())); - // [THEN] The review is now requested, carrying the downloaded score's path + // [THEN] The score is already usable, so both signals fire immediately + ASSERT_TRUE(convertFinished); + EXPECT_TRUE(convertFinishedRet); + ASSERT_TRUE(convertFinishedWatched.scoreId.has_value()); + EXPECT_EQ(*convertFinishedWatched.scoreId, 555); + EXPECT_EQ(convertFinishedWatched.name, u"My Score"); + ASSERT_TRUE(reviewRequested); - EXPECT_EQ(reviewType, ConvertType::Omr); - EXPECT_EQ(reviewQueueId, TEST_QUEUE_ID); - EXPECT_FALSE(reviewPath.empty()); + EXPECT_EQ(reviewScoreId, 555); } -TEST_F(Project_ConvertFileToScoreServiceTest, Poll_ItemDroppedFromQueue_TreatedAsDoneAndDownloads) +TEST_F(Project_ConvertFileToScoreServiceTest, Poll_ItemNeverInQueueWithoutScoreId_SilentlyDropped) { - // [THEN] A watched item that disappears from the queue is treated the same as "Done" - EXPECT_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .Times(1) - .WillOnce(Invoke([] { - return pendingPromise >(); - })); + // [GIVEN] The item never appears in the queue at all, and never reported a scoreId - there's + // no way to identify a resulting score, so it's silently dropped rather than reported as failed + + bool received = false; + m_service->convertFinished().onReceive(nullptr, [&](const Ret&, const WatchedScore&) { + received = true; + }); // [WHEN] Uploading, then polling an empty queue deliverQueueStatus({}, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); + + // [THEN] Nothing is reported - not failed, not succeeded + EXPECT_FALSE(received); } -TEST_F(Project_ConvertFileToScoreServiceTest, Poll_ItemDroppedFromQueue_DownloadRetryableFailure_RetriesOnNextPoll) +TEST_F(Project_ConvertFileToScoreServiceTest, Poll_PreviouslyReportedItemDropsFromQueue_SilentlyErasedWithoutDuplicateReport) { - // [GIVEN] The item is never present in the queue (already finished server-side and removed - // before it was ever observed), while an unrelated item stays in the queue across both polls + // [GIVEN] The item was already reported ready (AwaitingReview, with its scoreId) on the first poll, + // then disappears from the queue entirely on the second poll const int otherQueueId = TEST_QUEUE_ID + 1; + + ConvertQueueItem awaitingItem; + awaitingItem.id = TEST_QUEUE_ID; + awaitingItem.type = ConvertType::Omr; + awaitingItem.status = ConvertStatus::AwaitingReview; + awaitingItem.scoreId = 555; + ConvertQueueItem otherItem; otherItem.id = otherQueueId; otherItem.type = ConvertType::Omr; otherItem.status = ConvertStatus::Processing; - ON_CALL(*m_convertService, fetchQueue()) - .WillByDefault(Invoke([otherItem] { + EXPECT_CALL(*m_convertService, fetchQueue()) + .Times(2) + .WillOnce(Invoke([awaitingItem] { + return resolvedPromise >(RetVal::make_ok(ConvertQueueList { awaitingItem })); + })) + .WillOnce(Invoke([otherItem] { + //! NOTE: awaitingItem has now dropped out of the queue entirely return resolvedPromise >(RetVal::make_ok(ConvertQueueList { otherItem })); })); - SignedMsczUrl url; - url.url = QUrl("https://link.xyz/score.mscz"); - url.expiresInSeconds = 60; - ON_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .WillByDefault(Invoke([url] { - return resolvedPromise >(RetVal::make_ok(url)); - })); - - // [GIVEN] The actual download fails with a transient error the first time, succeeds the second - auto failingDownload = std::make_shared(); - auto succeedingDownload = std::make_shared(); - EXPECT_CALL(*m_convertService, downloadConvertedScore(_, _)) - .Times(2) - .WillOnce(Return(failingDownload)) - .WillOnce(Return(succeedingDownload)); - - ON_CALL(*m_fileSystem, makePath(_)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_fileSystem, writeFile(_, _)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_configuration, convertedScoresPath()) - .WillByDefault(Return(io::path_t("/scores"))); - ON_CALL(*m_configuration, uniqueFileNameAddition(_, _, _)) - .WillByDefault(Return(std::string())); - - bool received = false; - Ret receivedRet; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { - received = true; - receivedRet = ret; + int convertFinishedCount = 0; + m_service->convertFinished().onReceive(nullptr, [&](const Ret&, const WatchedScore&) { + ++convertFinishedCount; }); - // [WHEN] Starting the conversion - the first poll already reports an empty queue for it, - // so it's immediately treated as done and its download starts, but fails transiently + // [WHEN] Starting the conversion - the first poll reports it as awaiting review, already reporting it once uploadAndResolve(TEST_QUEUE_ID, "My Score", { "/some/path/a.pdf" }); - failingDownload->finish(make_ret(muse::network::Err::NetworkError)); - EXPECT_FALSE(received); + EXPECT_EQ(convertFinishedCount, 1); - // [WHEN] Starting an unrelated conversion triggers a second poll; the original item is - // still absent from the queue, but must still be retried rather than forgotten + // [WHEN] Starting an unrelated conversion triggers a second poll; the original item has now dropped uploadAndResolve(otherQueueId, "Other Score", { "/some/path/b.pdf" }); - succeedingDownload->finish(ProgressResult::make_ok(Val())); - // [THEN] The retried download succeeds and the conversion finishes - ASSERT_TRUE(received); - EXPECT_TRUE(receivedRet); + // [THEN] No duplicate report + EXPECT_EQ(convertFinishedCount, 1); } TEST_F(Project_ConvertFileToScoreServiceTest, Poll_SameIdDifferentType_DoesNotCrossMatch) @@ -1261,62 +1044,67 @@ TEST_F(Project_ConvertFileToScoreServiceTest, Poll_SameIdDifferentType_DoesNotCr JsonObject omrObj; omrObj["id"] = TEST_QUEUE_ID; omrObj["type"] = int(ConvertType::Omr); - omrObj["convertedFileName"] = "Omr Score"; + omrObj["startedLocally"] = true; + omrObj["convertedScoreName"] = "Omr Score"; JsonObject audioObj; audioObj["id"] = TEST_QUEUE_ID; audioObj["type"] = int(ConvertType::Audio2Score); - audioObj["convertedFileName"] = "Audio Score"; + audioObj["startedLocally"] = true; + audioObj["convertedScoreName"] = "Audio Score"; JsonArray array; array << omrObj << audioObj; JsonDocument json(array); - ON_CALL(*m_configuration, pendingConvertsJsonPath()) - .WillByDefault(Return(io::path_t("/pending.json"))); - ON_CALL(*m_fileSystem, readFile(io::path_t("/pending.json"))) + ON_CALL(*m_configuration, watchedConvertsJsonPath()) + .WillByDefault(Return(io::path_t("/watched.json"))); + ON_CALL(*m_fileSystem, readFile(io::path_t("/watched.json"))) .WillByDefault(Return(RetVal::make_ok(json.toJson()))); ON_CALL(*m_fileSystem, writeFile(_, _)) .WillByDefault(Return(make_ok())); - // [GIVEN] The queue reports the Omr item as failed; the Audio2Score item has already - // dropped out of the queue (finished) and must not be mistaken for the failed Omr one + // [GIVEN] The queue reports the Omr item as failed, and the Audio2Score item as done; they + // must not be mistaken for each other just because they share the same numeric id ConvertQueueItem failedOmrItem; failedOmrItem.id = TEST_QUEUE_ID; failedOmrItem.type = ConvertType::Omr; failedOmrItem.status = ConvertStatus::Failed; failedOmrItem.errorCode = ConvertErrorCode::FileTooLarge; + failedOmrItem.filename = "Omr Score"; - ON_CALL(*m_convertService, fetchQueue()) - .WillByDefault(Invoke([failedOmrItem] { - return resolvedPromise >(RetVal::make_ok(ConvertQueueList { failedOmrItem })); - })); + ConvertQueueItem doneAudioItem; + doneAudioItem.id = TEST_QUEUE_ID; + doneAudioItem.type = ConvertType::Audio2Score; + doneAudioItem.status = ConvertStatus::Done; + doneAudioItem.scoreId = 999; + doneAudioItem.filename = "Audio Score"; - // [THEN] Only the Audio2Score item's URL is fetched (correctly treated as done, dropped - // from the queue); the failed Omr item is never mistaken for it, or vice versa - EXPECT_CALL(*m_convertService, fetchMsczUrl(ConvertType::Audio2Score, TEST_QUEUE_ID)) - .Times(1) - .WillOnce(Invoke([] { - return pendingPromise >(); + ON_CALL(*m_convertService, fetchQueue()) + .WillByDefault(Invoke([failedOmrItem, doneAudioItem] { + return resolvedPromise >(RetVal::make_ok(ConvertQueueList { failedOmrItem, + doneAudioItem })); })); - EXPECT_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .Times(0); - Ret receivedRet; - int receivedCount = 0; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { - receivedRet = ret; - ++receivedCount; + std::vector receivedRets; + m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const WatchedScore&) { + receivedRets.push_back(ret); }); // [WHEN] Resuming loads both items and triggers a poll m_service->resumeConvert(); pumpEvents(); - // [THEN] Exactly one failure is reported, and it's for the Omr file, not the Audio2Score one - ASSERT_EQ(receivedCount, 1); - EXPECT_FALSE(receivedRet); - EXPECT_EQ(receivedRet.data(CONVERT_FAILED_FILE_NAME_KEY, String()), u"Omr Score"); + // [THEN] Exactly one failure (the Omr one) and one success (the Audio2Score one) are reported - + // if type were ignored during matching, the two items could be mixed up with each other + ASSERT_EQ(receivedRets.size(), 2u); + + const auto failureIt = std::find_if(receivedRets.begin(), receivedRets.end(), [](const Ret& ret) { return !ret; }); + ASSERT_NE(failureIt, receivedRets.end()); + EXPECT_EQ(failureIt->data(CONVERT_FAILED_FILE_NAME_KEY, String()), u"Omr Score"); + + const auto successIt = std::find_if(receivedRets.begin(), receivedRets.end(), [](const Ret& ret) { return bool(ret); }); + ASSERT_NE(successIt, receivedRets.end()); } TEST_F(Project_ConvertFileToScoreServiceTest, Poll_FailedStatus_ForwardsProcessingFailure) @@ -1327,10 +1115,11 @@ TEST_F(Project_ConvertFileToScoreServiceTest, Poll_FailedStatus_ForwardsProcessi item.type = ConvertType::Omr; item.status = ConvertStatus::Failed; item.errorCode = ConvertErrorCode::FileTooLarge; + item.filename = "My Score"; bool received = false; Ret receivedRet; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { + m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const WatchedScore&) { received = true; receivedRet = ret; }); @@ -1345,43 +1134,8 @@ TEST_F(Project_ConvertFileToScoreServiceTest, Poll_FailedStatus_ForwardsProcessi EXPECT_EQ(receivedRet.data(CONVERT_FAILED_FILE_NAME_KEY, String()), u"My Score"); } -TEST_F(Project_ConvertFileToScoreServiceTest, Poll_ExpiredDownloadLink_ForwardsFailure) -{ - // [GIVEN] The score is done, but its download link has already expired - ConvertQueueItem item; - item.id = TEST_QUEUE_ID; - item.type = ConvertType::Omr; - item.status = ConvertStatus::Done; - - ON_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .WillByDefault(Invoke([] { - SignedMsczUrl url; - url.url = QUrl("https://link.xyz/score.mscz"); - url.expiresInSeconds = 0; - return resolvedPromise >(RetVal::make_ok(url)); - })); - - // [THEN] No download is attempted - EXPECT_CALL(*m_convertService, downloadConvertedScore(_, _)).Times(0); - - bool received = false; - Ret receivedRet; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { - received = true; - receivedRet = ret; - }); - - // [WHEN] Uploading and polling the status - deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); - - // [THEN] The failure is forwarded as an expired download link - ASSERT_TRUE(received); - EXPECT_FALSE(receivedRet); - EXPECT_EQ(receivedRet.code(), int(mu::project::Err::DownloadLinkExpired)); -} - // ================================================== -// retry logic (poll / download failures) +// retry logic (poll failures) // ================================================== TEST_F(Project_ConvertFileToScoreServiceTest, Poll_NonRetryableFetchFailure_FinishesImmediatelyWithError) @@ -1411,13 +1165,14 @@ TEST_F(Project_ConvertFileToScoreServiceTest, Poll_NonRetryableFetchFailure_Fini TEST_F(Project_ConvertFileToScoreServiceTest, Poll_RetryableFetchFailure_KeepsWatchingItemForNextPoll) { // [GIVEN] The first status check fails with a transient network error; - // the second succeeds, reporting the originally watched item as awaiting review + // the second succeeds, reporting the originally watched item as done const int otherQueueId = TEST_QUEUE_ID + 1; ConvertQueueItem watchedItem; watchedItem.id = TEST_QUEUE_ID; watchedItem.type = ConvertType::Omr; - watchedItem.status = ConvertStatus::AwaitingReview; + watchedItem.status = ConvertStatus::Done; + watchedItem.scoreId = 555; ConvertQueueItem otherItem; otherItem.id = otherQueueId; @@ -1435,15 +1190,8 @@ TEST_F(Project_ConvertFileToScoreServiceTest, Poll_RetryableFetchFailure_KeepsWa otherItem })); })); - // [THEN] The originally watched item's download is attempted once the retried poll processes it - EXPECT_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .Times(1) - .WillOnce(Invoke([] { - return pendingPromise >(); - })); - bool received = false; - m_service->convertFinished().onReceive(nullptr, [&](const Ret&, const io::path_t&) { + m_service->convertFinished().onReceive(nullptr, [&](const Ret&, const WatchedScore&) { received = true; }); @@ -1453,8 +1201,11 @@ TEST_F(Project_ConvertFileToScoreServiceTest, Poll_RetryableFetchFailure_KeepsWa // [THEN] Nothing is finished yet, the item was not dropped from the watch list EXPECT_FALSE(received); - // [AND WHEN] Starting an unrelated conversion triggers a second poll, batching the original item in again + // [AND WHEN] Starting an unrelated conversion triggers a second poll, checking the original item again too uploadAndResolve(otherQueueId, "Other Score", { "/some/path/b.pdf" }); + + // [THEN] The item survived the transient failure and was processed once the queue succeeded + EXPECT_TRUE(received); } TEST_F(Project_ConvertFileToScoreServiceTest, Poll_ConsecutiveRetryableFetchFailures_GivesUpAfterMaxAttempts) @@ -1569,7 +1320,7 @@ TEST_F(Project_ConvertFileToScoreServiceTest, RetryPolling_NoPendingItems_DoesNo TEST_F(Project_ConvertFileToScoreServiceTest, Poll_SuccessBetweenFetchFailures_ResetsConsecutiveFailureCount) { - // [GIVEN] A pattern of failures with an intervening success: 3 failures, then a success, then + // [GIVEN] A pattern of failures with a success in between: 3 failures, then a success, then // 4 more failures - never 5 CONSECUTIVE failures, so polling should never give up ConvertQueueItem processingItem; processingItem.id = TEST_QUEUE_ID; @@ -1610,107 +1361,123 @@ TEST_F(Project_ConvertFileToScoreServiceTest, Poll_SuccessBetweenFetchFailures_R EXPECT_FALSE(gaveUp); } -TEST_F(Project_ConvertFileToScoreServiceTest, DownloadScore_RetryableFailure_RetriesAndSucceedsOnNextPoll) -{ - // [GIVEN] The queue reports the conversion as done on both polls, and fetching the download URL always succeeds - ConvertQueueItem doneItem; - doneItem.id = TEST_QUEUE_ID; - doneItem.type = ConvertType::Omr; - doneItem.status = ConvertStatus::Done; - - const int otherQueueId = TEST_QUEUE_ID + 1; - ConvertQueueItem otherItem; - otherItem.id = otherQueueId; - otherItem.type = ConvertType::Omr; - otherItem.status = ConvertStatus::Processing; - - ON_CALL(*m_convertService, fetchQueue()) - .WillByDefault(Invoke([doneItem, otherItem] { - return resolvedPromise >(RetVal::make_ok(ConvertQueueList { doneItem, otherItem })); - })); - - SignedMsczUrl url; - url.url = QUrl("https://link.xyz/score.mscz"); - url.expiresInSeconds = 60; - ON_CALL(*m_convertService, fetchMsczUrl(ConvertType::Omr, TEST_QUEUE_ID)) - .WillByDefault(Invoke([url] { - return resolvedPromise >(RetVal::make_ok(url)); - })); - - // [AND GIVEN] The actual download fails with a transient error the first time, succeeds the second - auto failingDownload = std::make_shared(); - auto succeedingDownload = std::make_shared(); - EXPECT_CALL(*m_convertService, downloadConvertedScore(_, _)) - .Times(2) - .WillOnce(Return(failingDownload)) - .WillOnce(Return(succeedingDownload)); - - ON_CALL(*m_fileSystem, makePath(_)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_fileSystem, writeFile(_, _)) - .WillByDefault(Return(make_ok())); - ON_CALL(*m_configuration, convertedScoresPath()) - .WillByDefault(Return(io::path_t("/scores"))); - ON_CALL(*m_configuration, uniqueFileNameAddition(_, _, _)) - .WillByDefault(Return(std::string())); - - bool received = false; - Ret receivedRet; - m_service->convertFinished().onReceive(nullptr, [&](const Ret& ret, const io::path_t&) { - received = true; - receivedRet = ret; - }); - - // [WHEN] Starting the conversion - the first poll's download fails partway through - uploadAndResolve(TEST_QUEUE_ID, "My Score", { "/some/path/a.pdf" }); - failingDownload->finish(make_ret(muse::network::Err::NetworkError)); - EXPECT_FALSE(received); - - // [AND WHEN] Starting an unrelated conversion triggers a second poll, retrying the download - uploadAndResolve(otherQueueId, "Other Score", { "/some/path/b.pdf" }); - succeedingDownload->finish(ProgressResult::make_ok(Val())); - - // [THEN] The retried download succeeds and the conversion finishes - ASSERT_TRUE(received); - EXPECT_TRUE(receivedRet); -} - // ================================================== // submitReview() / submitReviewComment() // ================================================== TEST_F(Project_ConvertFileToScoreServiceTest, SubmitReview_Good_DelegatesToConvertService) { - // [THEN] The rating is delegated to the convert service + // [GIVEN] A watched item already reported ready and awaiting review, identified by its scoreId + ConvertQueueItem item; + item.id = TEST_QUEUE_ID; + item.type = ConvertType::Omr; + item.status = ConvertStatus::AwaitingReview; + item.scoreId = 555; + + deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); + + // [THEN] The rating is delegated to the convert service, resolving the scoreId back to its conversion EXPECT_CALL(*m_convertService, submitReview(ConvertType::Omr, TEST_QUEUE_ID, ReviewRating::Good, QString())) .WillOnce(Invoke([](auto, auto, auto, auto) { return resolvedPromise >(RetVal::make_ok(ConvertResult {})); })); // [WHEN] Submitting a "Good" review with no comment - m_service->submitReview(ConvertType::Omr, TEST_QUEUE_ID, ReviewRating::Good); + m_service->submitReview(555, ReviewRating::Good); } TEST_F(Project_ConvertFileToScoreServiceTest, SubmitReview_BadWithComment_DelegatesToConvertService) { - // [THEN] The rating and comment are delegated to the convert service + // [GIVEN] A watched item already reported ready and awaiting review, identified by its scoreId + ConvertQueueItem item; + item.id = 7; + item.type = ConvertType::Audio2Score; + item.status = ConvertStatus::AwaitingReview; + item.scoreId = 555; + + deliverQueueStatus({ item }, ConvertType::Audio2Score, 7, "My Score"); + + // [THEN] The rating and comment are delegated to the convert service, resolving the scoreId back to its conversion EXPECT_CALL(*m_convertService, submitReview(ConvertType::Audio2Score, 7, ReviewRating::Bad, QString("Too many wrong notes"))) .WillOnce(Invoke([](auto, auto, auto, auto) { return resolvedPromise >(RetVal::make_ok(ConvertResult {})); })); // [WHEN] Submitting a "Bad" review with a comment - m_service->submitReview(ConvertType::Audio2Score, 7, ReviewRating::Bad, "Too many wrong notes"); + m_service->submitReview(555, ReviewRating::Bad, "Too many wrong notes"); } TEST_F(Project_ConvertFileToScoreServiceTest, SubmitReviewComment_DelegatesToConvertService) { - // [THEN] The comment is delegated to the convert service + // [GIVEN] A watched item already reported ready and awaiting review, identified by its scoreId + ConvertQueueItem item; + item.id = 7; + item.type = ConvertType::Audio2Score; + item.status = ConvertStatus::AwaitingReview; + item.scoreId = 555; + + deliverQueueStatus({ item }, ConvertType::Audio2Score, 7, "My Score"); + + // [THEN] The comment is delegated to the convert service, resolving the scoreId back to its conversion EXPECT_CALL(*m_convertService, submitReviewComment(ConvertType::Audio2Score, 7, QString("Great job"))) .WillOnce(Invoke([](auto, auto, auto) { return resolvedPromise >(RetVal::make_ok(ConvertResult {})); })); // [WHEN] Submitting a follow-up comment - m_service->submitReviewComment(ConvertType::Audio2Score, 7, "Great job"); + m_service->submitReviewComment(555, "Great job"); +} + +// ================================================== +// deleteConversion() +// ================================================== + +TEST_F(Project_ConvertFileToScoreServiceTest, DeleteConversion_Success_RemovesFromWatchedScores) +{ + // [GIVEN] A watched, still-processing conversion + ConvertQueueItem item; + item.id = TEST_QUEUE_ID; + item.type = ConvertType::Omr; + item.status = ConvertStatus::Processing; + + deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); + ASSERT_EQ(m_service->watchedScores().val.size(), 1u); + + // [THEN] The deletion is delegated to the convert service + EXPECT_CALL(*m_convertService, deleteConversion(ConvertType::Omr, TEST_QUEUE_ID)) + .WillOnce(Invoke([](auto, auto) { + return resolvedPromise(make_ok()); + })); + + // [WHEN] Deleting the conversion + m_service->deleteConversion(ConvertType::Omr, TEST_QUEUE_ID); + pumpEvents(); + + // [THEN] It is no longer watched + EXPECT_TRUE(m_service->watchedScores().val.empty()); +} + +TEST_F(Project_ConvertFileToScoreServiceTest, DeleteConversion_Fails_KeepsWatching) +{ + // [GIVEN] A watched, still-processing conversion + ConvertQueueItem item; + item.id = TEST_QUEUE_ID; + item.type = ConvertType::Omr; + item.status = ConvertStatus::Processing; + + deliverQueueStatus({ item }, ConvertType::Omr, TEST_QUEUE_ID, "My Score"); + ASSERT_EQ(m_service->watchedScores().val.size(), 1u); + + // [THEN] The deletion is delegated to the convert service, but fails + EXPECT_CALL(*m_convertService, deleteConversion(ConvertType::Omr, TEST_QUEUE_ID)) + .WillOnce(Invoke([](auto, auto) { + return resolvedPromise(make_ret(muse::cloud::Err::UnknownError)); + })); + + // [WHEN] Deleting the conversion + m_service->deleteConversion(ConvertType::Omr, TEST_QUEUE_ID); + pumpEvents(); + + // [THEN] It is still watched + EXPECT_EQ(m_service->watchedScores().val.size(), 1u); } diff --git a/src/project/tests/mocks/convertfiletoscoreservicemock.h b/src/project/tests/mocks/convertfiletoscoreservicemock.h index 11509d96322c7..225f26fb1c10c 100644 --- a/src/project/tests/mocks/convertfiletoscoreservicemock.h +++ b/src/project/tests/mocks/convertfiletoscoreservicemock.h @@ -36,16 +36,17 @@ class ConvertFileToScoreServiceMock : public IConvertFileToScoreService MOCK_METHOD(muse::Ret, validateLink, (const QUrl&), (const, override)); MOCK_METHOD(muse::Ret, startConvert, (const ConvertInput&, const muse::String&), (override)); - MOCK_METHOD((muse::async::Channel), convertFinished, (), (const, override)); + MOCK_METHOD((muse::async::Channel), convertFinished, (), (const, override)); - MOCK_METHOD(muse::StringList, fileNamesBeingConverted, (), (const, override)); - MOCK_METHOD(muse::async::Notification, fileNamesBeingConvertedChanged, (), (const, override)); + MOCK_METHOD(muse::ValNt, watchedScores, (), (const, override)); MOCK_METHOD((muse::async::Channel), pollingFailed, (), (const, override)); MOCK_METHOD(void, retryPolling, (), (override)); - MOCK_METHOD((muse::async::Channel), reviewRequested, (), (const, override)); - MOCK_METHOD(void, submitReview, (ConvertType, int, ReviewRating, const QString&), (override)); - MOCK_METHOD(void, submitReviewComment, (ConvertType, int, const QString&), (override)); + MOCK_METHOD((muse::async::Channel), reviewRequested, (), (const, override)); + MOCK_METHOD(void, submitReview, (int, ReviewRating, const QString&), (override)); + MOCK_METHOD(void, submitReviewComment, (int, const QString&), (override)); + + MOCK_METHOD(void, deleteConversion, (ConvertType, int), (override)); }; } diff --git a/src/project/tests/mocks/projectconfigurationmock.h b/src/project/tests/mocks/projectconfigurationmock.h index 169795ebd9275..ca86d67fdf45a 100644 --- a/src/project/tests/mocks/projectconfigurationmock.h +++ b/src/project/tests/mocks/projectconfigurationmock.h @@ -154,11 +154,7 @@ class ProjectConfigurationMock : public project::IProjectConfiguration MOCK_METHOD(bool, showConvertFileProcessingDialog, (), (const, override)); MOCK_METHOD(void, setShowConvertFileProcessingDialog, (bool), (override)); - MOCK_METHOD(muse::io::path_t, convertedScoresPath, (), (const, override)); - MOCK_METHOD(muse::io::path_t, pendingConvertsJsonPath, (), (const, override)); - - MOCK_METHOD(std::string, uniqueFileNameAddition, (const muse::io::path_t&, const muse::io::path_t&, const std::string&), - (const, override)); + MOCK_METHOD(muse::io::path_t, watchedConvertsJsonPath, (), (const, override)); }; } diff --git a/src/project/types/converttypes.h b/src/project/types/converttypes.h index b049bf45b5108..76918f449a688 100644 --- a/src/project/types/converttypes.h +++ b/src/project/types/converttypes.h @@ -22,20 +22,67 @@ #pragma once -#include "cloud/musescorecom/converttypes.h" +#include +#include +#include + +#include #include "filecategory.h" -#include "types/ret.h" -#include "types/secs.h" +#include "cloud/musescorecom/converttypes.h" +#include "cloud/cloudtypes.h" + +#include "global/io/path.h" +#include "global/types/string.h" +#include "global/types/secs.h" +#include "global/types/ret.h" namespace mu::project { using ConvertConfig = muse::cloud::ConvertConfig; using ConvertType = muse::cloud::ConvertType; -using ConvertInput = muse::cloud::ConvertInput; +using ConvertStatus = muse::cloud::ConvertStatus; using ReviewRating = muse::cloud::ReviewRating; using LinkSource = muse::cloud::LinkSource; using LinkSources = muse::cloud::LinkSources; +using ScoreInfo = muse::cloud::ScoreInfo; +using ScoreConversionInfo = muse::cloud::ScoreConversionInfo; + +struct OmrConvertInput { + muse::io::paths_t paths; +}; + +struct Audio2ScoreConvertInput { + std::variant data; // paths or link +}; + +using ConvertInput = std::variant; + +inline ConvertType convertTypeOf(const ConvertInput& input) +{ + return std::holds_alternative(input) ? ConvertType::Omr : ConvertType::Audio2Score; +} + +inline muse::io::paths_t convertPathsOf(const ConvertInput& input) +{ + if (const OmrConvertInput* omr = std::get_if(&input)) { + return omr->paths; + } + + const muse::io::paths_t* paths = std::get_if(&std::get(input).data); + return paths ? *paths : muse::io::paths_t(); +} + +inline QUrl convertLinkOf(const ConvertInput& input) +{ + const Audio2ScoreConvertInput* a2s = std::get_if(&input); + if (!a2s) { + return QUrl(); + } + + const QUrl* link = std::get_if(&a2s->data); + return link ? *link : QUrl(); +} struct ConvertFilesValidation { ConvertType type = ConvertType::Omr; @@ -46,7 +93,23 @@ struct PollingFailure { muse::Ret ret; int attempt = 0; int maxAttempts = 0; - muse::secs_t nextInterval; + muse::secs_t nextInterval = 0.; bool gaveUp = false; }; + +struct WatchedScore { + ScoreConversionInfo conversion; + std::optional scoreId; //! set once the score is ready and reported (Done/AwaitingReview) + bool startedLocally = false; //! true if started in MuseScore + muse::String name; + + bool operator==(const WatchedScore& other) const + { + return conversion == other.conversion + && scoreId == other.scoreId + && startedLocally == other.startedLocally + && name == other.name; + } +}; +using WatchedScoreList = std::vector; } diff --git a/src/stubs/project/projectconfigurationstub.cpp b/src/stubs/project/projectconfigurationstub.cpp index c7494a2b55c3e..92092f9a09bcd 100644 --- a/src/stubs/project/projectconfigurationstub.cpp +++ b/src/stubs/project/projectconfigurationstub.cpp @@ -419,17 +419,7 @@ void ProjectConfigurationStub::setShowConvertFileProcessingDialog(bool) { } -muse::io::path_t ProjectConfigurationStub::convertedScoresPath() const +muse::io::path_t ProjectConfigurationStub::watchedConvertsJsonPath() const { return muse::io::path_t(); } - -muse::io::path_t ProjectConfigurationStub::pendingConvertsJsonPath() const -{ - return muse::io::path_t(); -} - -std::string ProjectConfigurationStub::uniqueFileNameAddition(const muse::io::path_t&, const muse::io::path_t&, const std::string&) const -{ - return std::string(); -} diff --git a/src/stubs/project/projectconfigurationstub.h b/src/stubs/project/projectconfigurationstub.h index c926d257476b6..e5c27b409294d 100644 --- a/src/stubs/project/projectconfigurationstub.h +++ b/src/stubs/project/projectconfigurationstub.h @@ -154,10 +154,6 @@ class ProjectConfigurationStub : public IProjectConfiguration bool showConvertFileProcessingDialog() const override; void setShowConvertFileProcessingDialog(bool show) override; - muse::io::path_t convertedScoresPath() const override; - muse::io::path_t pendingConvertsJsonPath() const override; - - std::string uniqueFileNameAddition(const muse::io::path_t& filename, const muse::io::path_t& folderPath, - const std::string& suffix = std::string()) const override; + muse::io::path_t watchedConvertsJsonPath() const override; }; }