diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index b67055d1..0d730c78 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -26,18 +26,18 @@ jobs: - name: Checkout uses: actions/checkout@v3 - - name: Install Packages (cpp) - if: ${{ matrix.language == 'cpp' }} + - if: matrix.language == 'cpp' + name: Install dependencies (cpp) run: | sudo apt-get update sudo apt-get install --yes build-essential cmake libboost-all-dev libconfig++-dev libavcodec-dev libavutil-dev libavformat-dev libstb-dev libtag1-dev libpam0g-dev libgtest-dev - export WT_VERSION=4.7.2 + export WT_VERSION=4.9.0 export WT_INSTALL_PREFIX=/usr - git clone https://github.com/emweb/wt.git wt - pushd wt + git clone https://github.com/emweb/wt.git /tmp/wt + pushd /tmp/wt git checkout ${WT_VERSION} cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=${WT_INSTALL_PREFIX} -DBUILD_EXAMPLES=OFF -DENABLE_LIBWTTEST=OFF -DCONNECTOR_FCGI=OFF - make + make -j$(nproc) sudo make install popd @@ -47,9 +47,18 @@ jobs: languages: ${{ matrix.language }} queries: +security-and-quality - - name: Autobuild + - if: matrix.language == 'javascript' + name: Autobuild uses: github/codeql-action/autobuild@v2 + - if: matrix.language == 'cpp' + name: Build + run: | + mkdir -p build + cd build + cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Release .. + make -j$(nproc) + - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v2 with: diff --git a/.lgtm.yml b/.lgtm.yml deleted file mode 100644 index 11ce21a0..00000000 --- a/.lgtm.yml +++ /dev/null @@ -1,34 +0,0 @@ -path_classifiers: - test: - - src/test - -extraction: - cpp: - prepare: - packages: - - build-essential - - cmake - - libboost-all-dev - - libconfig++-dev - - libavcodec-dev - - libavutil-dev - - libavformat-dev - - libstb-dev - - libtag1-dev - - libpam0g-dev - after_prepare: - - export WT_VERSION=4.7.2 - - export WT_INSTALL_PREFIX=${LGTM_WORKSPACE}/wt-${WT_VERSION} - - pushd ${LGTM_WORKSPACE} - - git clone https://github.com/emweb/wt.git ${LGTM_WORKSPACE}/wt - - pushd ${LGTM_WORKSPACE}/wt - - git checkout ${WT_VERSION} - - cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=${WT_INSTALL_PREFIX} -DBUILD_EXAMPLES=OFF -DENABLE_LIBWTTEST=OFF -DCONNECTOR_FCGI=OFF - - make install - - popd - configure: - command: - - export WT_VERSION=4.7.2 - - export WT_INSTALL_PREFIX=${LGTM_WORKSPACE}/wt-${WT_VERSION} - - cmake -DCMAKE_PREFIX_PATH=${WT_INSTALL_PREFIX} -DCMAKE_BUILD_TYPE=Release - diff --git a/README.md b/README.md index c0fabf8a..82a56945 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # LMS - Lightweight Music Server -[![Last Release](https://img.shields.io/github/v/release/epoupon/lms?logo=github&label=latest)](https://github.com/epoupon/lms/releases) [![Build](https://img.shields.io/github/workflow/status/epoupon/lms/Build?logo=github)](https://github.com/epoupon/lms/actions) +[![Last Release](https://img.shields.io/github/v/release/epoupon/lms?logo=github&label=latest)](https://github.com/epoupon/lms/releases) _LMS_ is a self-hosted music streaming software: access your music collection from anywhere using a web interface! diff --git a/approot/mediaplayer.xml b/approot/mediaplayer.xml index 3494fa2b..f37292f4 100644 --- a/approot/mediaplayer.xml +++ b/approot/mediaplayer.xml @@ -44,6 +44,10 @@ - + + +
{1}
+ + diff --git a/approot/messages.xml b/approot/messages.xml index 241caf7a..1bbe9621 100644 --- a/approot/messages.xml +++ b/approot/messages.xml @@ -209,10 +209,6 @@ Clear Create new playlist - - Added {1} track - Added {1} tracks - Play Queue Play Queue full! Radio mode diff --git a/approot/messages_fr.xml b/approot/messages_fr.xml index 0439a9e5..48d8c295 100644 --- a/approot/messages_fr.xml +++ b/approot/messages_fr.xml @@ -209,10 +209,6 @@ Effacer Créer une nouvelle liste de lecture - - {1} piste ajoutée - {1} pistes ajoutées - Liste de lecture Liste de lecture pleine! Mode radio diff --git a/approot/messages_it.xml b/approot/messages_it.xml index 57d57209..3ed491c8 100644 --- a/approot/messages_it.xml +++ b/approot/messages_it.xml @@ -209,10 +209,6 @@ Cancella - - Aggiunta {1} traccia - Aggiunte {1} tracce - Coda di riproduzione Coda di riproduzione piena! Modalità radio diff --git a/docroot/js/mediaplayer.js b/docroot/js/mediaplayer.js index 279554c2..ea174380 100644 --- a/docroot/js/mediaplayer.js +++ b/docroot/js/mediaplayer.js @@ -114,15 +114,15 @@ LMS.mediaplayer = function () { } let _durationToString = function (duration) { - let minutes = parseInt(duration / 60, 10); - let seconds = parseInt(duration, 10) % 60; - - let res = ""; - - res += minutes + ":"; - res += (seconds < 10 ? "0" + seconds : seconds); - - return res; + const seconds = parseInt(duration, 10); + const h = Math.floor(seconds / 3600); + const m = Math.floor((seconds % 3600) / 60); + const s = Math.round(seconds % 60); + return [ + h, + m > 9 ? m : (h ? '0' + m : m || '0'), + s > 9 ? s : '0' + s + ].filter(Boolean).join(':'); } let _playTrack = function() { diff --git a/src/libs/av/impl/TranscodeResourceHandler.cpp b/src/libs/av/impl/TranscodeResourceHandler.cpp index 0f2687d3..5e85b72e 100644 --- a/src/libs/av/impl/TranscodeResourceHandler.cpp +++ b/src/libs/av/impl/TranscodeResourceHandler.cpp @@ -46,6 +46,8 @@ namespace Av { if (_estimatedContentLength) LMS_LOG(TRANSCODE, DEBUG) << "Estimated content length = " << *_estimatedContentLength; + else + LMS_LOG(TRANSCODE, DEBUG) << "Not using estimated content length"; } Wt::Http::ResponseContinuation* @@ -58,8 +60,8 @@ namespace Av if (_bytesReadyCount > 0) { response.out().write(reinterpret_cast(&_buffer[0]), _bytesReadyCount); - _bytesReadyCount = 0; _totalServedByteCount += _bytesReadyCount; + _bytesReadyCount = 0; } if (!_transcoder.finished()) diff --git a/src/libs/av/impl/Transcoder.cpp b/src/libs/av/impl/Transcoder.cpp index 4b828d5a..5519021d 100644 --- a/src/libs/av/impl/Transcoder.cpp +++ b/src/libs/av/impl/Transcoder.cpp @@ -178,19 +178,6 @@ Transcoder::start() } } -void -Transcoder::asyncWaitForData(WaitCallback cb) -{ - assert(_childProcess); - - LOG(DEBUG) << "Want to wait for data"; - - _childProcess->asyncWaitForData([cb = std::move(cb)] - { - cb(); - }); -} - void Transcoder::asyncRead(std::byte* buffer, std::size_t bufferSize, ReadCallback readCallback) { diff --git a/src/libs/av/impl/Transcoder.hpp b/src/libs/av/impl/Transcoder.hpp index a4de2976..0f6cf4c7 100644 --- a/src/libs/av/impl/Transcoder.hpp +++ b/src/libs/av/impl/Transcoder.hpp @@ -40,9 +40,6 @@ namespace Av Transcoder(Transcoder&&) = delete; Transcoder& operator=(Transcoder&&) = delete; - using WaitCallback = std::function; - void asyncWaitForData(WaitCallback cb); - // non blocking calls using ReadCallback = std::function; void asyncRead(std::byte* buffer, std::size_t bufferSize, ReadCallback); diff --git a/src/libs/metadata/impl/TagLibParser.cpp b/src/libs/metadata/impl/TagLibParser.cpp index d2e21215..1227c11d 100644 --- a/src/libs/metadata/impl/TagLibParser.cpp +++ b/src/libs/metadata/impl/TagLibParser.cpp @@ -140,6 +140,7 @@ getPerformerArtists(const TagLib::PropertyMap& properties, PerformerContainer performers; // picard stores like this: (see https://picard-docs.musicbrainz.org/en/appendices/tag_mapping.html#performer) + // We may hit both styles for the same track // PERFORMER: artist (role) if (const std::vector artistNames {getPropertyValuesFirstMatchAs(properties, artistTagNames)}; !artistNames.empty()) { @@ -151,23 +152,20 @@ getPerformerArtists(const TagLib::PropertyMap& properties, } } // PERFORMER:role (MP3) - else + for (const auto& [key, values] : properties) { - for (const auto& [key, values] : properties) + if (key.startsWith("PERFORMER:")) { - if (key.startsWith("PERFORMER")) + std::string performerStr {key.to8Bit(true)}; + std::string role; + if (const std::size_t rolePos {performerStr.find(':')}; rolePos != std::string::npos) { - std::string performerStr {key.to8Bit(true)}; - std::string role; - if (const std::size_t rolePos {performerStr.find(':')}; rolePos != std::string::npos) - { - role = StringUtils::stringToLower(performerStr.substr(rolePos + 1, performerStr.size() - rolePos + 1)); - StringUtils::capitalize(role); - } - - for (const auto& value : values) - performers[role].push_back(Artist {value.to8Bit(true)}); + role = StringUtils::stringToLower(performerStr.substr(rolePos + 1, performerStr.size() - rolePos + 1)); + StringUtils::capitalize(role); } + + for (const auto& value : values) + performers[role].push_back(Artist {value.to8Bit(true)}); } } diff --git a/src/libs/services/recommendation/impl/features/FeaturesEngine.cpp b/src/libs/services/recommendation/impl/features/FeaturesEngine.cpp index fc44eb48..1cfb6811 100644 --- a/src/libs/services/recommendation/impl/features/FeaturesEngine.cpp +++ b/src/libs/services/recommendation/impl/features/FeaturesEngine.cpp @@ -194,7 +194,6 @@ FeaturesEngine::loadFromTraining(const TrainSettings& trainSettings, const Progr [this] { return _loadCancelled; }); LMS_LOG(RECOMMENDATION, DEBUG) << "Training network DONE"; - LMS_LOG(RECOMMENDATION, DEBUG) << "Classifying tracks..."; TrackPositions trackPositions; for (std::size_t i {}; i < samples.size(); ++i) @@ -351,7 +350,7 @@ FeaturesEngine::load(bool forceReload, const ProgressCallback& progressCallback) trainSettings.featureSettingsMap = getDefaultTrainFeatureSettings(); loadFromTraining(trainSettings, progressCallback); - if (!_loadCancelled) + if (!_loadCancelled && _network) toCache().write(); } diff --git a/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp b/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp index 1e3e654d..e9265122 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp @@ -19,6 +19,7 @@ #include "ListenBrainzScrobbler.hpp" +#include #include #include #include @@ -234,8 +235,7 @@ namespace Scrobbling::ListenBrainz auto itContext {_userContexts.find(userId)}; if (itContext == std::cend(_userContexts)) { - [[maybe_unused]] auto [itNewContext, inserted] {_userContexts.emplace(userId, userId)}; - itContext = itNewContext; + std::tie(itContext, std::ignore) = _userContexts.emplace(userId, userId); } return itContext->second; @@ -246,8 +246,7 @@ namespace Scrobbling::ListenBrainz { return std::any_of(std::cbegin(_userContexts), std::cend(_userContexts), [](const auto& contextEntry) { - [[maybe_unused]] const auto& [userId, context] {contextEntry}; - return context.syncing; + return contextEntry.second.syncing; }); } diff --git a/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp b/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp index 2542ede7..f1a79df0 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/ListensSynchronizer.cpp @@ -19,6 +19,7 @@ #include "ListenBrainzScrobbler.hpp" +#include #include #include #include @@ -354,8 +355,7 @@ namespace Scrobbling::ListenBrainz auto itContext {_userContexts.find(userId)}; if (itContext == std::cend(_userContexts)) { - [[maybe_unused]] auto [itNewContext, inserted] {_userContexts.emplace(userId, userId)}; - itContext = itNewContext; + std::tie(itContext, std::ignore) = _userContexts.emplace(userId, userId); } return itContext->second; @@ -366,8 +366,7 @@ namespace Scrobbling::ListenBrainz { return std::any_of(std::cbegin(_userContexts), std::cend(_userContexts), [](const auto& contextEntry) { - [[maybe_unused]] const auto& [userId, context] {contextEntry}; - return context.syncing; + return contextEntry.second.syncing; }); } diff --git a/src/libs/subsonic/impl/ParameterParsing.hpp b/src/libs/subsonic/impl/ParameterParsing.hpp index 2773a08e..98635c68 100644 --- a/src/libs/subsonic/impl/ParameterParsing.hpp +++ b/src/libs/subsonic/impl/ParameterParsing.hpp @@ -80,5 +80,12 @@ namespace API::Subsonic return *res; } + + inline + bool + hasParameter(const Wt::Http::ParameterMap& parameterMap, const std::string& param) + { + return parameterMap.find(param) != std::cend(parameterMap); + } } diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index 82d1efa9..c8ffad2d 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -1169,49 +1169,103 @@ handleGetArtistsRequest(RequestContext& context) } static -Response -handleGetSimilarSongsRequestCommon(RequestContext& context, bool id3) +std::vector +findSimilarSongs(RequestContext& context, ArtistId artistId, std::size_t count) { - // Mandatory params - const ArtistId artistId {getMandatoryParameterAs(context.parameters, "id")}; + // API says: "Returns a random collection of songs from the given artist and similar artists" + const std::size_t similarArtistCount {count / 5}; + std::vector artistIds {Service::get()->getSimilarArtists(artistId, {TrackArtistLinkType::Artist, TrackArtistLinkType::ReleaseArtist}, similarArtistCount)}; + artistIds.push_back(artistId); - // Optional params - std::size_t count {getParameterAs(context.parameters, "count").value_or(50)}; - - const auto similarArtistIds {Service::get()->getSimilarArtists(artistId, {TrackArtistLinkType::Artist, TrackArtistLinkType::ReleaseArtist}, 5)}; + const std::size_t meanTrackCountPerArtist {(count / artistIds.size()) + 1}; auto transaction {context.dbSession.createSharedTransaction()}; - const Artist::pointer artist {Artist::find(context.dbSession, artistId)}; - if (!artist) - throw RequestedDataNotFoundError {}; + std::vector tracks; + tracks.reserve(count); - const User::pointer user {User::find(context.dbSession, context.userId)}; - if (!user) - throw UserNotAuthorizedError {}; - - // "Returns a random collection of songs from the given artist and similar artists" - const auto trackResults {Track::find(context.dbSession, Track::FindParameters {} - .setArtist(artist->getId()) - .setRange({0, count / 2}) - .setSortMethod(TrackSortMethod::Random))}; - - std::vector tracks {trackResults.results}; - - for (const ArtistId similarArtistId : similarArtistIds) + for (const ArtistId id : artistIds) { - const auto similarArtistTracks {Track::find(context.dbSession, Track::FindParameters {} - .setArtist(similarArtistId) - .setRange({0, (count / 2) / 5}) - .setSortMethod(TrackSortMethod::Random))}; + Track::FindParameters params; + params.setArtist(id); + params.setRange({0, meanTrackCountPerArtist}); + params.setSortMethod(TrackSortMethod::Random); + const auto artistTracks {Track::find(context.dbSession, params)}; tracks.insert(std::end(tracks), - std::begin(similarArtistTracks.results), - std::end(similarArtistTracks.results)); + std::begin(artistTracks.results), + std::end(artistTracks.results)); } + return tracks; +} + +static +std::vector +findSimilarSongs(RequestContext& context, ReleaseId releaseId, std::size_t count) +{ + // API says: "Returns a random collection of songs from the given artist and similar artists" + // so let's extend this for release + const std::size_t similarReleaseCount {count / 5}; + std::vector releaseIds {Service::get()->getSimilarReleases(releaseId, similarReleaseCount)}; + releaseIds.push_back(releaseId); + + const std::size_t meanTrackCountPerRelease {(count / releaseIds.size()) + 1}; + + auto transaction {context.dbSession.createSharedTransaction()}; + + std::vector tracks; + tracks.reserve(count); + + for (const ReleaseId id : releaseIds) + { + Track::FindParameters params; + params.setRelease(id); + params.setRange({0, meanTrackCountPerRelease}); + params.setSortMethod(TrackSortMethod::Random); + + const auto releaseTracks {Track::find(context.dbSession, params)}; + tracks.insert(std::end(tracks), + std::begin(releaseTracks.results), + std::end(releaseTracks.results)); + } + + return tracks; +} + +static +std::vector +findSimilarSongs(RequestContext&, TrackId trackId, std::size_t count) +{ + return Service::get()->findSimilarTracks({trackId}, count); +} + +static +Response +handleGetSimilarSongsRequestCommon(RequestContext& context, bool id3) +{ + // Optional params + std::size_t count {getParameterAs(context.parameters, "count").value_or(50)}; + + std::vector tracks; + + if (const auto artistId {getParameterAs(context.parameters, "id")}) + tracks = findSimilarSongs(context, *artistId, count); + else if (const auto releaseId {getParameterAs(context.parameters, "id")}) + tracks = findSimilarSongs(context, *releaseId, count); + else if (const auto trackId {getParameterAs(context.parameters, "id")}) + tracks = findSimilarSongs(context, *trackId, count); + else + throw BadParameterGenericError {"id"}; + Random::shuffleContainer(tracks); + auto transaction {context.dbSession.createSharedTransaction()}; + + User::pointer user {User::find(context.dbSession, context.userId)}; + if (!user) + throw UserNotAuthorizedError {}; + Response response {Response::createOkResponse(context.serverProtocolVersion)}; Response::Node& similarSongsNode {response.createNode(id3 ? "similarSongs2" : "similarSongs")}; for (const TrackId trackId : tracks) @@ -1445,7 +1499,12 @@ Response handleSearchRequestCommon(RequestContext& context, bool id3) { // Mandatory params - std::string query {getMandatoryParameterAs(context.parameters, "query")}; + std::string queryString {getMandatoryParameterAs(context.parameters, "query")}; + std::string_view query {queryString}; + + // Symfonium adds extra "" + if (context.clientInfo.name == "Symfonium") + query = StringUtils::stringTrim(query, "\""); std::vector keywords {StringUtils::splitString(query, " ")}; @@ -1466,6 +1525,7 @@ handleSearchRequestCommon(RequestContext& context, bool id3) Response response {Response::createOkResponse(context.serverProtocolVersion)}; Response::Node& searchResult2Node {response.createNode(id3 ? "searchResult3" : "searchResult2")}; + if (artistCount > 0) { Artist::FindParameters params; params.setKeywords(keywords); @@ -1480,6 +1540,7 @@ handleSearchRequestCommon(RequestContext& context, bool id3) } } + if (albumCount > 0) { Release::FindParameters params; params.setKeywords(keywords); @@ -1494,6 +1555,7 @@ handleSearchRequestCommon(RequestContext& context, bool id3) } } + if (songCount > 0) { Track::FindParameters params; params.setKeywords(keywords); @@ -2052,6 +2114,9 @@ SubsonicResource::getClientInfo(const Wt::Http::ParameterMap& parameters) { ClientInfo res; + if (hasParameter(parameters, "t")) + throw TokenAuthenticationNotSupportedForLDAPUsersError {}; + // Mandatory parameters res.name = getMandatoryParameterAs(parameters, "c"); res.version = getMandatoryParameterAs(parameters, "v"); diff --git a/src/libs/subsonic/impl/SubsonicResponse.hpp b/src/libs/subsonic/impl/SubsonicResponse.hpp index 321d7c76..d2d5294f 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.hpp +++ b/src/libs/subsonic/impl/SubsonicResponse.hpp @@ -48,6 +48,7 @@ class Error ClientMustUpgrade = 20, ServerMustUpgrade = 30, WrongUsernameOrPassword = 40, + TokenAuthenticationNotSupportedForLDAPUsers = 41, UserNotAuthorized = 50, RequestedDataNotFound = 70, }; @@ -105,6 +106,14 @@ class WrongUsernameOrPasswordError : public Error std::string getMessage() const override { return "Wrong username or password."; } }; +class TokenAuthenticationNotSupportedForLDAPUsersError : public Error +{ + public: + TokenAuthenticationNotSupportedForLDAPUsersError() : Error {Code::TokenAuthenticationNotSupportedForLDAPUsers} {} + private: + std::string getMessage() const override { return "Token authentication not supported for LDAP users."; } +}; + class UserNotAuthorizedError : public Error { public: diff --git a/src/libs/utils/impl/ChildProcess.cpp b/src/libs/utils/impl/ChildProcess.cpp index 90b44881..627bbcd9 100644 --- a/src/libs/utils/impl/ChildProcess.cpp +++ b/src/libs/utils/impl/ChildProcess.cpp @@ -44,7 +44,7 @@ namespace { public: SystemException(int err, const std::string& errMsg) - : ChildProcessException {errMsg + ": " + strerror(err)} + : ChildProcessException {errMsg + ": " + ::strerror(err)} {} SystemException(boost::system::error_code ec, const std::string& errMsg) @@ -117,42 +117,36 @@ ChildProcess::ChildProcess(boost::asio::io_context& ioContext, const std::filesy ChildProcess::~ChildProcess() { - if (!_waited) + LMS_LOG(CHILDPROCESS, DEBUG) << "Closing child process..."; { - LMS_LOG(CHILDPROCESS, DEBUG) << "Closing child process..."; - { - boost::system::error_code closeError; - _childStdout.close(closeError); - if (closeError) - LMS_LOG(CHILDPROCESS, ERROR) << "Closed failed: " << closeError.message(); - } - kill(); - wait(true); + boost::system::error_code closeError; + _childStdout.close(closeError); + if (closeError) + LMS_LOG(CHILDPROCESS, ERROR) << "Closed failed: " << closeError.message(); } -} -void -ChildProcess::drain() -{ - char buf[128]; + if (!_finished) + kill(); - while (boost::asio::read(_childStdout, boost::asio::buffer(buf)) > 0) - LMS_LOG(CHILDPROCESS, DEBUG) << "drained some bytes" << std::endl; + wait(true); } void ChildProcess::kill() { + // process may already have finished LMS_LOG(CHILDPROCESS, DEBUG) << "Killing child process..."; - ::kill(_childPID, SIGKILL); + if (::kill(_childPID, SIGKILL) == -1) + LMS_LOG(CHILDPROCESS, DEBUG) << "Kill failed: " << ::strerror(errno); } bool ChildProcess::wait(bool block) { - int wstatus {}; + assert(!_waited); - pid_t pid {waitpid(_childPID, &wstatus, block ? 0 : WNOHANG)}; + int wstatus {}; + const pid_t pid {waitpid(_childPID, &wstatus, block ? 0 : WNOHANG)}; if (pid == -1) throw SystemException {errno, "waitpid failed!"}; @@ -160,18 +154,22 @@ ChildProcess::wait(bool block) return false; if (WIFEXITED(wstatus)) + { _exitCode = WEXITSTATUS(wstatus); + LMS_LOG(CHILDPROCESS, DEBUG) << "Exit code = " << *_exitCode; + } _waited = true; return true; } - void ChildProcess::asyncRead(std::byte* data, std::size_t bufferSize, ReadCallback callback) { assert(!finished()); + LMS_LOG(CHILDPROCESS, DEBUG) << "Async read, bufferSize = " << bufferSize; + boost::asio::async_read(_childStdout, boost::asio::buffer(data, bufferSize), [this, callback {std::move(callback)}](const boost::system::error_code& error, std::size_t bytesTransferred) { @@ -180,33 +178,20 @@ ChildProcess::asyncRead(std::byte* data, std::size_t bufferSize, ReadCallback ca ReadResult readResult {ReadResult::Success}; if (error) { - _finished = true; - - if (error == boost::asio::error::eof) - readResult = ReadResult::EndOfFile; - else + if (error != boost::asio::error::eof) + { + // forbidden to read any captured param here as the ChildProcess instance may already have been killed return; + } + + readResult = ReadResult::EndOfFile; + _finished = true; } callback(readResult, bytesTransferred); }); } -void -ChildProcess::asyncWaitForData(WaitCallback cb) -{ - LMS_LOG(CHILDPROCESS, DEBUG) << "Async wait requested"; - assert(!finished()); - - _childStdout.async_wait(boost::asio::posix::stream_descriptor::wait_read, - [cb {std::move(cb)}](const boost::system::error_code& ec) - { - LMS_LOG(CHILDPROCESS, DEBUG) << "Wait CB, error = " << ec.message(); - if (!ec) - cb(); - }); -} - std::size_t ChildProcess::readSome(std::byte* data, std::size_t bufferSize) { @@ -220,7 +205,7 @@ ChildProcess::readSome(std::byte* data, std::size_t bufferSize) } bool -ChildProcess::finished() +ChildProcess::finished() const { return _finished; } diff --git a/src/libs/utils/impl/ChildProcess.hpp b/src/libs/utils/impl/ChildProcess.hpp index 46d7bcb0..8b5752dc 100644 --- a/src/libs/utils/impl/ChildProcess.hpp +++ b/src/libs/utils/impl/ChildProcess.hpp @@ -40,12 +40,10 @@ class ChildProcess : public IChildProcess private: void asyncRead(std::byte* data, std::size_t bufferSize, ReadCallback callback) override; - void asyncWaitForData(WaitCallback cb) override; std::size_t readSome(std::byte* data, std::size_t bufferSize) override; - bool finished() override; + bool finished() const override; void kill(); - void drain(); bool wait(bool block); // return true if waited using FileDescriptor = boost::asio::posix::stream_descriptor; diff --git a/src/libs/utils/include/utils/IChildProcess.hpp b/src/libs/utils/include/utils/IChildProcess.hpp index 7de5731a..72768892 100644 --- a/src/libs/utils/include/utils/IChildProcess.hpp +++ b/src/libs/utils/include/utils/IChildProcess.hpp @@ -48,9 +48,7 @@ class IChildProcess using ReadCallback = std::function; virtual void asyncRead(std::byte* data, std::size_t bufferSize, ReadCallback callback) = 0; - using WaitCallback = std::function; - virtual void asyncWaitForData(WaitCallback cb) = 0; virtual std::size_t readSome(std::byte* data, std::size_t bufferSize) = 0; - virtual bool finished() = 0; + virtual bool finished() const = 0; }; diff --git a/src/lms/ui/LmsApplication.cpp b/src/lms/ui/LmsApplication.cpp index 0e09f8ea..9aaf02af 100644 --- a/src/lms/ui/LmsApplication.cpp +++ b/src/lms/ui/LmsApplication.cpp @@ -224,9 +224,7 @@ LmsApplication::init() enableUpdates(true); if (_authenticatedUser) - { onUserLoggedIn(); - } else if (Service<::Auth::IPasswordService>::exists()) processPasswordAuth(); } @@ -339,7 +337,6 @@ handlePathChange(Wt::WStackedWidget& stack, bool isAdmin) LMS_LOG(UI, DEBUG) << "Internal path changed to '" << wApp->internalPath() << "'"; - LmsApp->doJavaScript(R"($('.navbar-nav a.active').removeClass('active'); $('.navbar-nav a[href="' + location.pathname + '"]').closest('a').addClass('active');)"); for (const auto& view : views) { if (wApp->internalPathMatches(view.path)) @@ -350,6 +347,8 @@ handlePathChange(Wt::WStackedWidget& stack, bool isAdmin) stack.setCurrentIndex(view.index); if (view.title) LmsApp->setTitle(*view.title); + + LmsApp->doJavaScript(LmsApp->javaScriptClass() + ".updateActiveNav('" + view.path +"')"); return; } } @@ -398,9 +397,21 @@ LmsApplication::createHome() _coverResource = std::make_shared(); declareJavaScriptFunction("onLoadCover", "function(id) { id.className += \" Lms-cover-loaded\"}"); + declareJavaScriptFunction("updateActiveNav", +R"(function(current) { + const menuItems = document.querySelectorAll('.nav-item a[href]:not([href=""])'); + for (const menuItem of menuItems) { + if (menuItem.getAttribute("href").indexOf(current) !== -1) { + menuItem.classList.add('active'); + } + else { + menuItem.classList.remove('active'); + } + + } +})"); Wt::WTemplate* main {root()->addWidget(std::make_unique(Wt::WString::tr("Lms.main.template")))}; - main->addFunction("tr", &Wt::WTemplate::Functions::tr); Template* navbar {main->bindNew