From a48f262316b04d59b4ca77fae0b181e7540fe006 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sat, 12 Sep 2026 16:24:52 +0200 Subject: [PATCH 1/7] feat: replace DocumentManager with RAII MusicXml (#95) DocumentManager was a singleton that returned numeric document ids, held a map of shared_ptr, and required the client to call destroyDocument by hand. The registry, the ids, the badDocumentId error code, and the getUniqueId counter all existed only to support that scheme. They are gone. The public entry point is now a move-only mx::api::MusicXml that owns its core::Document and frees it at scope exit: auto doc = MusicXml::fromFile(path); // Result auto score = intoScore(std::move(doc).value()); // consumes the doc auto doc = fromScore(score); // Result doc.writeToFile(out); // Result const auto& core = doc.getCoreDocument(); // escape hatch The free functions are the score conversions: getScore (const, non-consuming), intoScore (by value, consumes and frees the tree at return), and fromScore (authors a new document; can fail with the core writer's refusal errors). All four entry points preserve the Result error boundary: parse/serialize failures return ioError, xmlSyntaxError, the mirrored core parse codes, or internalError for any caught exception. A moved-from MusicXml fails safely on every path (internalError on read/write, an empty core document on getCoreDocument) rather than crashing. PartWriter's synthesized ids used to come from DocumentManager::getUniqueId; they are now a process-wide atomic counter inside PartWriter, seeded at the same high value so they still avoid collisions with ids present in parsed documents. Tests, examples, and the README were ported from the id/destroyDocument dance to the RAII ownership model. The DocumentManager-based test file is replaced by MusicXmlTest, which also pins the moved-from behavior. The api/impl layers are the only ones touched; core, the generator, and the roundtrip baseline are unchanged. Closes #95. --- .agents/skills/mx-api-doctrine/SKILL.md | 8 +- .../instructions/api-headers.instructions.md | 2 +- AGENTS.md | 4 +- CMakeLists.txt | 4 +- README.md | 49 +-- src/include/mx/api/DocumentManager.h | 91 ----- src/include/mx/api/MusicXml.h | 83 +++++ src/include/mx/api/Result.h | 3 +- .../api/{DocumentManager.cpp => MusicXml.cpp} | 302 +++++++--------- src/private/mx/examples/Hide.cpp | 2 +- .../mx/examples/IterateNotesAndTiming.cpp | 11 +- src/private/mx/examples/Read.cpp | 21 +- src/private/mx/examples/Write.cpp | 20 +- src/private/mx/impl/PartWriter.cpp | 14 +- src/private/mx/impl/ScoreConversions.h | 2 +- src/private/mx/impl/ScoreWriter.cpp | 4 +- src/private/mx/impl/WriteRefusal.h | 7 +- src/private/mxtest/api/ApiChordSimpleTest.cpp | 2 +- src/private/mxtest/api/ApiK007aTest.cpp | 2 +- src/private/mxtest/api/ApiK007cTest.cpp | 2 +- src/private/mxtest/api/ApiK009bSlurTest.cpp | 2 +- .../mxtest/api/ApiK014aFermatasTest.cpp | 2 +- src/private/mxtest/api/ApiK015aLayoutTest.cpp | 2 +- src/private/mxtest/api/ApiK016aMiscTest.cpp | 2 +- src/private/mxtest/api/ApiLoadSmokeTest.cpp | 15 +- src/private/mxtest/api/ApiLy43eTest.cpp | 2 +- .../mxtest/api/ApiMuAccidentals1Test.cpp | 2 +- src/private/mxtest/api/ApiTester.cpp | 71 ++-- src/private/mxtest/api/BombeTest.cpp | 9 +- src/private/mxtest/api/ChordApiTest.cpp | 22 +- .../mxtest/api/ChordDataSaveAndLoadTest.cpp | 15 +- src/private/mxtest/api/ChordTimeTest.cpp | 16 +- .../mxtest/api/CorpusRoundtripMain.cpp | 45 +-- .../mxtest/api/CreditRoundTripTest.cpp | 2 +- src/private/mxtest/api/CrossStaffApiTest.cpp | 2 +- .../mxtest/api/DefaultsFontsRoundTripTest.cpp | 2 +- src/private/mxtest/api/DirectionDataTest.cpp | 9 +- .../api/DirectionMarksRoundTripTest.cpp | 15 +- src/private/mxtest/api/FreezingRoundTrip.cpp | 235 ++++++------ src/private/mxtest/api/FreezingTest.cpp | 8 +- src/private/mxtest/api/GlissandoApiTest.cpp | 2 +- src/private/mxtest/api/GraceCueApiTest.cpp | 2 +- src/private/mxtest/api/IdAttributeApiTest.cpp | 2 +- .../api/IdentificationSourceApiTest.cpp | 2 +- src/private/mxtest/api/KeyDataTest.cpp | 110 +++--- src/private/mxtest/api/MarkRoundTripTest.cpp | 15 +- src/private/mxtest/api/MeasureDataTest.cpp | 26 +- src/private/mxtest/api/MetronomeApiTest.cpp | 20 +- .../mxtest/api/MidiNameRoundTripTest.cpp | 2 +- ...cumentManagerTest.cpp => MusicXmlTest.cpp} | 249 ++++++------- src/private/mxtest/api/MxlTest.cpp | 8 +- src/private/mxtest/api/NewSystemTest.cpp | 13 +- src/private/mxtest/api/NoteDataTest.cpp | 339 ++++++------------ .../mxtest/api/NoteVelocityApiTest.cpp | 2 +- src/private/mxtest/api/OttavaSizeApiTest.cpp | 2 +- src/private/mxtest/api/PageDataTest.cpp | 27 +- .../mxtest/api/PartGroupRoundTripTest.cpp | 2 +- src/private/mxtest/api/PitchDataTest.cpp | 34 +- .../mxtest/api/PrintLayoutRoundTripTest.cpp | 2 +- src/private/mxtest/api/RepeatApiTest.cpp | 2 +- src/private/mxtest/api/RoundTrip.h | 41 +-- .../mxtest/api/ScorePartGroupApiTest.cpp | 2 +- .../mxtest/api/SingleNoteSpannerApiTest.cpp | 2 +- src/private/mxtest/api/SoundApiTest.cpp | 2 +- .../mxtest/api/SpannerIdentityTest.cpp | 2 +- src/private/mxtest/api/StaffCountApiTest.cpp | 2 +- src/private/mxtest/api/TestHelpers.h | 25 +- .../mxtest/api/TimeSignatureApiTest.cpp | 15 +- src/private/mxtest/api/TranspositionTest.cpp | 28 +- src/private/mxtest/api/VoiceLabelApiTest.cpp | 2 +- src/private/mxtest/api/WavyLineApiTest.cpp | 2 +- src/private/mxtest/file/MxFileRepositoy.cpp | 11 +- .../mxtest/impl/PositionFunctionsTest.cpp | 19 +- 73 files changed, 860 insertions(+), 1265 deletions(-) delete mode 100644 src/include/mx/api/DocumentManager.h create mode 100644 src/include/mx/api/MusicXml.h rename src/private/mx/api/{DocumentManager.cpp => MusicXml.cpp} (51%) rename src/private/mxtest/api/{DocumentManagerTest.cpp => MusicXmlTest.cpp} (71%) diff --git a/.agents/skills/mx-api-doctrine/SKILL.md b/.agents/skills/mx-api-doctrine/SKILL.md index 24c801358..4df0e675f 100644 --- a/.agents/skills/mx-api-doctrine/SKILL.md +++ b/.agents/skills/mx-api-doctrine/SKILL.md @@ -43,9 +43,9 @@ Responses to wrong api usage, in order of preference: returns a default-constructed copy; the writer drops the half of an encoding that is meaningless for the note it is on (a tie on a silent cue note is written as `` notation only, never as a sound-level ``). No signal to the caller. -3. `Result` (`Result.h`): the error channel of last resort. It exists for the - `DocumentManager` I/O boundary, where failure is real (unreadable file, unparseable XML). Do - not spread it into the data model. +3. `Result` (`Result.h`): the error channel of last resort. It exists for the `MusicXml` + I/O boundary, where failure is real (unreadable file, unparseable XML). Do not spread it + into the data model. Never: @@ -53,7 +53,7 @@ Never: returned reference whose validity depends on a precondition, no "caller must check first or else". - Exceptions. Nothing throws across the api boundary, and an exception is never how a failed - precondition is reported to the caller. `DocumentManager` catches everything + precondition is reported to the caller. The `MusicXml` functions catch everything (`ResultCode::internalError`). ## Choice types: when you wish for a Rust enum diff --git a/.github/instructions/api-headers.instructions.md b/.github/instructions/api-headers.instructions.md index c10ec33f5..4fb4481db 100644 --- a/.github/instructions/api-headers.instructions.md +++ b/.github/instructions/api-headers.instructions.md @@ -17,7 +17,7 @@ simpler model (doctrine: `.claude/skills/mx-api-doctrine/SKILL.md`). Review for: duplicated, or id-linked; check the change against the principles doc. - A new positioned-in-a-measure type needs `int tickTimePosition`; durations are in ticks. - No UB or exceptions reachable through the public interface; a failed precondition must never - throw. Flag new `Result` usage outside `DocumentManager`, unchecked `std::get`, or accessors + throw. Flag new `Result` usage outside `MusicXml`, unchecked `std::get`, or accessors that return references guarded only by a precondition. - Kind-specific payloads use the choice-class pattern (`TimeChoice.h`, `MarkDataChoice.h`), not loose fields that apply only to some kinds. diff --git a/AGENTS.md b/AGENTS.md index 89db5fa8c..06bae3e3c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -152,9 +152,9 @@ comment on a PR or the Coverage workflow's "Run workflow" button (`.github/workf | File | What it is | |------|------------| -| `src/include/mx/api/DocumentManager.h` | The public API entry point: createFromFile, createFromScore, getData, writeToFile | +| `src/include/mx/api/MusicXml.h` | The public API entry point: fromFile, fromStream, fromScore, getScore, intoScore, writeTo* | | `src/include/mx/api/ScoreData.h` | The primary api data model (ScoreData, PartData, MeasureData, ...) | -| `src/private/mx/api/DocumentManager.cpp` | API implementation: error channel, parse/serialize orchestration | +| `src/private/mx/api/MusicXml.cpp` | API implementation: error channel, parse/serialize orchestration | | `src/private/mx/impl/ScoreReader.cpp` | Translates mx::core -> mx::api ScoreData | | `src/private/mx/impl/ScoreWriter.cpp` | Translates mx::api ScoreData -> mx::core | | `src/private/mx/impl/NoteReader.cpp` | Core -> api note translation (one of the largest impl files) | diff --git a/CMakeLists.txt b/CMakeLists.txt index e7a0a9736..0fe8a77c7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,10 +13,10 @@ set(EXECUTABLE_OUTPUT_PATH ${CMAKE_BINARY_DIR}) set(LIBRARY_OUTPUT_PATH ${CMAKE_BINARY_DIR}) # Emscripten disables exception catching by default; mx's internal MX_THROW -# (Throw.h) needs it, so DocumentManager can actually catch and convert it. +# (Throw.h) needs it, so the api boundary can actually catch and convert it. # # Emscripten's default wasm stack is 64 KiB. mx's write path is a deep, -# unoptimized (Debug) C++ call chain -- DocumentManager -> ScoreWriter -> +# unoptimized (Debug) C++ call chain -- fromScore -> ScoreWriter -> # PartWriter -> MeasureWriter -> NoteWriter -> NotationsWriter, several of # which carry their own sizable locals -- and it overflows that default with # only a little headroom to spare (issue #389 hit this by adding two more diff --git a/README.md b/README.md index b03d948b9..cc128e0dc 100644 --- a/README.md +++ b/README.md @@ -129,8 +129,8 @@ git add --all && git commit -m'mx sourcecode' # create a main.cpp file cat <<- "EOF" > main.cpp #include +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" -#include "mx/api/DocumentManager.h" int main () { using namespace mx::api; @@ -148,10 +148,8 @@ int main () { PartData part{}; part.measures.push_back(measure); score.parts.push_back(part); - auto& mgr = DocumentManager::getInstance(); - const auto idResult = mgr.createFromScore(score); - mgr.writeToStream(idResult.value(), std::cout); - mgr.destroyDocument(idResult.value()); + const auto docResult = fromScore(score); + std::move(docResult).value().writeToStream(std::cout); } EOF @@ -207,7 +205,7 @@ in `mx::api`, such as the need to manage beam starts and stops explicitly. #include #include -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" // set this to 1 if you want to see the xml in your console @@ -313,26 +311,22 @@ int main(int argc, const char * argv[]) note.beams.clear(); voice.notes.push_back( note ); - // the document manager is the liaison between our score data and the MusicXML DOM. - // it completely hides the MusicXML DOM from us when using mx::api - auto& mgr = DocumentManager::getInstance(); - const auto idResult = mgr.createFromScore( score ); - if( !idResult.ok() ) { return 1; } - const auto documentID = idResult.value(); + // a MusicXml document is created from the score data and owns the + // underlying MusicXML model, which is hidden from us when using mx::api + const auto docResult = fromScore( score ); + if( !docResult.ok() ) { return 1; } + const auto document = std::move( docResult ).value(); // write to the console #if MX_WRITE_THIS_TO_THE_CONSOLE - mgr.writeToStream( documentID, std::cout ); + document.writeToStream( std::cout ); std::cout << std::endl; #endif // write to a file. argv[1] overrides the default output path so the build // system can send the file to a gitignored location (see issue #150). const std::string outputPath = ( argc > 1 ) ? argv[1] : "./example.musicxml"; - const auto writeResult = mgr.writeToFile( documentID, outputPath ); - - // we need to explicitly delete the object held by the manager - mgr.destroyDocument( documentID ); + const auto writeResult = document.writeToFile( outputPath ); return writeResult.ok() ? 0 : 1; } @@ -341,7 +335,7 @@ int main(int argc, const char * argv[]) #### Reading MusicXML with `mx::api` ```C++ -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include @@ -396,23 +390,16 @@ int main(int argc, const char * argv[]) { using namespace mx::api; - // create a reference to the singleton which holds documents in memory for us - auto& mgr = DocumentManager::getInstance(); - // place the xml from above into a stream object std::istringstream istr{ xml }; - // ask the document manager to parse the xml into memory for us, returns a document ID. - const auto idResult = mgr.createFromStream( istr ); - if( !idResult.ok() ) { return MX_IS_A_FAILURE; } - const auto documentID = idResult.value(); - - // get the structural representation of the score from the document manager - const auto scoreResult = mgr.getData( documentID ); - - // we need to explicitly destroy the document from memory - mgr.destroyDocument( documentID ); + // parse the xml into a MusicXml document that we own + const auto docResult = MusicXml::fromStream( istr ); + if( !docResult.ok() ) { return MX_IS_A_FAILURE; } + // take the score out of the document. intoScore also consumes the + // document, so its memory is freed as the function returns + const auto scoreResult = intoScore( std::move( docResult ).value() ); if( !scoreResult.ok() ) { return MX_IS_A_FAILURE; } const auto& score = scoreResult.value(); diff --git a/src/include/mx/api/DocumentManager.h b/src/include/mx/api/DocumentManager.h deleted file mode 100644 index 38b7d637a..000000000 --- a/src/include/mx/api/DocumentManager.h +++ /dev/null @@ -1,91 +0,0 @@ -// MusicXML Class Library -// Copyright (c) by Matthew James Briggs -// Distributed under the MIT License - -#pragma once - -#include "mx/api/Result.h" -#include "mx/api/ScoreData.h" - -#include -#include - -namespace mx -{ -namespace core -{ -class Document; -using DocumentPtr = std::shared_ptr; -} // namespace core - -namespace api -{ -// The mx::api error channel: no exceptions escape this boundary; failures -// speak Result/ApiError, and any exception from -// below becomes ResultCode::internalError. -class DocumentManager -{ - public: - DocumentManager(const DocumentManager &other) = delete; - DocumentManager(DocumentManager &&other) = delete; - DocumentManager &operator=(const DocumentManager &other) = delete; - DocumentManager &operator=(DocumentManager &&other) = delete; - ~DocumentManager(); - - // this class is a singleton, get it like this - // auto& docMngr = DocumentManager::getInstance() - static DocumentManager &getInstance(); - - // creates a MusicXML document from a file and returns the document's - // ID number. Errors: ioError (file open/read), xmlSyntaxError (the - // bytes are not XML), or the mirrored core parse errors. - Result createFromFile(const std::string &filePath); - - // creates a MusicXML document from a character stream and returns the - // document's ID number. Same errors as createFromFile, minus file I/O. - Result createFromStream(std::istream &stream); - - // creates a MusicXML document from a Score structure and returns the - // document's ID number. CAN fail: when the ScoreData describes - // something the core model will not represent (e.g. more than 8 - // beams), the error is returned rather than silently dropping data. - Result createFromScore(const ScoreData &score); - - // saves an existing MusicXML document to a file. - // Errors: badDocumentId, ioError on write failure. - Result writeToFile(int documentId, const std::string &filePath) const; - - // saves an existing MusicXML document to a character stream. - // Errors: badDocumentId. - Result writeToStream(int documentId, std::ostream &stream) const; - - // retrieves the data from an existing document and returns it in the - // Score structure. Errors: badDocumentId. - Result getData(int documentId) const; - - // destroys the shared_ptr which is holding the document internally and - // stops tracking ownership of the document. idempotent: destroying a - // nonexistent id is a silent no-op (that is a feature). - void destroyDocument(int documentId); - - // avoid using this function if possible, only use this function - // if your requirements are not met by the Score structure - // note: returns a nullptr if the documentId is bad. if you - // hold a DocumentPtr and call destroyDocument then the - // DocumentManager will no longer know about the object, but the - // object will still exist - mx::core::DocumentPtr getDocument(int documentId) const; - - // returns a unique number and increments the unique number - // generator. this is here as an aid to any client code that - // needs unique numbers since DocumentManager is already a - // thread-locking singleton, it can easily implement this. - int getUniqueId(); - - private: - DocumentManager(); - class Impl; - std::unique_ptr myImpl; -}; -} // namespace api -} // namespace mx diff --git a/src/include/mx/api/MusicXml.h b/src/include/mx/api/MusicXml.h new file mode 100644 index 000000000..14acb4884 --- /dev/null +++ b/src/include/mx/api/MusicXml.h @@ -0,0 +1,83 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#pragma once + +#include "mx/api/Result.h" +#include "mx/api/ScoreData.h" + +#include +#include +#include + +namespace mx +{ +namespace core +{ +class Document; +using DocumentPtr = std::shared_ptr; +} // namespace core + +namespace api +{ +// A MusicXML document that you own. Parse one from a file or a stream, or +// author one from ScoreData, then read the score back out of it or write it +// to disk. The document is freed automatically when it goes out of scope; +// mx keeps no registry of documents and tracks no ids. A document cannot be +// copied, only moved. +class MusicXml +{ + public: + // parses a .musicxml file. Errors: ioError (file open/read), + // xmlSyntaxError (the bytes are not XML), or the mirrored core parse + // errors. + static Result fromFile(const std::string &filePath); + + // parses from any character stream. Same errors as fromFile, minus the + // file I/O. + static Result fromStream(std::istream &stream); + + MusicXml(const MusicXml &other) = delete; + MusicXml &operator=(const MusicXml &other) = delete; + MusicXml(MusicXml &&other) noexcept; + MusicXml &operator=(MusicXml &&other) noexcept; + ~MusicXml(); + + // writes the document to a file. Errors: ioError on write failure. + Result writeToFile(const std::string &filePath) const; + + // writes the document to a character stream. Fails only with + // internalError. + Result writeToStream(std::ostream &stream) const; + + // access to the underlying core document for requirements that ScoreData + // does not meet. Prefer the score functions above; the core model is a + // much larger interface and it is not frozen the way mx::api is. + const core::Document &getCoreDocument() const; + + private: + MusicXml(core::DocumentPtr &&coreDocument, bool writeMxVersion); + class Impl; + std::unique_ptr myImpl; + + friend Result getScore(const MusicXml &document); + friend Result fromScore(const ScoreData &score); +}; + +// reads the score out of the document. The document stays alive and can be +// read again or written out. Fails only with internalError. +Result getScore(const MusicXml &document); + +// reads the score out of the document and consumes it: the underlying tree +// is freed when this function returns rather than when your MusicXml binding +// goes out of scope. Pass the document with std::move, or hand over the +// Result's value directly. Fails only with internalError. +Result intoScore(MusicXml document); + +// authors a new document from ScoreData. CAN fail: when the ScoreData +// describes something the core model will not represent (e.g. more than 8 +// beams), the error is returned rather than silently dropping data. +Result fromScore(const ScoreData &score); +} // namespace api +} // namespace mx diff --git a/src/include/mx/api/Result.h b/src/include/mx/api/Result.h index 43d95d9c2..12c7a3dd5 100644 --- a/src/include/mx/api/Result.h +++ b/src/include/mx/api/Result.h @@ -18,7 +18,7 @@ namespace api // The mx::api error vocabulary. mx::api owns its own codes: the core-boundary // failures are mirrored (public headers never // include private mx::core headers), and the api adds the codes core has no -// business knowing. No exceptions escape the DocumentManager boundary. +// business knowing. No exceptions escape the MusicXml boundary. enum class ResultCode { ioError, // file open/read/write failure (api-level) @@ -31,7 +31,6 @@ enum class ResultCode tooManyElements, invalidDocument, unsupportedVersion, // mirrored from the core parse boundary - badDocumentId, // handle not in the registry (api-level) internalError, // caught exception; nothing escapes (api-level) }; diff --git a/src/private/mx/api/DocumentManager.cpp b/src/private/mx/api/MusicXml.cpp similarity index 51% rename from src/private/mx/api/DocumentManager.cpp rename to src/private/mx/api/MusicXml.cpp index 43c830cb0..39c9ee01a 100644 --- a/src/private/mx/api/DocumentManager.cpp +++ b/src/private/mx/api/MusicXml.cpp @@ -2,7 +2,7 @@ // Copyright (c) by Matthew James Briggs // Distributed under the MIT License -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/Attribution.h" #include "mx/core/Error.h" #include "mx/core/generated/Document.h" @@ -13,34 +13,37 @@ #include "pugixml.hpp" -#include -#include #include -#define LOCK_DOCUMENT_MANAGER std::lock_guard lock(myImpl->myMutex); - namespace mx { namespace api { -// A stored document plus the api write directives that the core model cannot -// carry. writeMxVersion governs whether writeTo*() stamps mx's provenance -// (see EncodingData::writeMxVersion); it defaults true, including for -// parsed documents (whose source never had the stamp). -struct StoredDocument -{ - mx::core::DocumentPtr document; - bool writeMxVersion = true; -}; - -using DocumentMap = std::map; - -namespace +// The write side always emits version="4.0" unconditionally: echoing a +// declared "3.0" (or ScoreData::musicXmlVersion) from a 4.0 model was a +// fiction. Enforced here at the write boundary on a copy, so the owned +// document (and the getCoreDocument escape hatch) keeps what was parsed. +core::Document withWriteVersion(const core::Document &document) { + core::Document copy = document; + if (copy.isScorePartwise()) + { + core::ScorePartwise root = copy.asScorePartwise(); + root.setVersion(std::string{"4.0"}); + copy.setRoot(core::Document::Root{std::move(root)}); + } + else + { + core::ScoreTimewise root = copy.asScoreTimewise(); + root.setVersion(std::string{"4.0"}); + copy.setRoot(core::Document::Root{std::move(root)}); + } + return copy; +} // mx::api owns its error vocabulary; the core parse codes are mirrored // one-to-one. -ResultCode mirror(core::ErrorCode code) +ResultCode mirrorToApiResultCode(core::ErrorCode code) { switch (code) { @@ -65,17 +68,17 @@ ResultCode mirror(core::ErrorCode code) } } -ApiError mirror(const core::Error &error) +ApiError mirrorToApiError(const core::Error &error) { - return ApiError{mirror(error.code), error.path, error.message}; + return ApiError{mirrorToApiResultCode(error.code), error.path, error.message}; } -ApiError internalError(const char *function, const std::string &message) +ApiError musicXmlInternalError(const char *function, const std::string &message) { return ApiError{ResultCode::internalError, "", std::string{function} + ": " + message}; } -std::string fileExtension(const std::string &filePath) +std::string musicXmlFileExtension(const std::string &filePath) { const auto dotPos = filePath.find_last_of('.'); if (dotPos == std::string::npos || dotPos == filePath.size() - 1) @@ -85,61 +88,41 @@ std::string fileExtension(const std::string &filePath) return filePath.substr(dotPos + 1); } -// The write side always emits version="4.0" unconditionally: echoing a -// declared "3.0" (or ScoreData::musicXmlVersion) from a 4.0 model was a -// fiction. Enforced here at the write boundary on a copy, so the stored -// document (and the getDocument escape hatch) keeps what was parsed. -core::Document withWriteVersion(const core::Document &stored) +class MusicXml::Impl { - core::Document copy = stored; - if (copy.isScorePartwise()) - { - core::ScorePartwise root = copy.asScorePartwise(); - root.setVersion(std::string{"4.0"}); - copy.setRoot(core::Document::Root{std::move(root)}); - } - else + public: + // writeMxVersion governs whether writeTo*() stamps mx's provenance + // (see EncodingData::writeMxVersion); it defaults true, + // including for parsed documents (whose source never had the stamp). + Impl(core::DocumentPtr inDocument, bool inWriteMxVersion) + : document{std::move(inDocument)}, writeMxVersion{inWriteMxVersion} { - core::ScoreTimewise root = copy.asScoreTimewise(); - root.setVersion(std::string{"4.0"}); - copy.setRoot(core::Document::Root{std::move(root)}); } - return copy; -} -} // namespace + core::DocumentPtr document; + bool writeMxVersion; +}; -class DocumentManager::Impl +MusicXml::MusicXml(core::DocumentPtr &&coreDocument, bool writeMxVersion) + : myImpl{new MusicXml::Impl{std::move(coreDocument), writeMxVersion}} { - public: - std::mutex myMutex; - int myCurrentId; - DocumentMap myMap; - int myCurrentUniqueId; - - // myCurrentUniqueId is seeded high so synthesized ids ("ID1000000"...) are unlikely to - // collide with ids already present in parsed documents (see PartWriter). - Impl() : myMutex{}, myCurrentId{0}, myMap{}, myCurrentUniqueId{1000000} - { - } -}; +} -DocumentManager::DocumentManager() : myImpl{new DocumentManager::Impl()} +MusicXml::MusicXml(MusicXml &&other) noexcept : myImpl{std::move(other.myImpl)} { - myImpl->myCurrentId = 1; } -DocumentManager::~DocumentManager() +MusicXml &MusicXml::operator=(MusicXml &&other) noexcept { + myImpl = std::move(other.myImpl); + return *this; } -DocumentManager &DocumentManager::getInstance() +MusicXml::~MusicXml() { - static DocumentManager instance; - return instance; } -Result DocumentManager::createFromFile(const std::string &filePath) +Result MusicXml::fromFile(const std::string &filePath) { try { @@ -153,7 +136,7 @@ Result DocumentManager::createFromFile(const std::string &filePath) { return ApiError{ResultCode::ioError, filePath, loaded.description()}; } - if (fileExtension(filePath) == "mxl") + if (musicXmlFileExtension(filePath) == "mxl") { std::stringstream ss; ss << "it looks like you are trying to parse a compressed musicxml file, which is currently " @@ -166,26 +149,23 @@ Result DocumentManager::createFromFile(const std::string &filePath) auto parsed = core::parse(xdoc); if (!parsed) { - return mirror(parsed.error()); + return mirrorToApiError(parsed.error()); } - auto mxdoc = std::make_shared(std::move(parsed).value()); - - LOCK_DOCUMENT_MANAGER - myImpl->myMap[myImpl->myCurrentId] = StoredDocument{std::move(mxdoc), true}; - return myImpl->myCurrentId++; + core::DocumentPtr mxdoc = std::make_shared(std::move(parsed).value()); + return MusicXml{std::move(mxdoc), true}; } catch (const std::exception &e) { - return internalError("createFromFile", e.what()); + return musicXmlInternalError("MusicXml::fromFile", e.what()); } catch (...) { - return internalError("createFromFile", "unknown exception"); + return musicXmlInternalError("MusicXml::fromFile", "unknown exception"); } } -Result DocumentManager::createFromStream(std::istream &stream) +Result MusicXml::fromStream(std::istream &stream) { try { @@ -199,77 +179,69 @@ Result DocumentManager::createFromStream(std::istream &stream) auto parsed = core::parse(xdoc); if (!parsed) { - return mirror(parsed.error()); + return mirrorToApiError(parsed.error()); } - auto mxdoc = std::make_shared(std::move(parsed).value()); - - LOCK_DOCUMENT_MANAGER - myImpl->myMap[myImpl->myCurrentId] = StoredDocument{std::move(mxdoc), true}; - return myImpl->myCurrentId++; + core::DocumentPtr mxdoc = std::make_shared(std::move(parsed).value()); + return MusicXml{std::move(mxdoc), true}; } catch (const std::exception &e) { - return internalError("createFromStream", e.what()); + return musicXmlInternalError("MusicXml::fromStream", e.what()); } catch (...) { - return internalError("createFromStream", "unknown exception"); + return musicXmlInternalError("MusicXml::fromStream", "unknown exception"); } } -Result DocumentManager::createFromScore(const ScoreData &score) +Result MusicXml::writeToFile(const std::string &filePath) const { try { - impl::ScoreWriter writer{score}; - core::ScorePartwise scorePartwise = writer.getScorePartwise(); + if (!myImpl) + { + return musicXmlInternalError("MusicXml::writeToFile", "the document has been moved from"); + } - core::DocumentPtr mxdoc; - if (score.musicXmlType == "timewise") + pugi::xml_document xdoc; + const core::Document toWrite = withWriteVersion(*myImpl->document); + if (myImpl->writeMxVersion) { - mxdoc = std::make_shared(impl::partwiseTimewise(scorePartwise)); + core::serializeWithAttribution(toWrite, xdoc); } else { - mxdoc = std::make_shared(std::move(scorePartwise)); + core::serialize(toWrite, xdoc); } - - LOCK_DOCUMENT_MANAGER - myImpl->myMap[myImpl->myCurrentId] = StoredDocument{std::move(mxdoc), score.encoding.writeMxVersion}; - return myImpl->myCurrentId++; - } - catch (const impl::WriteRefusal &refusal) - { - // Refuse, don't drop: the ScoreData describes something the core - // model will not represent. - return refusal.error(); + if (!xdoc.save_file(filePath.c_str(), " ")) + { + return ApiError{ResultCode::ioError, filePath, "writeToFile: could not write the file"}; + } + return Result{}; } catch (const std::exception &e) { - return internalError("createFromScore", e.what()); + return musicXmlInternalError("MusicXml::writeToFile", e.what()); } catch (...) { - return internalError("createFromScore", "unknown exception"); + return musicXmlInternalError("MusicXml::writeToFile", "unknown exception"); } } -Result DocumentManager::writeToFile(int documentId, const std::string &filePath) const +Result MusicXml::writeToStream(std::ostream &stream) const { try { - LOCK_DOCUMENT_MANAGER - - const DocumentMap::const_iterator it = myImpl->myMap.find(documentId); - if (it == myImpl->myMap.cend()) + if (!myImpl) { - return ApiError{ResultCode::badDocumentId, "", "writeToFile: bad document id"}; + return musicXmlInternalError("MusicXml::writeToStream", "the document has been moved from"); } pugi::xml_document xdoc; - const core::Document toWrite = withWriteVersion(*it->second.document); - if (it->second.writeMxVersion) + const core::Document toWrite = withWriteVersion(*myImpl->document); + if (myImpl->writeMxVersion) { core::serializeWithAttribution(toWrite, xdoc); } @@ -277,126 +249,106 @@ Result DocumentManager::writeToFile(int documentId, const std::string &fil { core::serialize(toWrite, xdoc); } - if (!xdoc.save_file(filePath.c_str(), " ")) - { - return ApiError{ResultCode::ioError, filePath, "writeToFile: could not write the file"}; - } + xdoc.save(stream, " "); return Result{}; } catch (const std::exception &e) { - return internalError("writeToFile", e.what()); + return musicXmlInternalError("MusicXml::writeToStream", e.what()); } catch (...) { - return internalError("writeToFile", "unknown exception"); + return musicXmlInternalError("MusicXml::writeToStream", "unknown exception"); } } -Result DocumentManager::writeToStream(int documentId, std::ostream &stream) const +const core::Document &MusicXml::getCoreDocument() const { - try + if (myImpl) { - LOCK_DOCUMENT_MANAGER + return *myImpl->document; + } + // a moved-from document holds nothing; reading it yields an empty core + // document rather than a crash + static const core::Document emptyDocument{}; + return emptyDocument; +} - const DocumentMap::const_iterator it = myImpl->myMap.find(documentId); - if (it == myImpl->myMap.cend()) - { - return ApiError{ResultCode::badDocumentId, "", "writeToStream: bad document id"}; - } +Result fromScore(const ScoreData &score) +{ + try + { + impl::ScoreWriter writer{score}; + core::ScorePartwise scorePartwise = writer.getScorePartwise(); - pugi::xml_document xdoc; - const core::Document toWrite = withWriteVersion(*it->second.document); - if (it->second.writeMxVersion) + core::DocumentPtr mxdoc; + if (score.musicXmlType == "timewise") { - core::serializeWithAttribution(toWrite, xdoc); + mxdoc = std::make_shared(impl::partwiseTimewise(scorePartwise)); } else { - core::serialize(toWrite, xdoc); + mxdoc = std::make_shared(std::move(scorePartwise)); } - xdoc.save(stream, " "); - return Result{}; + + return MusicXml{std::move(mxdoc), score.encoding.writeMxVersion}; + } + catch (const impl::WriteRefusal &refusal) + { + // Refuse, don't drop: the ScoreData describes something the core + // model will not represent. + return refusal.error(); } catch (const std::exception &e) { - return internalError("writeToStream", e.what()); + return musicXmlInternalError("fromScore", e.what()); } catch (...) { - return internalError("writeToStream", "unknown exception"); + return musicXmlInternalError("fromScore", "unknown exception"); } } -Result DocumentManager::getData(int documentId) const +Result getScore(const MusicXml &document) { try { - LOCK_DOCUMENT_MANAGER - - const DocumentMap::const_iterator it = myImpl->myMap.find(documentId); - if (it == myImpl->myMap.cend()) + if (!document.myImpl) { - return ApiError{ResultCode::badDocumentId, "", "getData: bad document id"}; + return musicXmlInternalError("getScore", "the document has been moved from"); } - // Convert into a local partwise copy and read that; the stored - // document is untouched. The old mutate-and-restore dance (convert - // the stored document to partwise, read, convert back) is gone. - if (it->second.document->isScoreTimewise()) + const core::Document &coreDocument = *document.myImpl->document; + + // Convert a timewise document into a local partwise copy and read + // that; the owned document is untouched. + if (coreDocument.isScoreTimewise()) { - const core::ScorePartwise scorePartwise = impl::timewisePartwise(it->second.document->asScoreTimewise()); + const core::ScorePartwise scorePartwise = impl::timewisePartwise(coreDocument.asScoreTimewise()); impl::ScoreReader reader{scorePartwise}; auto score = reader.getScoreData(); score.musicXmlType = "timewise"; return score; } - impl::ScoreReader reader{it->second.document->asScorePartwise()}; + impl::ScoreReader reader{coreDocument.asScorePartwise()}; return reader.getScoreData(); } catch (const std::exception &e) { - return internalError("getData", e.what()); + return musicXmlInternalError("getScore", e.what()); } catch (...) { - return internalError("getData", "unknown exception"); + return musicXmlInternalError("getScore", "unknown exception"); } } -void DocumentManager::destroyDocument(int documentId) -{ - LOCK_DOCUMENT_MANAGER - const DocumentMap::const_iterator it = myImpl->myMap.find(documentId); - - if (it == myImpl->myMap.cend()) - { - return; - } - - myImpl->myMap.erase(it); -} - -mx::core::DocumentPtr DocumentManager::getDocument(int documentId) const -{ - LOCK_DOCUMENT_MANAGER - const DocumentMap::const_iterator it = myImpl->myMap.find(documentId); - - if (it == myImpl->myMap.cend()) - { - return mx::core::DocumentPtr{}; - } - - return it->second.document; -} - -int DocumentManager::getUniqueId() +Result intoScore(MusicXml document) { - LOCK_DOCUMENT_MANAGER - int returnValue = myImpl->myCurrentUniqueId; - ++myImpl->myCurrentUniqueId; - return returnValue; + // the parameter owns the document; its destructor frees the underlying + // tree when this function returns + return getScore(document); } } // namespace api } // namespace mx diff --git a/src/private/mx/examples/Hide.cpp b/src/private/mx/examples/Hide.cpp index deda223bf..deab8e075 100644 --- a/src/private/mx/examples/Hide.cpp +++ b/src/private/mx/examples/Hide.cpp @@ -20,7 +20,6 @@ #include "mx/api/CurveData.h" #include "mx/api/DefaultsData.h" #include "mx/api/DirectionData.h" -#include "mx/api/DocumentManager.h" #include "mx/api/DurationData.h" #include "mx/api/EncodingData.h" #include "mx/api/FontData.h" @@ -31,6 +30,7 @@ #include "mx/api/MeasureData.h" #include "mx/api/MeasureLocation.h" #include "mx/api/MiscData.h" +#include "mx/api/MusicXml.h" #include "mx/api/NoteAttachmentData.h" #include "mx/api/NoteData.h" #include "mx/api/OttavaData.h" diff --git a/src/private/mx/examples/IterateNotesAndTiming.cpp b/src/private/mx/examples/IterateNotesAndTiming.cpp index 9d461a21e..1018e22e4 100644 --- a/src/private/mx/examples/IterateNotesAndTiming.cpp +++ b/src/private/mx/examples/IterateNotesAndTiming.cpp @@ -25,7 +25,7 @@ #include #include -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #define MX_IS_A_SUCCESS 0 @@ -194,18 +194,15 @@ int main(int argc, const char *argv[]) { using namespace mx::api; - auto &mgr = DocumentManager::getInstance(); std::istringstream istr{xml}; - const auto idResult = mgr.createFromStream(istr); - if (!idResult.ok()) + auto docResult = MusicXml::fromStream(istr); + if (!docResult.ok()) { return MX_IS_A_FAILURE; } - const auto documentID = idResult.value(); - const auto scoreResult = mgr.getData(documentID); - mgr.destroyDocument(documentID); + const auto scoreResult = intoScore(std::move(docResult).value()); if (!scoreResult.ok()) { return MX_IS_A_FAILURE; diff --git a/src/private/mx/examples/Read.cpp b/src/private/mx/examples/Read.cpp index 6826002ae..15ffbfed1 100644 --- a/src/private/mx/examples/Read.cpp +++ b/src/private/mx/examples/Read.cpp @@ -3,7 +3,7 @@ #include #include -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #define MX_IS_A_SUCCESS 0 @@ -53,32 +53,25 @@ int main(int argc, const char *argv[]) { using namespace mx::api; - // create a reference to the singleton which holds documents in memory for us - auto &mgr = DocumentManager::getInstance(); - // place the xml from above into a stream object std::istringstream istr{xml}; - // ask the document manager to parse the xml into memory for us, returns a document ID. - const auto idResult = mgr.createFromStream(istr); - if (!idResult.ok()) + // parse the xml into a MusicXml document that we own + auto docResult = MusicXml::fromStream(istr); + if (!docResult.ok()) { return MX_IS_A_FAILURE; } - const auto documentID = idResult.value(); - // get the structural representation of the score from the document manager - const auto scoreResult = mgr.getData(documentID); + // take the score out of the document. intoScore also consumes the + // document, so its memory is freed as the function returns + const auto scoreResult = intoScore(std::move(docResult).value()); if (!scoreResult.ok()) { - mgr.destroyDocument(documentID); return MX_IS_A_FAILURE; } const auto score = scoreResult.value(); - // we need to explicitly destroy the document from memory - mgr.destroyDocument(documentID); - // make sure we have exactly one part if (score.parts.size() != 1) { diff --git a/src/private/mx/examples/Write.cpp b/src/private/mx/examples/Write.cpp index 12c682dfb..5877516f9 100644 --- a/src/private/mx/examples/Write.cpp +++ b/src/private/mx/examples/Write.cpp @@ -3,7 +3,7 @@ #include #include -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" // set this to 1 if you want to see the xml in your console @@ -108,19 +108,18 @@ int main(int argc, const char *argv[]) note.beams.clear(); voice.notes.push_back(note); - // the document manager is the liaison between our score data and the MusicXML DOM. - // it completely hides the MusicXML DOM from us when using mx::api - auto &mgr = DocumentManager::getInstance(); - const auto idResult = mgr.createFromScore(score); - if (!idResult.ok()) + // a MusicXml document is created from the score data and owns the + // underlying MusicXML model, which is hidden from us when using mx::api + auto docResult = fromScore(score); + if (!docResult.ok()) { return 1; } - const auto documentID = idResult.value(); + const auto document = std::move(docResult).value(); // write to the console #if MX_WRITE_THIS_TO_THE_CONSOLE - (void)mgr.writeToStream(documentID, std::cout); + (void)document.writeToStream(std::cout); std::cout << std::endl; #endif @@ -128,10 +127,7 @@ int main(int argc, const char *argv[]) // system can send the file to a gitignored location during automated runs; // see issue #150. const std::string outputPath = (argc > 1) ? argv[1] : "./example.musicxml"; - const auto writeResult = mgr.writeToFile(documentID, outputPath); - - // we need to explicitly delete the object held by the manager - mgr.destroyDocument(documentID); + const auto writeResult = document.writeToFile(outputPath); return writeResult.ok() ? 0 : 1; } \ No newline at end of file diff --git a/src/private/mx/impl/PartWriter.cpp b/src/private/mx/impl/PartWriter.cpp index cec80b887..f8c9edac5 100644 --- a/src/private/mx/impl/PartWriter.cpp +++ b/src/private/mx/impl/PartWriter.cpp @@ -3,7 +3,6 @@ // Distributed under the MIT License #include "mx/impl/PartWriter.h" -#include "mx/api/DocumentManager.h" #include "mx/core/Decimal.h" #include "mx/core/OneOrMore.h" #include "mx/core/Token.h" @@ -36,6 +35,7 @@ #include "mx/impl/NameDisplayFunctions.h" #include "mx/impl/ScoreWriter.h" +#include #include namespace mx @@ -57,6 +57,16 @@ void applyPrintObject(api::Bool printObject, core::PartName &out) } } // namespace +// Synthesized ids, e.g. "ID1000000". Seeded high so they +// are unlikely to collide with ids already present in parsed documents. The +// sequence is shared process-wide so that instruments of different parts +// cannot collide inside one document. +int partWriterNextSynthesizedId() +{ + static std::atomic nextId{1000000}; + return nextId.fetch_add(1); +} + PartWriter::PartWriter(const api::PartData &inPartData, int inPartIndex, int inTicksPerQuarter, const ScoreWriter &inScoreWriter) : myPartData{inPartData}, myPartIndex{inPartIndex}, myTicksPerQuarter{inTicksPerQuarter}, myMutex{}, @@ -253,7 +263,7 @@ core::ScorePart PartWriter::getScorePart() const { std::stringstream ss; ss << "ID"; - ss << api::DocumentManager::getInstance().getUniqueId(); + ss << partWriterNextSynthesizedId(); scoreInstrument.setID(core::Token{ss.str()}); } else diff --git a/src/private/mx/impl/ScoreConversions.h b/src/private/mx/impl/ScoreConversions.h index dd529f276..19165debf 100644 --- a/src/private/mx/impl/ScoreConversions.h +++ b/src/private/mx/impl/ScoreConversions.h @@ -13,7 +13,7 @@ namespace impl { // The timewise <-> partwise pivot: a regroup of parts-of-measures -// <-> measures-of-parts plus header. Its only consumer is DocumentManager. +// <-> measures-of-parts plus header. Its only consumer is the api boundary (MusicXml). // Under value semantics the old shallow copies become real copies -- // strictly safer, behavior-identical for this use. diff --git a/src/private/mx/impl/ScoreWriter.cpp b/src/private/mx/impl/ScoreWriter.cpp index d179360ad..e4ef84e9c 100644 --- a/src/private/mx/impl/ScoreWriter.cpp +++ b/src/private/mx/impl/ScoreWriter.cpp @@ -57,8 +57,8 @@ core::ScorePartwise ScoreWriter::getScorePartwise() const break; // ThreePointZero also represents parsed "4.0" documents (see ScoreReader). The "3.0" - // written here only reaches the getDocument escape hatch -- writeTo*() overrides the - // version to "4.0" via DocumentManager's withWriteVersion. + // written here only reaches the getCoreDocument escape hatch -- writeTo*() overrides the + // version to "4.0" via withWriteVersion at the api boundary. case api::MusicXmlVersion::ThreePointZero: { myOutScorePartwise.setVersion(std::string{"3.0"}); } diff --git a/src/private/mx/impl/WriteRefusal.h b/src/private/mx/impl/WriteRefusal.h index 02f787ca3..59bf2f2e4 100644 --- a/src/private/mx/impl/WriteRefusal.h +++ b/src/private/mx/impl/WriteRefusal.h @@ -14,12 +14,11 @@ namespace mx namespace impl { -// Internal to the impl layer (createFromScore): when a ScoreData describes +// Internal to the impl layer (fromScore): when a ScoreData describes // something the new core won't represent (e.g. a // ninth beam against the bounded addBeam), the writer refuses rather than -// silently dropping user data. Writers throw this; DocumentManager catches -// it at the api boundary and returns the carried ApiError. Never escapes -// mx::api. +// silently dropping user data. Writers throw this; the api boundary catches +// it and returns the carried ApiError. Never escapes mx::api. class WriteRefusal : public std::exception { public: diff --git a/src/private/mxtest/api/ApiChordSimpleTest.cpp b/src/private/mxtest/api/ApiChordSimpleTest.cpp index b3393c647..663215e3e 100644 --- a/src/private/mxtest/api/ApiChordSimpleTest.cpp +++ b/src/private/mxtest/api/ApiChordSimpleTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/ApiChordSimpleScoreData.h" #include "mxtest/api/ApiTester.h" diff --git a/src/private/mxtest/api/ApiK007aTest.cpp b/src/private/mxtest/api/ApiK007aTest.cpp index 8e7d64262..f0fb4f131 100644 --- a/src/private/mxtest/api/ApiK007aTest.cpp +++ b/src/private/mxtest/api/ApiK007aTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/ApiK007aScoreData.h" #include "mxtest/api/ApiTester.h" diff --git a/src/private/mxtest/api/ApiK007cTest.cpp b/src/private/mxtest/api/ApiK007cTest.cpp index 7502afd4f..9477dafe1 100644 --- a/src/private/mxtest/api/ApiK007cTest.cpp +++ b/src/private/mxtest/api/ApiK007cTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/ApiK007cScoreData.h" #include "mxtest/api/ApiTester.h" diff --git a/src/private/mxtest/api/ApiK009bSlurTest.cpp b/src/private/mxtest/api/ApiK009bSlurTest.cpp index 11383bd80..3760577d2 100644 --- a/src/private/mxtest/api/ApiK009bSlurTest.cpp +++ b/src/private/mxtest/api/ApiK009bSlurTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/ApiK009bSlurScoreData.h" #include "mxtest/api/ApiTester.h" diff --git a/src/private/mxtest/api/ApiK014aFermatasTest.cpp b/src/private/mxtest/api/ApiK014aFermatasTest.cpp index 7cf6ea169..e6fc927ad 100644 --- a/src/private/mxtest/api/ApiK014aFermatasTest.cpp +++ b/src/private/mxtest/api/ApiK014aFermatasTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/ApiK014aFermatasScoreData.h" #include "mxtest/api/ApiTester.h" diff --git a/src/private/mxtest/api/ApiK015aLayoutTest.cpp b/src/private/mxtest/api/ApiK015aLayoutTest.cpp index 810e34b26..c9a93bb07 100644 --- a/src/private/mxtest/api/ApiK015aLayoutTest.cpp +++ b/src/private/mxtest/api/ApiK015aLayoutTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/ApiK015aLayoutScoreData.h" #include "mxtest/api/ApiTester.h" diff --git a/src/private/mxtest/api/ApiK016aMiscTest.cpp b/src/private/mxtest/api/ApiK016aMiscTest.cpp index 68a647b51..f97db4757 100644 --- a/src/private/mxtest/api/ApiK016aMiscTest.cpp +++ b/src/private/mxtest/api/ApiK016aMiscTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/ApiK016aMiscScoreData.h" #include "mxtest/api/ApiTester.h" diff --git a/src/private/mxtest/api/ApiLoadSmokeTest.cpp b/src/private/mxtest/api/ApiLoadSmokeTest.cpp index feba0bf3d..dfd72ffe7 100644 --- a/src/private/mxtest/api/ApiLoadSmokeTest.cpp +++ b/src/private/mxtest/api/ApiLoadSmokeTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/file/MxFileTest.h" #include "mxtest/file/MxFileTestGroup.h" @@ -32,21 +32,18 @@ class ApiLoadSmokeTest : public mxtest::MxFileTest setIsSuccess(true); return; } - auto &docMgr = mx::api::DocumentManager::getInstance(); - const auto docIdResult = docMgr.createFromFile(testFilePath()); - if (!docIdResult.ok()) + auto docResult = mx::api::MusicXml::fromFile(testFilePath()); + if (!docResult.ok()) { setIsSuccess(false); - setFailureMessage("docMgr.createFromFile failed: " + docIdResult.error().message); + setFailureMessage("MusicXml::fromFile failed: " + docResult.error().message); return; } - const int docId = docIdResult.value(); - const auto scoreDataResult = docMgr.getData(docId); - docMgr.destroyDocument(docId); + const auto scoreDataResult = mx::api::getScore(std::move(docResult).value()); if (!scoreDataResult.ok()) { setIsSuccess(false); - setFailureMessage("docMgr.getData failed: " + scoreDataResult.error().message); + setFailureMessage("getScore failed: " + scoreDataResult.error().message); return; } bool isSuccess = scoreDataResult.value().parts.size() > 0; diff --git a/src/private/mxtest/api/ApiLy43eTest.cpp b/src/private/mxtest/api/ApiLy43eTest.cpp index 69ec8ab1d..8aa5e6062 100644 --- a/src/private/mxtest/api/ApiLy43eTest.cpp +++ b/src/private/mxtest/api/ApiLy43eTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/ApiLy43eScoreData.h" #include "mxtest/api/ApiTester.h" diff --git a/src/private/mxtest/api/ApiMuAccidentals1Test.cpp b/src/private/mxtest/api/ApiMuAccidentals1Test.cpp index a7ed8aab8..038a34525 100644 --- a/src/private/mxtest/api/ApiMuAccidentals1Test.cpp +++ b/src/private/mxtest/api/ApiMuAccidentals1Test.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/ApiMuAccidentals1ScoreData.h" #include "mxtest/api/ApiTester.h" diff --git a/src/private/mxtest/api/ApiTester.cpp b/src/private/mxtest/api/ApiTester.cpp index 91f06485a..7eb3f1f13 100644 --- a/src/private/mxtest/api/ApiTester.cpp +++ b/src/private/mxtest/api/ApiTester.cpp @@ -1,6 +1,9 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License #include "mxtest/api/ApiTester.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/file/StupidFileFunctions.h" #include "pugixml.hpp" @@ -39,58 +42,49 @@ void ApiTester::runTestCode() const auto expectedScoreData = myScoreDataCreator->createScoreData(); // load the data from disk - auto &docMgr = DocumentManager::getInstance(); - const auto initialLoadDocIdResult = docMgr.createFromFile(testFilePath()); - if (!initialLoadDocIdResult.ok()) + auto initialLoadDocResult = MusicXml::fromFile(testFilePath()); + if (!initialLoadDocResult.ok()) { setIsSuccess(false); - setFailureMessage("createFromFile failed: " + initialLoadDocIdResult.error().message); + setFailureMessage("fromFile failed: " + initialLoadDocResult.error().message); return; } - const int initialLoadDocId = initialLoadDocIdResult.value(); - const auto initialLoadScoreDataResult = docMgr.getData(initialLoadDocId); + MusicXml initialLoadDoc = std::move(initialLoadDocResult).value(); + const auto initialLoadScoreDataResult = getScore(initialLoadDoc); if (!initialLoadScoreDataResult.ok()) { - docMgr.destroyDocument(initialLoadDocId); setIsSuccess(false); - setFailureMessage("getData failed: " + initialLoadScoreDataResult.error().message); + setFailureMessage("getScore failed: " + initialLoadScoreDataResult.error().message); return; } const auto initialLoadScoreData = initialLoadScoreDataResult.value(); // save what we loaded back to disk - const auto initialScoreDataDocIdResult = docMgr.createFromScore(initialLoadScoreData); - if (!initialScoreDataDocIdResult.ok()) + auto initialScoreDataDocResult = fromScore(initialLoadScoreData); + if (!initialScoreDataDocResult.ok()) { - docMgr.destroyDocument(initialLoadDocId); setIsSuccess(false); - setFailureMessage("createFromScore failed"); + setFailureMessage("fromScore failed"); return; } - const int initialScoreDataDocId = initialScoreDataDocIdResult.value(); + const MusicXml initialScoreDataDoc = std::move(initialScoreDataDocResult).value(); // save the 'intermediate' ScoreData - docMgr.writeToFile(initialScoreDataDocId, myIntermediateFilePath); + initialScoreDataDoc.writeToFile(myIntermediateFilePath); - // load what what we just saved back up into memory - const auto intermediateFileLoadDocIdResult = docMgr.createFromFile(myIntermediateFilePath); - if (!intermediateFileLoadDocIdResult.ok()) + // load what we just saved back into memory + auto intermediateFileLoadDocResult = MusicXml::fromFile(myIntermediateFilePath); + if (!intermediateFileLoadDocResult.ok()) { - docMgr.destroyDocument(initialLoadDocId); - docMgr.destroyDocument(initialScoreDataDocId); setIsSuccess(false); - setFailureMessage("createFromFile(intermediate) failed"); + setFailureMessage("fromFile(intermediate) failed"); return; } - const int intermediateFileLoadDocId = intermediateFileLoadDocIdResult.value(); - const auto actualScoreDataResult = docMgr.getData(intermediateFileLoadDocId); + const auto actualScoreDataResult = getScore(std::move(intermediateFileLoadDocResult).value()); if (!actualScoreDataResult.ok()) { - docMgr.destroyDocument(initialLoadDocId); - docMgr.destroyDocument(initialScoreDataDocId); - docMgr.destroyDocument(intermediateFileLoadDocId); setIsSuccess(false); - setFailureMessage("getData(intermediate) failed"); + setFailureMessage("getScore(intermediate) failed"); return; } const auto actualScoreData = actualScoreDataResult.value(); @@ -100,9 +94,6 @@ void ApiTester::runTestCode() { // test was successful, return without registering a failure setIsSuccess(true); - docMgr.destroyDocument(initialLoadDocId); - docMgr.destroyDocument(initialScoreDataDocId); - docMgr.destroyDocument(intermediateFileLoadDocId); deleteFiles(); return; } @@ -119,26 +110,18 @@ void ApiTester::runTestCode() } // save the 'expected' ScoreData - const auto expectedScoreDataDocIdResult = docMgr.createFromScore(expectedScoreData); - if (expectedScoreDataDocIdResult.ok()) + auto expectedScoreDataDocResult = fromScore(expectedScoreData); + if (expectedScoreDataDocResult.ok()) { - const int expectedScoreDataDocId = expectedScoreDataDocIdResult.value(); - docMgr.writeToFile(expectedScoreDataDocId, myExpectedFilePath); - docMgr.destroyDocument(expectedScoreDataDocId); + std::move(expectedScoreDataDocResult).value().writeToFile(myExpectedFilePath); } // save the 'actual' ScoreData - const auto finalDocIdResult = docMgr.createFromScore(actualScoreData); - if (finalDocIdResult.ok()) + auto finalDocResult = fromScore(actualScoreData); + if (finalDocResult.ok()) { - const int finalDocId = finalDocIdResult.value(); - docMgr.writeToFile(finalDocId, myFinalFilePath); - docMgr.destroyDocument(finalDocId); + std::move(finalDocResult).value().writeToFile(myFinalFilePath); } - - docMgr.destroyDocument(initialLoadDocId); - docMgr.destroyDocument(initialScoreDataDocId); - docMgr.destroyDocument(intermediateFileLoadDocId); } void ApiTester::deleteFiles() const diff --git a/src/private/mxtest/api/BombeTest.cpp b/src/private/mxtest/api/BombeTest.cpp index 985ece92c..44907b2e5 100644 --- a/src/private/mxtest/api/BombeTest.cpp +++ b/src/private/mxtest/api/BombeTest.cpp @@ -7,7 +7,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" using namespace std; @@ -20,13 +20,10 @@ inline ScoreData getBombe() { const std::string fileName{fname}; const std::string path{MxFileRepository::getFullPath(fileName)}; - auto &docMgr = mx::api::DocumentManager::getInstance(); - const auto docIdResult = docMgr.createFromFile(path); + auto docIdResult = mx::api::MusicXml::fromFile(path); if (!docIdResult.ok()) return {}; - const int docId = docIdResult.value(); - const auto scoreDataResult = docMgr.getData(docId); - docMgr.destroyDocument(docId); + const auto scoreDataResult = mx::api::getScore(std::move(docIdResult).value()); if (!scoreDataResult.ok()) return {}; return scoreDataResult.value(); diff --git a/src/private/mxtest/api/ChordApiTest.cpp b/src/private/mxtest/api/ChordApiTest.cpp index 74fdc859a..07626a3eb 100644 --- a/src/private/mxtest/api/ChordApiTest.cpp +++ b/src/private/mxtest/api/ChordApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/FullNoteGroup.h" #include "mx/core/generated/MusicDataChoice.h" @@ -106,12 +106,9 @@ T_END TEST(chordSaveNotes, ChordApi) { const auto originalData = mxtest::MxFileRepository::loadFile(fileName); - auto &docMgr = DocumentManager::getInstance(); - const auto savedDocIdResult = docMgr.createFromScore(originalData); + auto savedDocIdResult = fromScore(originalData); REQUIRE(savedDocIdResult.ok()); - const int savedDocId = savedDocIdResult.value(); - const auto scoreDataResult = docMgr.getData(savedDocId); - docMgr.destroyDocument(savedDocId); + const auto scoreDataResult = getScore(std::move(savedDocIdResult).value()); REQUIRE(scoreDataResult.ok()); const auto &scoreData = scoreDataResult.value(); @@ -245,15 +242,12 @@ TEST(KompChordBug_PIVOTAL_147058063, ChordApi) note.isChord = true; originalStaffPtr->voices[0].notes.push_back(note); - auto &docMgr = DocumentManager::getInstance(); - const auto docIdResult = docMgr.createFromScore(originalScore); + auto docIdResult = fromScore(originalScore); REQUIRE(docIdResult.ok()); - const int docID = docIdResult.value(); - const auto documentPtr = docMgr.getDocument(docID); - docMgr.destroyDocument(docID); - REQUIRE(documentPtr != nullptr); - REQUIRE(documentPtr->isScorePartwise()); - const auto &scorePartwise = documentPtr->asScorePartwise(); + const auto document = std::move(docIdResult).value(); + const auto &coreDoc = document.getCoreDocument(); + REQUIRE(coreDoc.isScorePartwise()); + const auto &scorePartwise = coreDoc.asScorePartwise(); const auto xml = mxtest::toXml(originalScore); const auto savedScore = mxtest::fromXml(xml); const auto &savedPart = savedScore.parts.at(0); diff --git a/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp b/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp index 733dbc828..ee804dbde 100644 --- a/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp +++ b/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/generated/Bass.h" #include "mx/core/generated/BassStep.h" #include "mx/core/generated/Document.h" @@ -38,15 +38,12 @@ using namespace mxtest; TEST(Save, ChordDataSaveTest) { const auto scoreData = apiChordSimpleScoreData(); - auto &mgr = DocumentManager::getInstance(); - const auto docIdResult = mgr.createFromScore(scoreData); + auto docIdResult = fromScore(scoreData); REQUIRE(docIdResult.ok()); - const int docId = docIdResult.value(); - const auto documentPtr = mgr.getDocument(docId); - mgr.destroyDocument(docId); - REQUIRE(documentPtr != nullptr); - REQUIRE(documentPtr->isScorePartwise()); - const auto &scorePartwise = documentPtr->asScorePartwise(); + const auto document = std::move(docIdResult).value(); + const auto &coreDoc = document.getCoreDocument(); + REQUIRE(coreDoc.isScorePartwise()); + const auto &scorePartwise = coreDoc.asScorePartwise(); const auto partwiseParts = scorePartwise.part(); REQUIRE(!partwiseParts.empty()); diff --git a/src/private/mxtest/api/ChordTimeTest.cpp b/src/private/mxtest/api/ChordTimeTest.cpp index 592790c9c..652013303 100644 --- a/src/private/mxtest/api/ChordTimeTest.cpp +++ b/src/private/mxtest/api/ChordTimeTest.cpp @@ -7,7 +7,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/FullNoteGroup.h" #include "mx/core/generated/MusicDataChoice.h" @@ -74,18 +74,14 @@ TEST(chordTest, Chords) noteP->durationData.durationTimeTicks = 120; // noteP->beams.emplace_back(Beam::end); - auto &mgr = DocumentManager::getInstance(); - const auto docIdResult = mgr.createFromScore(score); + auto docIdResult = fromScore(score); REQUIRE(docIdResult.ok()); - const int docId = docIdResult.value(); + const auto doc = std::move(docIdResult).value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - auto doc = mgr.getDocument(docId); - mgr.destroyDocument(docId); + doc.writeToStream(ss); - REQUIRE(doc != nullptr); - REQUIRE(doc->isScorePartwise()); - const auto &scorePartwise = doc->asScorePartwise(); + REQUIRE(doc.getCoreDocument().isScorePartwise()); + const auto &scorePartwise = doc.getCoreDocument().asScorePartwise(); const auto parts = scorePartwise.part(); REQUIRE(!parts.empty()); const auto &firstPart = parts[0]; diff --git a/src/private/mxtest/api/CorpusRoundtripMain.cpp b/src/private/mxtest/api/CorpusRoundtripMain.cpp index 21b40ba46..052538de0 100644 --- a/src/private/mxtest/api/CorpusRoundtripMain.cpp +++ b/src/private/mxtest/api/CorpusRoundtripMain.cpp @@ -20,7 +20,7 @@ // Print one line per file: PASS|FAIL|SKIPrelpathdetail. // Exit 0 always. Use to grow the pinned list. -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/Attribution.h" #include "mxtest/corert/Compare.h" #include "mxtest/corert/Fixer.h" @@ -386,21 +386,17 @@ RoundtripResult runRoundtrip(const std::string &absolutePath) { RoundtripResult r; - auto &mgr = mx::api::DocumentManager::getInstance(); - // Load via the api - const auto idResult = mgr.createFromFile(absolutePath); - if (!idResult.ok()) + auto docResult = mx::api::MusicXml::fromFile(absolutePath); + if (!docResult.ok()) { r.status = RoundtripResult::Status::loadFail; - r.detail = idResult.error().message; + r.detail = docResult.error().message; return r; } - const int docId = idResult.value(); // Get score data - const auto scoreResult = mgr.getData(docId); - mgr.destroyDocument(docId); + const auto scoreResult = mx::api::getScore(std::move(docResult).value()); if (!scoreResult.ok()) { r.status = RoundtripResult::Status::getDataFail; @@ -409,19 +405,17 @@ RoundtripResult runRoundtrip(const std::string &absolutePath) } // Re-create from ScoreData - const auto id2Result = mgr.createFromScore(scoreResult.value()); - if (!id2Result.ok()) + auto doc2Result = mx::api::fromScore(scoreResult.value()); + if (!doc2Result.ok()) { r.status = RoundtripResult::Status::createFail; - r.detail = id2Result.error().message; + r.detail = doc2Result.error().message; return r; } - const int docId2 = id2Result.value(); // Write to string std::ostringstream ss; - const auto writeResult = mgr.writeToStream(docId2, ss); - mgr.destroyDocument(docId2); + const auto writeResult = std::move(doc2Result).value().writeToStream(ss); if (!writeResult.ok()) { r.status = RoundtripResult::Status::fail; @@ -581,34 +575,29 @@ void dumpDocuments(const std::string &absolutePath, const std::string &relPath, return; } - // Actual side: re-run the api pipeline (load -> getData -> createFromScore + // Actual side: re-run the api pipeline (load -> getScore -> fromScore // -> writeToStream), then normalize. The provenance stamp is kept (the // expected side has it added to match). Mirrors runRoundtrip(). - auto &mgr = mx::api::DocumentManager::getInstance(); - const auto idResult = mgr.createFromFile(absolutePath); + auto idResult = mx::api::MusicXml::fromFile(absolutePath); if (!idResult.ok()) { - std::cerr << "dump: no actual for " << relPath << " (createFromFile failed)\n"; + std::cerr << "dump: no actual for " << relPath << " (fromFile failed)\n"; return; } - const int docId = idResult.value(); - const auto scoreResult = mgr.getData(docId); - mgr.destroyDocument(docId); + const auto scoreResult = mx::api::getScore(std::move(idResult).value()); if (!scoreResult.ok()) { - std::cerr << "dump: no actual for " << relPath << " (getData failed)\n"; + std::cerr << "dump: no actual for " << relPath << " (getScore failed)\n"; return; } - const auto id2Result = mgr.createFromScore(scoreResult.value()); + auto id2Result = mx::api::fromScore(scoreResult.value()); if (!id2Result.ok()) { - std::cerr << "dump: no actual for " << relPath << " (createFromScore failed)\n"; + std::cerr << "dump: no actual for " << relPath << " (fromScore failed)\n"; return; } - const int docId2 = id2Result.value(); std::ostringstream ss; - const auto writeResult = mgr.writeToStream(docId2, ss); - mgr.destroyDocument(docId2); + const auto writeResult = std::move(id2Result).value().writeToStream(ss); if (!writeResult.ok()) { std::cerr << "dump: no actual for " << relPath << " (writeToStream failed)\n"; diff --git a/src/private/mxtest/api/CreditRoundTripTest.cpp b/src/private/mxtest/api/CreditRoundTripTest.cpp index 54fb09af4..a0eb66ca3 100644 --- a/src/private/mxtest/api/CreditRoundTripTest.cpp +++ b/src/private/mxtest/api/CreditRoundTripTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/CrossStaffApiTest.cpp b/src/private/mxtest/api/CrossStaffApiTest.cpp index e43622fbf..fc0ae2823 100644 --- a/src/private/mxtest/api/CrossStaffApiTest.cpp +++ b/src/private/mxtest/api/CrossStaffApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/DefaultsFontsRoundTripTest.cpp b/src/private/mxtest/api/DefaultsFontsRoundTripTest.cpp index db5b2fbf4..699ebe4ab 100644 --- a/src/private/mxtest/api/DefaultsFontsRoundTripTest.cpp +++ b/src/private/mxtest/api/DefaultsFontsRoundTripTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" using namespace std; diff --git a/src/private/mxtest/api/DirectionDataTest.cpp b/src/private/mxtest/api/DirectionDataTest.cpp index e34f523a7..c6269d050 100644 --- a/src/private/mxtest/api/DirectionDataTest.cpp +++ b/src/private/mxtest/api/DirectionDataTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/OttavaData.h" #include "mx/api/RehearsalData.h" #include "mx/api/ScoreData.h" @@ -246,12 +246,9 @@ T_END; TEST(RehearsalSyntheticFileRead, DirectionData) { const std::string path = mxtest::getResourcesDirectoryPath() + "synthetic/rehearsal.3.1.xml"; - auto &docMgr = DocumentManager::getInstance(); - const auto docIdResult = docMgr.createFromFile(path); + auto docIdResult = MusicXml::fromFile(path); REQUIRE(docIdResult.ok()); - const int docId = docIdResult.value(); - const auto scoreResult = docMgr.getData(docId); - docMgr.destroyDocument(docId); + const auto scoreResult = getScore(std::move(docIdResult).value()); REQUIRE(scoreResult.ok()); const auto &score = scoreResult.value(); REQUIRE(score.parts.size() == 1); diff --git a/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp b/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp index 7da61896c..cfb709875 100644 --- a/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp +++ b/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include @@ -27,23 +27,18 @@ static std::vector roundTripDirectionData(const DirectionData &in auto &staff = measure.staves.back(); staff.directions.push_back(inDirectionData); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); if (!r1.ok()) return {}; - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); if (!r2.ok()) return {}; - docId = r2.value(); - const auto rd = mgr.getData(docId); - mgr.destroyDocument(docId); + const auto rd = getScore(std::move(r2).value()); if (!rd.ok()) return {}; const auto &oscore = rd.value(); diff --git a/src/private/mxtest/api/FreezingRoundTrip.cpp b/src/private/mxtest/api/FreezingRoundTrip.cpp index 1a4b578a1..c23f7d201 100644 --- a/src/private/mxtest/api/FreezingRoundTrip.cpp +++ b/src/private/mxtest/api/FreezingRoundTrip.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/generated/DirectionTypeChoice.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/DynamicsChoice.h" @@ -42,36 +42,31 @@ inline bool writeRoundTrip(std::string inFilename) const auto outAfterFilepath = outDir + nameWithoutExtension + std::string{"_after.xml"}; const auto scoreData = mxtest::MxFileRepository::loadFile(inFilename); const auto filePath = mxtest::MxFileRepository::getFullPath(inFilename); - auto &docMgr = DocumentManager::getInstance(); - const auto rBefore = docMgr.createFromFile(filePath); + auto rBefore = MusicXml::fromFile(filePath); if (!rBefore.ok()) { return false; } - const int docId = rBefore.value(); - docMgr.writeToFile(docId, outBeforeFilepath); - docMgr.destroyDocument(docId); - const auto rAfter = docMgr.createFromScore(scoreData); + std::move(rBefore).value().writeToFile(outBeforeFilepath); + auto rAfter = fromScore(scoreData); if (!rAfter.ok()) { return false; } - const int apiDocId = rAfter.value(); - docMgr.writeToFile(apiDocId, outAfterFilepath); - docMgr.destroyDocument(apiDocId); - return docId != apiDocId; + std::move(rAfter).value().writeToFile(outAfterFilepath); + return true; } -/// Holds references into Documents that must be kept alive. +/// Holds the two round-tripped documents and their score data. struct TestData { - mx::core::DocumentPtr originalDoc; - mx::core::DocumentPtr savedDoc; + mx::api::MusicXml originalDoc; + mx::api::MusicXml savedDoc; mx::api::ScoreData originalScoreData; mx::api::ScoreData savedScoreData; - TestData(mx::core::DocumentPtr inOriginalDoc, mx::core::DocumentPtr inSavedDoc, - mx::api::ScoreData inOriginalScoreData, mx::api::ScoreData inSavedScoreData) + TestData(mx::api::MusicXml inOriginalDoc, mx::api::MusicXml inSavedDoc, mx::api::ScoreData inOriginalScoreData, + mx::api::ScoreData inSavedScoreData) : originalDoc{std::move(inOriginalDoc)}, savedDoc{std::move(inSavedDoc)}, originalScoreData{std::move(inOriginalScoreData)}, savedScoreData{std::move(inSavedScoreData)} { @@ -79,12 +74,12 @@ struct TestData const mx::core::ScorePartwise &originalScore() const { - return originalDoc->asScorePartwise(); + return originalDoc.getCoreDocument().asScorePartwise(); } const mx::core::ScorePartwise &savedScore() const { - return savedDoc->asScorePartwise(); + return savedDoc.getCoreDocument().asScorePartwise(); } //////////////////////////// original score ///////////////////// saved score /////////////////// @@ -103,21 +98,20 @@ struct TestData inline TestData getTestData(std::string filename) { - auto &mgr = DocumentManager::getInstance(); const auto filePath = mxtest::MxFileRepository::getFullPath(filename); - const auto rOrig = mgr.createFromFile(filePath); - const int originalId = rOrig.ok() ? rOrig.value() : -1; - const auto rOrigData = mgr.getData(originalId); - const auto originalScoreData = rOrigData.ok() ? rOrigData.value() : mx::api::ScoreData{}; - const auto rSaved = mgr.createFromScore(originalScoreData); - const int savedId = rSaved.ok() ? rSaved.value() : -1; - const auto rSavedData = mgr.getData(savedId); - const auto savedScoreData = rSavedData.ok() ? rSavedData.value() : mx::api::ScoreData{}; - auto originalDoc = mgr.getDocument(originalId); - auto savedDoc = mgr.getDocument(savedId); - mgr.destroyDocument(originalId); - mgr.destroyDocument(savedId); - return TestData{originalDoc, savedDoc, originalScoreData, savedScoreData}; + auto rOrig = mx::api::MusicXml::fromFile(filePath); + REQUIRE(rOrig.ok()); + mx::api::MusicXml originalDoc = std::move(rOrig).value(); + const auto rOrigData = mx::api::getScore(originalDoc); + REQUIRE(rOrigData.ok()); + const auto originalScoreData = rOrigData.value(); + auto rSaved = mx::api::fromScore(originalScoreData); + REQUIRE(rSaved.ok()); + mx::api::MusicXml savedDoc = std::move(rSaved).value(); + const auto rSavedData = mx::api::getScore(savedDoc); + REQUIRE(rSavedData.ok()); + const auto savedScoreData = rSavedData.value(); + return TestData{std::move(originalDoc), std::move(savedDoc), originalScoreData, savedScoreData}; } } // namespace @@ -169,30 +163,22 @@ TEST(roundTripViolaDynamicWrongTime, Freezing) { // in the original file measure number="X7" implicit="yes" width="389" // or search for font-family="roundTripViolaDynamicWrongTime" - auto &mgr = DocumentManager::getInstance(); const auto filePath = mxtest::MxFileRepository::getFullPath(freezingFile); - const auto rOrigId = mgr.createFromFile(filePath); - REQUIRE(rOrigId.ok()); - const int originalId = rOrigId.value(); - const auto rOrigData = mgr.getData(originalId); + auto rOrig = MusicXml::fromFile(filePath); + REQUIRE(rOrig.ok()); + MusicXml originalDoc = std::move(rOrig).value(); + const auto rOrigData = getScore(originalDoc); REQUIRE(rOrigData.ok()); const auto originalScoreData = rOrigData.value(); - auto originalDoc = mgr.getDocument(originalId); - const auto rSavedId = mgr.createFromScore(originalScoreData); - REQUIRE(rSavedId.ok()); - const int savedId = rSavedId.value(); - const auto rSavedData = mgr.getData(savedId); - REQUIRE(rSavedData.ok()); - const auto savedScoreData = rSavedData.value(); - auto savedDoc = mgr.getDocument(savedId); - mgr.destroyDocument(originalId); - mgr.destroyDocument(savedId); + auto rSaved = fromScore(originalScoreData); + REQUIRE(rSaved.ok()); + MusicXml savedDoc = std::move(rSaved).value(); const size_t partIndex = 0; const size_t measureIndex = 7; - const auto &originalScore = originalDoc->asScorePartwise(); - const auto &savedScore = savedDoc->asScorePartwise(); + const auto &originalScore = originalDoc.getCoreDocument().asScorePartwise(); + const auto &savedScore = savedDoc.getCoreDocument().asScorePartwise(); const auto originalMdcSpan = originalScore.part()[partIndex].measure()[measureIndex].musicData(); auto originalMdcIter = originalMdcSpan.begin(); @@ -317,46 +303,36 @@ T_END TEST(missingMusicXMLVersion, Freezing) { - auto &mgr = DocumentManager::getInstance(); const auto filePath = mxtest::MxFileRepository::getFullPath(freezingFile); - const auto rOrigId = mgr.createFromFile(filePath); - REQUIRE(rOrigId.ok()); - const int originalId = rOrigId.value(); - auto originalDoc = mgr.getDocument(originalId); - const auto rOrigData = mgr.getData(originalId); + auto rOrig = MusicXml::fromFile(filePath); + REQUIRE(rOrig.ok()); + MusicXml originalDoc = std::move(rOrig).value(); + const auto rOrigData = getScore(originalDoc); REQUIRE(rOrigData.ok()); - const auto rSavedId = mgr.createFromScore(rOrigData.value()); - REQUIRE(rSavedId.ok()); - const int savedId = rSavedId.value(); - auto savedDoc = mgr.getDocument(savedId); - mgr.destroyDocument(originalId); - mgr.destroyDocument(savedId); - - const bool originalScoreHasVersion = originalDoc->asScorePartwise().version().has_value(); - const bool savedScoreHasVersion = savedDoc->asScorePartwise().version().has_value(); + auto rSaved = fromScore(rOrigData.value()); + REQUIRE(rSaved.ok()); + MusicXml savedDoc = std::move(rSaved).value(); + + const bool originalScoreHasVersion = originalDoc.getCoreDocument().asScorePartwise().version().has_value(); + const bool savedScoreHasVersion = savedDoc.getCoreDocument().asScorePartwise().version().has_value(); CHECK(originalScoreHasVersion); CHECK(savedScoreHasVersion); } TEST(HasDefaultsHasAppearance, Freezing) { - auto &mgr = DocumentManager::getInstance(); const auto filePath = mxtest::MxFileRepository::getFullPath(freezingFile); - const auto rOrigId = mgr.createFromFile(filePath); - REQUIRE(rOrigId.ok()); - const int originalId = rOrigId.value(); - auto originalDoc = mgr.getDocument(originalId); - const auto rOrigData = mgr.getData(originalId); + auto rOrig = MusicXml::fromFile(filePath); + REQUIRE(rOrig.ok()); + MusicXml originalDoc = std::move(rOrig).value(); + const auto rOrigData = getScore(originalDoc); REQUIRE(rOrigData.ok()); - const auto rSavedId = mgr.createFromScore(rOrigData.value()); - REQUIRE(rSavedId.ok()); - const int savedId = rSavedId.value(); - auto savedDoc = mgr.getDocument(savedId); - mgr.destroyDocument(originalId); - mgr.destroyDocument(savedId); + auto rSaved = fromScore(rOrigData.value()); + REQUIRE(rSaved.ok()); + MusicXml savedDoc = std::move(rSaved).value(); - const auto &origHeader = originalDoc->asScorePartwise().scoreHeader(); - const auto &savedHeader = savedDoc->asScorePartwise().scoreHeader(); + const auto &origHeader = originalDoc.getCoreDocument().asScorePartwise().scoreHeader(); + const auto &savedHeader = savedDoc.getCoreDocument().asScorePartwise().scoreHeader(); const bool originalHasDefaults = origHeader.defaults().has_value(); const bool savedHasDefaults = savedHeader.defaults().has_value(); @@ -406,28 +382,25 @@ TEST(HasDefaultsHasAppearance, Freezing) TEST(appearanceLineWidths, Freezing) { - auto &mgr = DocumentManager::getInstance(); const auto filePath = mxtest::MxFileRepository::getFullPath(freezingFile); - const auto rOrigId = mgr.createFromFile(filePath); - REQUIRE(rOrigId.ok()); - const int originalId = rOrigId.value(); - auto originalDoc = mgr.getDocument(originalId); - const auto rOrigData = mgr.getData(originalId); + auto rOrig = MusicXml::fromFile(filePath); + REQUIRE(rOrig.ok()); + MusicXml originalDoc = std::move(rOrig).value(); + const auto rOrigData = getScore(originalDoc); REQUIRE(rOrigData.ok()); - const auto rSavedId = mgr.createFromScore(rOrigData.value()); - REQUIRE(rSavedId.ok()); - const int savedId = rSavedId.value(); - auto savedDoc = mgr.getDocument(savedId); - mgr.destroyDocument(originalId); - mgr.destroyDocument(savedId); + auto rSaved = fromScore(rOrigData.value()); + REQUIRE(rSaved.ok()); + MusicXml savedDoc = std::move(rSaved).value(); - REQUIRE(originalDoc->asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(savedDoc->asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(originalDoc->asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - REQUIRE(savedDoc->asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - const auto &originalAppearance = originalDoc->asScorePartwise().scoreHeader().defaults()->appearance().value(); - const auto &savedAppearance = savedDoc->asScorePartwise().scoreHeader().defaults()->appearance().value(); + const auto &originalAppearance = + originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); + const auto &savedAppearance = + savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto lineWidthSetSize = savedAppearance.lineWidth().size(); CHECK(lineWidthSetSize > 0); @@ -447,28 +420,25 @@ TEST(appearanceLineWidths, Freezing) TEST(appearanceNoteSize, Freezing) { - auto &mgr = DocumentManager::getInstance(); const auto filePath = mxtest::MxFileRepository::getFullPath(freezingFile); - const auto rOrigId = mgr.createFromFile(filePath); - REQUIRE(rOrigId.ok()); - const int originalId = rOrigId.value(); - auto originalDoc = mgr.getDocument(originalId); - const auto rOrigData = mgr.getData(originalId); + auto rOrig = MusicXml::fromFile(filePath); + REQUIRE(rOrig.ok()); + MusicXml originalDoc = std::move(rOrig).value(); + const auto rOrigData = getScore(originalDoc); REQUIRE(rOrigData.ok()); - const auto rSavedId = mgr.createFromScore(rOrigData.value()); - REQUIRE(rSavedId.ok()); - const int savedId = rSavedId.value(); - auto savedDoc = mgr.getDocument(savedId); - mgr.destroyDocument(originalId); - mgr.destroyDocument(savedId); + auto rSaved = fromScore(rOrigData.value()); + REQUIRE(rSaved.ok()); + MusicXml savedDoc = std::move(rSaved).value(); - REQUIRE(originalDoc->asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(savedDoc->asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(originalDoc->asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - REQUIRE(savedDoc->asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - const auto &originalAppearance = originalDoc->asScorePartwise().scoreHeader().defaults()->appearance().value(); - const auto &savedAppearance = savedDoc->asScorePartwise().scoreHeader().defaults()->appearance().value(); + const auto &originalAppearance = + originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); + const auto &savedAppearance = + savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto noteSizeSetSize = savedAppearance.noteSize().size(); CHECK(noteSizeSetSize > 0); @@ -488,28 +458,25 @@ TEST(appearanceNoteSize, Freezing) TEST(appearancDistance, Freezing) { - auto &mgr = DocumentManager::getInstance(); const auto filePath = mxtest::MxFileRepository::getFullPath(freezingFile); - const auto rOrigId = mgr.createFromFile(filePath); - REQUIRE(rOrigId.ok()); - const int originalId = rOrigId.value(); - auto originalDoc = mgr.getDocument(originalId); - const auto rOrigData = mgr.getData(originalId); + auto rOrig = MusicXml::fromFile(filePath); + REQUIRE(rOrig.ok()); + MusicXml originalDoc = std::move(rOrig).value(); + const auto rOrigData = getScore(originalDoc); REQUIRE(rOrigData.ok()); - const auto rSavedId = mgr.createFromScore(rOrigData.value()); - REQUIRE(rSavedId.ok()); - const int savedId = rSavedId.value(); - auto savedDoc = mgr.getDocument(savedId); - mgr.destroyDocument(originalId); - mgr.destroyDocument(savedId); - - REQUIRE(originalDoc->asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(savedDoc->asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(originalDoc->asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - REQUIRE(savedDoc->asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - - const auto &originalAppearance = originalDoc->asScorePartwise().scoreHeader().defaults()->appearance().value(); - const auto &savedAppearance = savedDoc->asScorePartwise().scoreHeader().defaults()->appearance().value(); + auto rSaved = fromScore(rOrigData.value()); + REQUIRE(rSaved.ok()); + MusicXml savedDoc = std::move(rSaved).value(); + + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + + const auto &originalAppearance = + originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); + const auto &savedAppearance = + savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto distanceSetSize = savedAppearance.distance().size(); CHECK(distanceSetSize > 0); diff --git a/src/private/mxtest/api/FreezingTest.cpp b/src/private/mxtest/api/FreezingTest.cpp index 7eeda58d2..7395f9840 100644 --- a/src/private/mxtest/api/FreezingTest.cpp +++ b/src/private/mxtest/api/FreezingTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" using namespace std; @@ -23,10 +23,8 @@ TEST( x, Freezing ) const std::string fileName{ "freezing.xml" }; const std::string path{ MxFileRepository::getFullPath( fileName ) }; - auto& docMgr = mx::api::DocumentManager::getInstance(); - auto docId = docMgr.createFromFile( path ); - auto scoreData = docMgr.getData( docId ); - docMgr.destroyDocument( docId ); + auto docResult = mx::api::MusicXml::fromFile( path ); + auto scoreData = mx::api::getScore( std::move( docResult ).value() ); const auto& part = scoreData.parts.at( partIndex ); const auto& measure = part.measures.at( measureIndex ); diff --git a/src/private/mxtest/api/GlissandoApiTest.cpp b/src/private/mxtest/api/GlissandoApiTest.cpp index f5b57358f..b6187013a 100644 --- a/src/private/mxtest/api/GlissandoApiTest.cpp +++ b/src/private/mxtest/api/GlissandoApiTest.cpp @@ -10,7 +10,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" #include "mxtest/file/MxFileRepository.h" diff --git a/src/private/mxtest/api/GraceCueApiTest.cpp b/src/private/mxtest/api/GraceCueApiTest.cpp index 3cb32e734..2fd248941 100644 --- a/src/private/mxtest/api/GraceCueApiTest.cpp +++ b/src/private/mxtest/api/GraceCueApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/IdAttributeApiTest.cpp b/src/private/mxtest/api/IdAttributeApiTest.cpp index ea70b8057..0fddcd647 100644 --- a/src/private/mxtest/api/IdAttributeApiTest.cpp +++ b/src/private/mxtest/api/IdAttributeApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/IdentificationSourceApiTest.cpp b/src/private/mxtest/api/IdentificationSourceApiTest.cpp index 1a72b67be..55f723101 100644 --- a/src/private/mxtest/api/IdentificationSourceApiTest.cpp +++ b/src/private/mxtest/api/IdentificationSourceApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/KeyDataTest.cpp b/src/private/mxtest/api/KeyDataTest.cpp index 779272164..1e4a9c73f 100644 --- a/src/private/mxtest/api/KeyDataTest.cpp +++ b/src/private/mxtest/api/KeyDataTest.cpp @@ -8,7 +8,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/generated/Attributes.h" #include "mx/core/generated/Cancel.h" #include "mx/core/generated/Document.h" @@ -42,11 +42,10 @@ ScoreData putKeyInScore(KeyData key) } /// Helper: get the first Key element from the first Attributes element of the first measure of the first part. -const mx::core::Key &getFirstCoreKey(const mx::core::DocumentPtr &corePtr) +const mx::core::Key &getFirstCoreKey(const mx::core::Document &coreDoc) { - REQUIRE(corePtr != nullptr); - REQUIRE(corePtr->isScorePartwise()); - const auto &scorePartwise = corePtr->asScorePartwise(); + REQUIRE(coreDoc.isScorePartwise()); + const auto &scorePartwise = coreDoc.asScorePartwise(); const auto parts = scorePartwise.part(); REQUIRE(!parts.empty()); const auto &part = parts[0]; @@ -125,13 +124,12 @@ TEST(EMajor, KeyData) key.staffIndex = 0; const auto original = putKeyInScore(key); - auto &docMgr = DocumentManager::getInstance(); - const auto originalIdResult = docMgr.createFromScore(original); + auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); - const int originalId = originalIdResult.value(); - const mx::core::DocumentPtr corePtr = docMgr.getDocument(originalId); + const auto originalDoc = std::move(originalIdResult).value(); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); - const auto &coreKey = getFirstCoreKey(corePtr); + const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); CHECK(coreKeyChoice.isTraditionalKey()); const auto &coreTraditionalKey = coreKeyChoice.asTraditionalKey(); @@ -162,14 +160,11 @@ TEST(EMajor, KeyData) // serialize and deserialize std::stringstream xml; - docMgr.writeToStream(originalId, xml); - docMgr.destroyDocument(originalId); + originalDoc.writeToStream(xml); std::istringstream iss{xml.str()}; - const auto deserializedIdResult = docMgr.createFromStream(iss); + auto deserializedIdResult = MusicXml::fromStream(iss); REQUIRE(deserializedIdResult.ok()); - const int deserializedId = deserializedIdResult.value(); - const auto deserializedScoreResult = docMgr.getData(deserializedId); - docMgr.destroyDocument(deserializedId); + const auto deserializedScoreResult = getScore(std::move(deserializedIdResult).value()); REQUIRE(deserializedScoreResult.ok()); const auto &deserializedScore = deserializedScoreResult.value(); const auto &deserializedKeys = deserializedScore.parts.at(0).measures.at(0).keys; @@ -193,13 +188,12 @@ TEST(AbMinor, KeyData) key.staffIndex = -10; const auto original = putKeyInScore(key); - auto &docMgr = DocumentManager::getInstance(); - const auto originalIdResult = docMgr.createFromScore(original); + auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); - const int originalId = originalIdResult.value(); - const mx::core::DocumentPtr corePtr = docMgr.getDocument(originalId); + const auto originalDoc = std::move(originalIdResult).value(); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); - const auto &coreKey = getFirstCoreKey(corePtr); + const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); CHECK(coreKeyChoice.isTraditionalKey()); const auto &coreTraditionalKey = coreKeyChoice.asTraditionalKey(); @@ -222,14 +216,11 @@ TEST(AbMinor, KeyData) // serialize and deserialize std::stringstream xml; - docMgr.writeToStream(originalId, xml); - docMgr.destroyDocument(originalId); + originalDoc.writeToStream(xml); std::istringstream iss{xml.str()}; - const auto deserializedIdResult = docMgr.createFromStream(iss); + auto deserializedIdResult = MusicXml::fromStream(iss); REQUIRE(deserializedIdResult.ok()); - const int deserializedId = deserializedIdResult.value(); - const auto deserializedScoreResult = docMgr.getData(deserializedId); - docMgr.destroyDocument(deserializedId); + const auto deserializedScoreResult = getScore(std::move(deserializedIdResult).value()); REQUIRE(deserializedScoreResult.ok()); const auto &deserializedScore = deserializedScoreResult.value(); const auto &deserializedKeys = deserializedScore.parts.at(0).measures.at(0).keys; @@ -257,13 +248,12 @@ TEST(NonTraditional1, KeyData) key.nonTraditional.push_back(dQuarterFlat); const auto original = putKeyInScore(key); - auto &docMgr = DocumentManager::getInstance(); - const auto originalIdResult = docMgr.createFromScore(original); + auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); - const int originalId = originalIdResult.value(); - const mx::core::DocumentPtr corePtr = docMgr.getDocument(originalId); + const auto originalDoc = std::move(originalIdResult).value(); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); - const auto &coreKey = getFirstCoreKey(corePtr); + const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); CHECK(coreKeyChoice.isNonTraditionalKey()); const auto &coreKeyComponents = coreKeyChoice.asNonTraditionalKey(); @@ -294,14 +284,11 @@ TEST(NonTraditional1, KeyData) // serialize and deserialize std::stringstream xml; - docMgr.writeToStream(originalId, xml); - docMgr.destroyDocument(originalId); + originalDoc.writeToStream(xml); std::istringstream iss{xml.str()}; - const auto deserializedIdResult = docMgr.createFromStream(iss); + auto deserializedIdResult = MusicXml::fromStream(iss); REQUIRE(deserializedIdResult.ok()); - const int deserializedId = deserializedIdResult.value(); - const auto deserializedScoreResult = docMgr.getData(deserializedId); - docMgr.destroyDocument(deserializedId); + const auto deserializedScoreResult = getScore(std::move(deserializedIdResult).value()); REQUIRE(deserializedScoreResult.ok()); const auto &deserializedScore = deserializedScoreResult.value(); const auto &deserializedKeys = deserializedScore.parts.at(0).measures.at(0).keys; @@ -489,13 +476,12 @@ TEST(CancelLocationBeforeBarline, KeyData) key.cancelLocation = CancelLocation::beforeBarline; const auto original = putKeyInScore(key); - auto &docMgr = DocumentManager::getInstance(); - const auto originalIdResult = docMgr.createFromScore(original); + auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); - const int originalId = originalIdResult.value(); - const mx::core::DocumentPtr corePtr = docMgr.getDocument(originalId); + const auto originalDoc = std::move(originalIdResult).value(); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); - const auto &coreKey = getFirstCoreKey(corePtr); + const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); CHECK(coreKeyChoice.isTraditionalKey()); const auto &coreTraditionalKey = coreKeyChoice.asTraditionalKey(); @@ -511,15 +497,12 @@ TEST(CancelLocationBeforeBarline, KeyData) // serialize and deserialize std::stringstream xml; - docMgr.writeToStream(originalId, xml); - docMgr.destroyDocument(originalId); + originalDoc.writeToStream(xml); CHECK(xml.str().find("location=\"before-barline\"") != std::string::npos); std::istringstream iss{xml.str()}; - const auto deserializedIdResult = docMgr.createFromStream(iss); + auto deserializedIdResult = MusicXml::fromStream(iss); REQUIRE(deserializedIdResult.ok()); - const int deserializedId = deserializedIdResult.value(); - const auto deserializedScoreResult = docMgr.getData(deserializedId); - docMgr.destroyDocument(deserializedId); + const auto deserializedScoreResult = getScore(std::move(deserializedIdResult).value()); REQUIRE(deserializedScoreResult.ok()); const auto &deserializedScore = deserializedScoreResult.value(); const auto &deserializedKeys = deserializedScore.parts.at(0).measures.at(0).keys; @@ -538,13 +521,12 @@ TEST(CancelLocationUnspecified, KeyData) key.cancel = -3; const auto original = putKeyInScore(key); - auto &docMgr = DocumentManager::getInstance(); - const auto originalIdResult = docMgr.createFromScore(original); + auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); - const int originalId = originalIdResult.value(); - const mx::core::DocumentPtr corePtr = docMgr.getDocument(originalId); + const auto originalDoc = std::move(originalIdResult).value(); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); - const auto &coreKey = getFirstCoreKey(corePtr); + const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); CHECK(coreKeyChoice.isTraditionalKey()); const auto &coreTraditionalKey = coreKeyChoice.asTraditionalKey(); @@ -554,15 +536,12 @@ TEST(CancelLocationUnspecified, KeyData) // serialize and deserialize std::stringstream xml; - docMgr.writeToStream(originalId, xml); - docMgr.destroyDocument(originalId); + originalDoc.writeToStream(xml); CHECK(xml.str().find("location=") == std::string::npos); std::istringstream iss{xml.str()}; - const auto deserializedIdResult = docMgr.createFromStream(iss); + auto deserializedIdResult = MusicXml::fromStream(iss); REQUIRE(deserializedIdResult.ok()); - const int deserializedId = deserializedIdResult.value(); - const auto deserializedScoreResult = docMgr.getData(deserializedId); - docMgr.destroyDocument(deserializedId); + const auto deserializedScoreResult = getScore(std::move(deserializedIdResult).value()); REQUIRE(deserializedScoreResult.ok()); const auto &deserializedScore = deserializedScoreResult.value(); const auto &deserializedKeys = deserializedScore.parts.at(0).measures.at(0).keys; @@ -744,20 +723,17 @@ TEST(ModeNoneIsNotNonTraditional, KeyData) key.mode = KeyMode::none; const auto original = putKeyInScore(key); - auto &docMgr = DocumentManager::getInstance(); - const auto originalIdResult = docMgr.createFromScore(original); + auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); - const int originalId = originalIdResult.value(); - const mx::core::DocumentPtr corePtr = docMgr.getDocument(originalId); + const auto originalDoc = std::move(originalIdResult).value(); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); - const auto &coreKey = getFirstCoreKey(corePtr); + const auto &coreKey = getFirstCoreKey(coreDoc); CHECK(coreKey.choice().isTraditionalKey()); const auto &coreTraditionalKey = coreKey.choice().asTraditionalKey(); CHECK_EQUAL(0, coreTraditionalKey.fifths().value()); REQUIRE(coreTraditionalKey.mode().has_value()); CHECK_EQUAL(std::string{"none"}, coreTraditionalKey.mode()->value()); - - docMgr.destroyDocument(originalId); } TEST(KeyDataEquality_change_cancelLocation, KeyData) diff --git a/src/private/mxtest/api/MarkRoundTripTest.cpp b/src/private/mxtest/api/MarkRoundTripTest.cpp index 361a83358..d3204a3b5 100644 --- a/src/private/mxtest/api/MarkRoundTripTest.cpp +++ b/src/private/mxtest/api/MarkRoundTripTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include #include @@ -33,23 +33,18 @@ std::vector roundTripMarkData(const MarkData &inMarkData) auto ¬e = voice.notes.back(); note.noteAttachmentData.marks.push_back(inMarkData); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); if (!r1.ok()) return {}; - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); if (!r2.ok()) return {}; - docId = r2.value(); - const auto rd = mgr.getData(docId); - mgr.destroyDocument(docId); + const auto rd = getScore(std::move(r2).value()); if (!rd.ok()) return {}; const auto &oscore = rd.value(); diff --git a/src/private/mxtest/api/MeasureDataTest.cpp b/src/private/mxtest/api/MeasureDataTest.cpp index c1444cd9f..d9a287579 100644 --- a/src/private/mxtest/api/MeasureDataTest.cpp +++ b/src/private/mxtest/api/MeasureDataTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" @@ -34,19 +34,15 @@ TEST(forwardRepeat, MeasureData) barlineData.repeat = true; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto rDocId = mgr.createFromScore(score); + auto rDocId = fromScore(score); REQUIRE(rDocId.ok()); - int docId = rDocId.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(rDocId).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto rDocId2 = mgr.createFromStream(iss); + auto rDocId2 = MusicXml::fromStream(iss); REQUIRE(rDocId2.ok()); - docId = rDocId2.value(); - const auto rOscore = mgr.getData(docId); + const auto rOscore = getScore(std::move(rDocId2).value()); REQUIRE(rOscore.ok()); const auto oscore = rOscore.value(); @@ -86,19 +82,15 @@ TEST(backwardRepeat, MeasureData) barlineData.repeat = true; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto rDocId = mgr.createFromScore(score); + auto rDocId = fromScore(score); REQUIRE(rDocId.ok()); - int docId = rDocId.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(rDocId).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto rDocId2 = mgr.createFromStream(iss); + auto rDocId2 = MusicXml::fromStream(iss); REQUIRE(rDocId2.ok()); - docId = rDocId2.value(); - const auto rOscore = mgr.getData(docId); + const auto rOscore = getScore(std::move(rDocId2).value()); REQUIRE(rOscore.ok()); const auto oscore = rOscore.value(); diff --git a/src/private/mxtest/api/MetronomeApiTest.cpp b/src/private/mxtest/api/MetronomeApiTest.cpp index bdd6cb0e4..45488e78e 100644 --- a/src/private/mxtest/api/MetronomeApiTest.cpp +++ b/src/private/mxtest/api/MetronomeApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include @@ -100,16 +100,14 @@ TEST(nonNumericPerMinuteRoundTrips, MetronomeApi) fast )"); - auto &mgr = DocumentManager::getInstance(); std::istringstream iss{xml}; - const auto idResult = mgr.createFromStream(iss); - CHECK(idResult.ok()); - if (!idResult.ok()) + auto docResult = MusicXml::fromStream(iss); + CHECK(docResult.ok()); + if (!docResult.ok()) { return; } - const auto dataResult = mgr.getData(idResult.value()); - mgr.destroyDocument(idResult.value()); + const auto dataResult = getScore(std::move(docResult).value()); CHECK(dataResult.ok()); if (!dataResult.ok()) { @@ -135,12 +133,8 @@ TEST(nonNumericPerMinuteRoundTrips, MetronomeApi) CHECK(std::string{"fast"} == bpm.beatsPerMinute); // The mark must also write back without error. - const auto id2Result = mgr.createFromScore(dataResult.value()); - CHECK(id2Result.ok()); - if (id2Result.ok()) - { - mgr.destroyDocument(id2Result.value()); - } + const auto doc2Result = fromScore(dataResult.value()); + CHECK(doc2Result.ok()); } T_END; diff --git a/src/private/mxtest/api/MidiNameRoundTripTest.cpp b/src/private/mxtest/api/MidiNameRoundTripTest.cpp index a33c0ca25..9bdc291c7 100644 --- a/src/private/mxtest/api/MidiNameRoundTripTest.cpp +++ b/src/private/mxtest/api/MidiNameRoundTripTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" using namespace std; diff --git a/src/private/mxtest/api/DocumentManagerTest.cpp b/src/private/mxtest/api/MusicXmlTest.cpp similarity index 71% rename from src/private/mxtest/api/DocumentManagerTest.cpp rename to src/private/mxtest/api/MusicXmlTest.cpp index 4c7ffdf49..cc8404082 100644 --- a/src/private/mxtest/api/DocumentManagerTest.cpp +++ b/src/private/mxtest/api/MusicXmlTest.cpp @@ -7,7 +7,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/DefaultsData.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/Attribution.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/MarginType.h" @@ -17,26 +17,16 @@ using namespace std; using namespace mx::api; -inline int loadDoc() +inline ScoreData loadDichterliebe() { - auto &docMngr = DocumentManager::getInstance(); - const auto r = docMngr.createFromFile(std::string{mxtest::getResourcesDirectoryPath()} + - std::string{"/recsuite/Dichterliebe01.xml"}); - return r.ok() ? r.value() : -1; -} - -inline void destroyDoc(int documentId) -{ - auto &docMngr = DocumentManager::getInstance(); - docMngr.destroyDocument(documentId); -} - -inline ScoreData getScore() -{ - const int documentId = loadDoc(); - const auto r = DocumentManager::getInstance().getData(documentId); - destroyDoc(documentId); - return r.ok() ? r.value() : ScoreData{}; + auto docResult = MusicXml::fromFile(std::string{mxtest::getResourcesDirectoryPath()} + + std::string{"/recsuite/Dichterliebe01.xml"}); + if (!docResult.ok()) + return ScoreData{}; + const auto scoreResult = getScore(std::move(docResult).value()); + if (!scoreResult.ok()) + return ScoreData{}; + return scoreResult.value(); } // Serializes a ScoreData to a stream and parses it back, returning the @@ -44,44 +34,58 @@ inline ScoreData getScore() // current test via REQUIRE rather than throwing across the boundary. inline ScoreData roundTripScore(const ScoreData &input) { - auto &docMngr = DocumentManager::getInstance(); - const auto createResult = docMngr.createFromScore(input); + auto createResult = fromScore(input); REQUIRE(createResult.ok()); - const int writeId = createResult.value(); std::ostringstream oss; - const auto writeResult = docMngr.writeToStream(writeId, oss); + const auto writeResult = std::move(createResult).value().writeToStream(oss); REQUIRE(writeResult.ok()); - docMngr.destroyDocument(writeId); std::istringstream iss{oss.str()}; - const auto reloadResult = docMngr.createFromStream(iss); + auto reloadResult = MusicXml::fromStream(iss); REQUIRE(reloadResult.ok()); - const int readId = reloadResult.value(); - const auto dataResult = docMngr.getData(readId); + const auto dataResult = getScore(std::move(reloadResult).value()); REQUIRE(dataResult.ok()); - auto output = dataResult.value(); - docMngr.destroyDocument(readId); - return output; + return dataResult.value(); } -// --- Document handle lifecycle ---------------------------------------------- -// The DocumentManager registry contract: a live id yields a document, and -// destroying it makes the id resolve to nullptr. Covered nowhere else. +// --- RAII ownership --------------------------------------------------------- +// A MusicXml owns its document. It cannot be copied, it moves, and a +// moved-from document fails safely on every path rather than crashing. -TEST(createFromFile, DocumentManager) +TEST(moveTransfersOwnership, MusicXml) { - auto &docMngr = DocumentManager::getInstance(); - const auto createResult = docMngr.createFromFile(std::string{mxtest::getResourcesDirectoryPath()} + - std::string{"/recsuite/Dichterliebe01.xml"}); - REQUIRE(createResult.ok()); - const int documentId = createResult.value(); - CHECK(documentId > 0); + auto docResult = MusicXml::fromFile(std::string{mxtest::getResourcesDirectoryPath()} + + std::string{"/recsuite/Dichterliebe01.xml"}); + REQUIRE(docResult.ok()); + MusicXml doc = std::move(docResult).value(); + + // moving transfers the document to the destination + MusicXml other = std::move(doc); + const auto scoreResult = getScore(other); + REQUIRE(scoreResult.ok()); + CHECK_EQUAL("Dichterliebe", scoreResult.value().workTitle); + + // the moved-from document fails safely on the read and write paths + const auto movedScoreResult = getScore(doc); + CHECK(!movedScoreResult.ok()); + CHECK(movedScoreResult.error().code == ResultCode::internalError); + std::stringstream ss; + const auto movedWriteResult = doc.writeToStream(ss); + CHECK(!movedWriteResult.ok()); + CHECK(movedWriteResult.error().code == ResultCode::internalError); +} - auto mxdocPtr = docMngr.getDocument(documentId); - CHECK(mxdocPtr != nullptr); - docMngr.destroyDocument(documentId); +T_END - auto shouldBeNull = docMngr.getDocument(documentId); - CHECK(shouldBeNull == nullptr); +// intoScore takes the document by value: the underlying tree is freed when +// the function returns, and the call site must spell std::move. +TEST(intoScoreConsumesDocument, MusicXml) +{ + auto docResult = MusicXml::fromFile(std::string{mxtest::getResourcesDirectoryPath()} + + std::string{"/recsuite/Dichterliebe01.xml"}); + REQUIRE(docResult.ok()); + const auto scoreResult = intoScore(std::move(docResult).value()); + REQUIRE(scoreResult.ok()); + CHECK_EQUAL("Dichterliebe", scoreResult.value().workTitle); } T_END @@ -90,80 +94,78 @@ T_END // Pins the reader against a frozen reference file. The corpus survival tests // only assert "loads without crashing"; these assert the actual values. -TEST(musicXmlType, DocumentManager) +TEST(musicXmlType, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_EQUAL("partwise", score.musicXmlType); } T_END -TEST(workTitle, DocumentManager) +TEST(workTitle, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_EQUAL("Dichterliebe", score.workTitle); } T_END -TEST(workNumber, DocumentManager) +TEST(workNumber, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_EQUAL("Op. 48", score.workNumber); } T_END -TEST(movementTitle, DocumentManager) +TEST(movementTitle, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_EQUAL("Im wunderschönen Monat Mai", score.movementTitle); } T_END -TEST(movementNumber, DocumentManager) +TEST(movementNumber, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_EQUAL("1", score.movementNumber); } T_END -TEST(composerName, DocumentManager) +TEST(composerName, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_EQUAL("Robert Schumann", score.composer); } T_END -TEST(lyricistName, DocumentManager) +TEST(lyricistName, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_EQUAL("Heinrich Heine", score.lyricist); } T_END -TEST(copyright, DocumentManager) +TEST(copyright, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_EQUAL("Copyright © 2002 Recordare LLC", score.copyright); } T_END -TEST(RoundTrip_copyrightType_defaultIsCopyright, DocumentManager) +TEST(RoundTrip_copyrightType_defaultIsCopyright, MusicXml) { auto input = ScoreData{}; input.copyright = "Public Domain"; std::stringstream ss; - auto &docMngr = DocumentManager::getInstance(); - const auto createResult = docMngr.createFromScore(input); + auto createResult = fromScore(input); REQUIRE(createResult.ok()); - docMngr.writeToStream(createResult.value(), ss); - docMngr.destroyDocument(createResult.value()); + std::move(createResult).value().writeToStream(ss); CHECK(ss.str().find(R"(Public Domain)") != std::string::npos); const auto output = roundTripScore(input); @@ -176,33 +178,29 @@ T_END // The reader only recognizes a typed "copyright" (or untyped) as the // source of ScoreData::copyright; any other type value is out of scope for // this simplified field, so only the write side is asserted here. -TEST(WriteHonorsExplicitCopyrightType, DocumentManager) +TEST(WriteHonorsExplicitCopyrightType, MusicXml) { auto input = ScoreData{}; input.copyright = "All rights reserved"; input.copyrightType = std::string{"mechanical"}; std::stringstream ss; - auto &docMngr = DocumentManager::getInstance(); - const auto createResult = docMngr.createFromScore(input); + auto createResult = fromScore(input); REQUIRE(createResult.ok()); - docMngr.writeToStream(createResult.value(), ss); - docMngr.destroyDocument(createResult.value()); + std::move(createResult).value().writeToStream(ss); CHECK(ss.str().find(R"(All rights reserved)") != std::string::npos); } T_END -TEST(RoundTrip_copyrightType_unsetOmitsAttribute, DocumentManager) +TEST(RoundTrip_copyrightType_unsetOmitsAttribute, MusicXml) { auto input = ScoreData{}; input.copyright = "Public Domain"; input.copyrightType = std::nullopt; std::stringstream ss; - auto &docMngr = DocumentManager::getInstance(); - const auto createResult = docMngr.createFromScore(input); + auto createResult = fromScore(input); REQUIRE(createResult.ok()); - docMngr.writeToStream(createResult.value(), ss); - docMngr.destroyDocument(createResult.value()); + std::move(createResult).value().writeToStream(ss); CHECK(ss.str().find(R"(Public Domain)") != std::string::npos); const auto output = roundTripScore(input); @@ -211,27 +209,27 @@ TEST(RoundTrip_copyrightType_unsetOmitsAttribute, DocumentManager) T_END -TEST(scalingMillimeters, DocumentManager) +TEST(scalingMillimeters, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_DOUBLES_EQUAL(6.35, score.defaults.scalingMillimeters, MX_API_EQUALITY_EPSILON) } T_END -TEST(scalingTenths, DocumentManager) +TEST(scalingTenths, MusicXml) { - auto score = getScore(); + auto score = loadDichterliebe(); CHECK_DOUBLES_EQUAL(40, score.defaults.scalingTenths, MX_API_EQUALITY_EPSILON) } T_END // --- Header / encoding round-trips ------------------------------------------ -// createFromScore -> writeToStream -> createFromStream -> getData fidelity for -// the identification and encoding metadata fields. +// fromScore -> writeToStream -> fromStream -> getScore fidelity for the +// identification and encoding metadata fields. -TEST(RoundTrip_WorkTitle, DocumentManager) +TEST(RoundTrip_WorkTitle, MusicXml) { const auto value = std::string{"value"}; auto input = ScoreData{}; @@ -243,7 +241,7 @@ TEST(RoundTrip_WorkTitle, DocumentManager) T_END #define ROUND_TRIP_TEST_SCALAR(scalarType, fieldPath, fieldName, value, nameSuffix) \ - TEST(RoundTrip_##fieldName##_##nameSuffix, DocumentManager) \ + TEST(RoundTrip_##fieldName##_##nameSuffix, MusicXml) \ { \ const auto testValue = scalarType{value}; \ auto input = ScoreData{}; \ @@ -274,7 +272,7 @@ ROUND_TRIP_TEST_SCALAR(int, encoding.encodingDate.day, day, 12, 0); // into ScoreData::lyricist and rewritten as type="lyricist", so a file carrying both lost the // lyricist; the publisher was dropped on the way out entirely. -TEST(RoundTrip_AllCreatorTypes, DocumentManager) +TEST(RoundTrip_AllCreatorTypes, MusicXml) { auto input = ScoreData{}; input.composer = "MetaComposer"; @@ -282,14 +280,11 @@ TEST(RoundTrip_AllCreatorTypes, DocumentManager) input.arranger = "MetaArranger"; input.publisher = "MetaPublisher"; - auto &docMngr = DocumentManager::getInstance(); - const auto createResult = docMngr.createFromScore(input); - REQUIRE(createResult.ok()); - const int writeId = createResult.value(); std::ostringstream oss; - const auto writeResult = docMngr.writeToStream(writeId, oss); + auto createResult = fromScore(input); + REQUIRE(createResult.ok()); + const auto writeResult = std::move(createResult).value().writeToStream(oss); REQUIRE(writeResult.ok()); - docMngr.destroyDocument(writeId); const auto xml = oss.str(); CHECK(xml.find("MetaComposer") != std::string::npos); @@ -310,17 +305,14 @@ T_END // composer and a lyricist, plus creator types mx::api does not model, which must not disturb // the ones it does. -TEST(ReadAllCreatorTypes, DocumentManager) +TEST(ReadAllCreatorTypes, MusicXml) { - auto &docMngr = DocumentManager::getInstance(); - const auto createResult = docMngr.createFromFile(std::string{mxtest::getResourcesDirectoryPath()} + - std::string{"/musuite/testMetaData.xml"}); - REQUIRE(createResult.ok()); - const int documentId = createResult.value(); - const auto dataResult = docMngr.getData(documentId); + auto docResult = + MusicXml::fromFile(std::string{mxtest::getResourcesDirectoryPath()} + std::string{"/musuite/testMetaData.xml"}); + REQUIRE(docResult.ok()); + const auto dataResult = getScore(std::move(docResult).value()); REQUIRE(dataResult.ok()); const auto score = dataResult.value(); - docMngr.destroyDocument(documentId); CHECK_EQUAL("MetaComposer", score.composer); CHECK_EQUAL("MetaLyricist", score.lyricist); @@ -335,7 +327,7 @@ T_END // unequal margins emit separate odd and even entries. This rule is exercised // nowhere else. -TEST(Layout_PageMarginsBoth, DocumentManager) +TEST(Layout_PageMarginsBoth, MusicXml) { auto score = ScoreData{}; const long double left = 0.1; @@ -352,13 +344,12 @@ TEST(Layout_PageMarginsBoth, DocumentManager) score.defaults.pageLayout.margins.even.value().top = top; score.defaults.pageLayout.margins.odd.value().bottom = bottom; score.defaults.pageLayout.margins.even.value().bottom = bottom; - const auto rDocId = DocumentManager::getInstance().createFromScore(score); - REQUIRE(rDocId.ok()); - const int docId = rDocId.value(); - auto mxDoc = DocumentManager::getInstance().getDocument(docId); - REQUIRE(mxDoc != nullptr); - REQUIRE(mxDoc->isScorePartwise()); - const auto &defaults = mxDoc->asScorePartwise().scoreHeader().defaults(); + auto docResult = fromScore(score); + REQUIRE(docResult.ok()); + const MusicXml doc = std::move(docResult).value(); + const auto &mxDoc = doc.getCoreDocument(); + REQUIRE(mxDoc.isScorePartwise()); + const auto &defaults = mxDoc.asScorePartwise().scoreHeader().defaults(); REQUIRE(defaults.has_value()); const auto &pageLayout = defaults->layout().pageLayout(); REQUIRE(pageLayout.has_value()); @@ -369,12 +360,11 @@ TEST(Layout_PageMarginsBoth, DocumentManager) REQUIRE(pageMarginsSpan[0].type().has_value()); CHECK(mx::core::MarginType::Tag::both == pageMarginsSpan[0].type()->tag()); } - DocumentManager::getInstance().destroyDocument(docId); } T_END -TEST(Layout_PageMarginsEvenOdd, DocumentManager) +TEST(Layout_PageMarginsEvenOdd, MusicXml) { auto score = ScoreData{}; const long double left = 0.1; @@ -391,13 +381,12 @@ TEST(Layout_PageMarginsEvenOdd, DocumentManager) score.defaults.pageLayout.margins.even.value().top = top; score.defaults.pageLayout.margins.odd.value().bottom = bottom; score.defaults.pageLayout.margins.even.value().bottom = bottom; - const auto rDocId = DocumentManager::getInstance().createFromScore(score); - REQUIRE(rDocId.ok()); - const int docId = rDocId.value(); - auto mxDoc = DocumentManager::getInstance().getDocument(docId); - REQUIRE(mxDoc != nullptr); - REQUIRE(mxDoc->isScorePartwise()); - const auto &defaults = mxDoc->asScorePartwise().scoreHeader().defaults(); + auto docResult = fromScore(score); + REQUIRE(docResult.ok()); + const MusicXml doc = std::move(docResult).value(); + const auto &mxDoc = doc.getCoreDocument(); + REQUIRE(mxDoc.isScorePartwise()); + const auto &defaults = mxDoc.asScorePartwise().scoreHeader().defaults(); REQUIRE(defaults.has_value()); const auto &pageLayout = defaults->layout().pageLayout(); REQUIRE(pageLayout.has_value()); @@ -410,7 +399,6 @@ TEST(Layout_PageMarginsEvenOdd, DocumentManager) REQUIRE(pageMarginsSpan[1].type().has_value()); CHECK(mx::core::MarginType::Tag::even == pageMarginsSpan[1].type()->tag()); } - DocumentManager::getInstance().destroyDocument(docId); } T_END @@ -419,7 +407,7 @@ T_END // The only coverage of the element: no other unit test touches it, // and none of the corpus files that use it are in the api-roundtrip baseline. -TEST(RoundTrip_SupportedItems_elementName, DocumentManager) +TEST(RoundTrip_SupportedItems_elementName, MusicXml) { const auto testValue0 = std::string{"value0"}; const auto testValue1 = std::string{"value1"}; @@ -438,7 +426,7 @@ TEST(RoundTrip_SupportedItems_elementName, DocumentManager) T_END -TEST(RoundTrip_SupportedItems_attributeName, DocumentManager) +TEST(RoundTrip_SupportedItems_attributeName, MusicXml) { const auto testValue0 = std::string{"value0"}; const auto testValue1 = std::string{"value1"}; @@ -459,7 +447,7 @@ TEST(RoundTrip_SupportedItems_attributeName, DocumentManager) T_END -TEST(RoundTrip_SupportedItems_specificValue, DocumentManager) +TEST(RoundTrip_SupportedItems_specificValue, MusicXml) { const auto testValue0 = std::string{"value0"}; const auto testValue1 = std::string{"value1"}; @@ -482,7 +470,7 @@ TEST(RoundTrip_SupportedItems_specificValue, DocumentManager) T_END -TEST(RoundTrip_SupportedItems_software, DocumentManager) +TEST(RoundTrip_SupportedItems_software, MusicXml) { const auto testValue0 = std::string{"value0"}; const auto testValue1 = std::string{"value1"}; @@ -497,7 +485,7 @@ TEST(RoundTrip_SupportedItems_software, DocumentManager) T_END -TEST(RoundTrip_SupportedItems_isSupported, DocumentManager) +TEST(RoundTrip_SupportedItems_isSupported, MusicXml) { const auto testValue0 = true; const auto testValue1 = false; @@ -527,22 +515,19 @@ T_END // Serialize a ScoreData to a string via the api write path (no reload). inline std::string writeScoreToString(const ScoreData &input) { - auto &docMngr = DocumentManager::getInstance(); - const auto createResult = docMngr.createFromScore(input); + auto createResult = fromScore(input); REQUIRE(createResult.ok()); - const int writeId = createResult.value(); std::ostringstream oss; - const auto writeResult = docMngr.writeToStream(writeId, oss); + const auto writeResult = std::move(createResult).value().writeToStream(oss); REQUIRE(writeResult.ok()); - docMngr.destroyDocument(writeId); return oss.str(); } -TEST(writeMxVersion_defaultsTrueAndStamps, DocumentManager) +TEST(writeMxVersion_defaultsTrueAndStamps, MusicXml) { // Parsed from a real file that never carried mx's stamp; the flag still // defaults true, so the written output gains the stamp. - ScoreData score = getScore(); + ScoreData score = loadDichterliebe(); CHECK(score.encoding.writeMxVersion); const std::string xml = writeScoreToString(score); CHECK(xml.find(std::string{mx::core::kMxSoftwareMarker}) != std::string::npos); @@ -550,10 +535,10 @@ TEST(writeMxVersion_defaultsTrueAndStamps, DocumentManager) T_END -TEST(writeMxVersion_offSuppressesStamp, DocumentManager) +TEST(writeMxVersion_offSuppressesStamp, MusicXml) { // Turn the stamp off after parsing: the written output must not contain it. - ScoreData score = getScore(); + ScoreData score = loadDichterliebe(); score.encoding.writeMxVersion = false; const std::string xml = writeScoreToString(score); CHECK(xml.find(std::string{mx::core::kMxSoftwareMarker}) == std::string::npos); diff --git a/src/private/mxtest/api/MxlTest.cpp b/src/private/mxtest/api/MxlTest.cpp index fe540392c..354f3652b 100644 --- a/src/private/mxtest/api/MxlTest.cpp +++ b/src/private/mxtest/api/MxlTest.cpp @@ -6,17 +6,15 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/file/MxFileRepository.h" #include TEST(Mxl, TemporaryNoCrashTest) { const auto filepath = mxtest::MxFileRepository::getFullPath("Dichterliebe01.mxl"); - auto &docMgr = mx::api::DocumentManager::getInstance(); - - // The new API does not throw; it returns Result with an error code - const auto result = docMgr.createFromFile(filepath); + // The api does not throw; it returns a Result with an error code + const auto result = mx::api::MusicXml::fromFile(filepath); CHECK(!result.ok()); if (!result.ok()) { diff --git a/src/private/mxtest/api/NewSystemTest.cpp b/src/private/mxtest/api/NewSystemTest.cpp index c0d888894..cd9e16b02 100644 --- a/src/private/mxtest/api/NewSystemTest.cpp +++ b/src/private/mxtest/api/NewSystemTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/MusicDataChoice.h" #include "mx/core/generated/Print.h" @@ -64,15 +64,10 @@ TEST(newSystem, doesItWork) addSystemBreak(25); addSystemBreak(50); addSystemBreak(75); - auto &docMgr = DocumentManager::getInstance(); - const auto rId = docMgr.createFromScore(s); + auto rId = fromScore(s); REQUIRE(rId.ok()); - const int id = rId.value(); - const auto doc = docMgr.getDocument(id); - docMgr.destroyDocument(id); - REQUIRE(doc != nullptr); - REQUIRE(doc->isScorePartwise()); - const auto &sp = doc->asScorePartwise(); + const auto doc = std::move(rId).value(); + const auto &sp = doc.getCoreDocument().asScorePartwise(); const auto &p = sp.part()[0]; size_t index = 0; diff --git a/src/private/mxtest/api/NoteDataTest.cpp b/src/private/mxtest/api/NoteDataTest.cpp index 97cda8019..7264add70 100644 --- a/src/private/mxtest/api/NoteDataTest.cpp +++ b/src/private/mxtest/api/NoteDataTest.cpp @@ -7,7 +7,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/generated/Direction.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/MusicDataChoice.h" @@ -38,19 +38,15 @@ TEST(otherArticulation, NoteData) note.noteAttachmentData.marks.back().name = "october 2018"; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -96,25 +92,20 @@ TEST(pitchedRestDisplayStepOctave, NoteData) rest.durationData.durationTimeTicks = 96; voice.notes.push_back(rest); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const auto xml = ss.str(); CHECK(xml.find("E") != std::string::npos); CHECK(xml.find("4") != std::string::npos); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto outScore = rd.value(); - mgr.destroyDocument(docId); const auto &outRest = outScore.parts.back().measures.back().staves.back().voices.begin()->second.notes.back(); CHECK(outRest.isRest); @@ -154,19 +145,15 @@ TEST(customArticulation, NoteData) note.noteAttachmentData.marks.back().positionData.defaultX = 333.3; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -210,19 +197,15 @@ TEST(otherOrnament, NoteData) note.noteAttachmentData.marks.back().name = "**()00))&"; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -270,19 +253,15 @@ TEST(technical, NoteData) note.noteAttachmentData.marks.back().name = "Bob"; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -343,21 +322,16 @@ TEST(technical_fingering_pluck_roundtrip, NoteData) note.noteAttachmentData.marks.emplace_back(Placement::above, MarkType::pluck); note.noteAttachmentData.marks.back().name = "p"; - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); std::istringstream iss{ss.str()}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto oscore = rd.value(); - mgr.destroyDocument(docId); const auto &omarks = oscore.parts.back().measures.back().staves.back().voices.at(0).notes.back().noteAttachmentData.marks; @@ -390,15 +364,12 @@ T_END; TEST(technical_import_file, NoteData) { - auto &mgr = DocumentManager::getInstance(); const auto path = std::string{mxtest::getResourcesDirectoryPath()} + std::string{"/ksuite/k004a_Technical.xml"}; - const auto r = mgr.createFromFile(path); + auto r = MusicXml::fromFile(path); REQUIRE(r.ok()); - const int docId = r.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r).value()); REQUIRE(rd.ok()); const auto score = rd.value(); - mgr.destroyDocument(docId); const auto &part = score.parts.at(0); @@ -446,25 +417,19 @@ TEST(technical_hole_arrow_handbell_roundtrip, NoteData) return score; }; - auto &mgr = DocumentManager::getInstance(); - for (const auto markType : {MarkType::hole, MarkType::arrow, MarkType::handbell}) { - const auto r1 = mgr.createFromScore(makeScore(markType)); + auto r1 = fromScore(makeScore(markType)); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); std::istringstream iss{ss.str()}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto outScore = rd.value(); - mgr.destroyDocument(docId); const auto &outMarks = outScore.parts.back() .measures.back() @@ -512,19 +477,15 @@ TEST(words, NoteData) directions.push_back(direction); // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -596,19 +557,15 @@ TEST(tremolos, NoteData) marks.emplace_back(mark); // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -689,12 +646,9 @@ T_END; TEST(measuredTremoloFromSyntheticFile, NoteData) { const std::string path = mxtest::getResourcesDirectoryPath() + "synthetic/tremolo.3.0.xml"; - auto &docMgr = DocumentManager::getInstance(); - const auto docIdResult = docMgr.createFromFile(path); - REQUIRE(docIdResult.ok()); - const int docId = docIdResult.value(); - const auto scoreResult = docMgr.getData(docId); - docMgr.destroyDocument(docId); + auto docResult = MusicXml::fromFile(path); + REQUIRE(docResult.ok()); + const auto scoreResult = getScore(std::move(docResult).value()); REQUIRE(scoreResult.ok()); const auto &score = scoreResult.value(); @@ -752,12 +706,9 @@ T_END; TEST(unmeasuredTremoloFromSyntheticFile, NoteData) { const std::string path = mxtest::getResourcesDirectoryPath() + "synthetic/tremolo.unmeasured.3.1.xml"; - auto &docMgr = DocumentManager::getInstance(); - const auto docIdResult = docMgr.createFromFile(path); - REQUIRE(docIdResult.ok()); - const int docId = docIdResult.value(); - const auto scoreResult = docMgr.getData(docId); - docMgr.destroyDocument(docId); + auto docResult = MusicXml::fromFile(path); + REQUIRE(docResult.ok()); + const auto scoreResult = getScore(std::move(docResult).value()); REQUIRE(scoreResult.ok()); const auto &score = scoreResult.value(); @@ -790,19 +741,15 @@ TEST(miscFields, NoteData) note.miscData.push_back("Bishop"); // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -855,21 +802,16 @@ TEST(SlurTieNumberLevelA, NoteData) note.noteAttachmentData.curveStarts.push_back(curveStart); voice.notes.push_back(note); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - const int id = r1.value(); std::stringstream ss; - mgr.writeToStream(id, ss); - mgr.destroyDocument(id); + std::move(r1).value().writeToStream(ss); std::istringstream iss{ss.str()}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - const int id2 = r2.value(); - const auto rd = mgr.getData(id2); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto scoreData = rd.value(); - mgr.destroyDocument(id2); const auto ¬eData = scoreData.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.front(); const auto &cs = noteData.noteAttachmentData.curveStarts.front(); @@ -897,21 +839,16 @@ TEST(SlurTieNumberLevelB, NoteData) note.noteAttachmentData.curveContinuations.push_back(curveContinue); voice.notes.push_back(note); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - const int id = r1.value(); std::stringstream ss; - mgr.writeToStream(id, ss); - mgr.destroyDocument(id); + std::move(r1).value().writeToStream(ss); std::istringstream iss{ss.str()}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - const int id2 = r2.value(); - const auto rd = mgr.getData(id2); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto scoreData = rd.value(); - mgr.destroyDocument(id2); const auto ¬eData = scoreData.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.front(); const auto &cc = noteData.noteAttachmentData.curveContinuations.front(); @@ -939,21 +876,16 @@ TEST(SlurTieNumberLevelC, NoteData) note.noteAttachmentData.curveStops.push_back(curveStop); voice.notes.push_back(note); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - const int id = r1.value(); std::stringstream ss; - mgr.writeToStream(id, ss); - mgr.destroyDocument(id); + std::move(r1).value().writeToStream(ss); std::istringstream iss{ss.str()}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - const int id2 = r2.value(); - const auto rd = mgr.getData(id2); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto scoreData = rd.value(); - mgr.destroyDocument(id2); const auto ¬eData = scoreData.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.front(); const auto &cs = noteData.noteAttachmentData.curveStops.front(); @@ -985,19 +917,15 @@ TEST(ornaments, NoteData) note.noteAttachmentData.marks.back().positionData.defaultY = -456.0; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -1045,19 +973,15 @@ TEST(pedalStart, NoteData) direction.tickTimePosition = 7; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -1095,19 +1019,15 @@ TEST(pedalStop, NoteData) direction.tickTimePosition = 70342; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); @@ -1185,18 +1105,15 @@ TEST(directionOrder, NoteData) staff.directions.push_back(direction); // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - const int docId = r1.value(); - auto docPtr = mgr.getDocument(docId); + const auto doc = std::move(r1).value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + doc.writeToStream(ss); - REQUIRE(docPtr != nullptr); - REQUIRE(docPtr->isScorePartwise()); - const auto &partwise = docPtr->asScorePartwise(); + const auto &docPtr = doc.getCoreDocument(); + REQUIRE(docPtr.isScorePartwise()); + const auto &partwise = docPtr.asScorePartwise(); const auto partwiseParts = partwise.part(); REQUIRE(!partwiseParts.empty()); const auto &partwisePart = partwiseParts[0]; @@ -1343,19 +1260,15 @@ TEST(directionOrderRoundTrip, NoteData) staff.directions.push_back(direction); // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); // The write side always emits version="4.0"; normalize so version fields @@ -1392,19 +1305,15 @@ TEST(notePositionRoundTrip, NoteData) voice.notes.push_back(note); // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto oscore = rd.value(); // The write side always emits version="4.0"; normalize so version fields @@ -1434,25 +1343,20 @@ TEST(noteheadFaUpRoundtrip, NoteData) note.notehead = Notehead::faUp; voice.notes.push_back(note); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); CHECK(xml.find("fa up") != std::string::npos); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto outScore = rd.value(); - mgr.destroyDocument(docId); const auto &outNote = outScore.parts.back().measures.back().staves.back().voices.at(0).notes.back(); CHECK(outNote.notehead == Notehead::faUp); @@ -1480,23 +1384,18 @@ TEST(noteheadCircledRoundtrip, NoteData) note.notehead = Notehead::circled; voice.notes.push_back(note); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto outScore = rd.value(); - mgr.destroyDocument(docId); const auto &outNote = outScore.parts.back().measures.back().staves.back().voices.at(0).notes.back(); CHECK(outNote.notehead == Notehead::circled); @@ -1522,23 +1421,18 @@ TEST(noteheadOtherRoundtrip, NoteData) note.notehead = Notehead::other; voice.notes.push_back(note); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto outScore = rd.value(); - mgr.destroyDocument(docId); const auto &outNote = outScore.parts.back().measures.back().staves.back().voices.at(0).notes.back(); CHECK(outNote.notehead == Notehead::other); @@ -1668,12 +1562,9 @@ T_END; TEST(noteheadSyntheticFileRead, NoteData) { const std::string path = mxtest::getResourcesDirectoryPath() + "synthetic/notehead.3.1.xml"; - auto &docMgr = DocumentManager::getInstance(); - const auto docIdResult = docMgr.createFromFile(path); - REQUIRE(docIdResult.ok()); - const int docId = docIdResult.value(); - const auto scoreResult = docMgr.getData(docId); - docMgr.destroyDocument(docId); + auto docResult = MusicXml::fromFile(path); + REQUIRE(docResult.ok()); + const auto scoreResult = getScore(std::move(docResult).value()); REQUIRE(scoreResult.ok()); const auto &score = scoreResult.value(); REQUIRE(score.parts.size() == 1); @@ -1706,24 +1597,19 @@ TEST(printObjectNo, NoteData) note.printData.printObject = Bool::no; voice.notes.push_back(note); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); CHECK(xml.find(R"(print-object="no")") != std::string::npos); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto outScore = rd.value(); - mgr.destroyDocument(docId); const auto &outNote = outScore.parts.back().measures.back().staves.back().voices.at(0).notes.back(); CHECK(outNote.printData.printObject == Bool::no); @@ -1763,26 +1649,21 @@ TEST(strongAccentDirection, NoteData) plainNote.noteAttachmentData.marks.emplace_back(MarkType::strongAccent); voice.notes.push_back(plainNote); - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - auto docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); CHECK(xml.find(R"()") != std::string::npos); CHECK(xml.find(R"()") != std::string::npos); CHECK(xml.find("") != std::string::npos); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - docId = r2.value(); - const auto rd = mgr.getData(docId); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); const auto outScore = rd.value(); - mgr.destroyDocument(docId); const auto &outNotes = outScore.parts.back().measures.back().staves.back().voices.at(0).notes; REQUIRE(outNotes.size() == static_cast(3)); diff --git a/src/private/mxtest/api/NoteVelocityApiTest.cpp b/src/private/mxtest/api/NoteVelocityApiTest.cpp index 1acc21352..3aa53a2f6 100644 --- a/src/private/mxtest/api/NoteVelocityApiTest.cpp +++ b/src/private/mxtest/api/NoteVelocityApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/OttavaSizeApiTest.cpp b/src/private/mxtest/api/OttavaSizeApiTest.cpp index 8bbc364bc..8e9446d09 100644 --- a/src/private/mxtest/api/OttavaSizeApiTest.cpp +++ b/src/private/mxtest/api/OttavaSizeApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/OttavaData.h" #include "mx/api/ScoreData.h" #include "mxtest/api/RoundTrip.h" diff --git a/src/private/mxtest/api/PageDataTest.cpp b/src/private/mxtest/api/PageDataTest.cpp index 1c1506d81..c5010beb6 100644 --- a/src/private/mxtest/api/PageDataTest.cpp +++ b/src/private/mxtest/api/PageDataTest.cpp @@ -6,7 +6,7 @@ #include "mxtest/control/CompileControl.h" #ifdef MX_COMPILE_API_TESTS -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/file/MxFileRepository.h" @@ -91,7 +91,6 @@ inline mx::api::ScoreData makeSomeBoringMusic(int inNumMeasures) TEST(TestPageData, PageData) { auto score1 = pageDataTest::makeSomeBoringMusic(33); - auto &docMgr = DocumentManager::getInstance(); SystemData sd{Bool::yes}; MeasureIndex measureIndex = 1; score1.layout[measureIndex].system = sd; @@ -127,17 +126,14 @@ TEST(TestPageData, PageData) pd = PageData{}; pd.newPage = Bool::no; score1.layout[10].page = pd; - const auto rId1 = docMgr.createFromScore(score1); + auto rId1 = fromScore(score1); REQUIRE(rId1.ok()); - const int id1 = rId1.value(); std::stringstream xml1; - docMgr.writeToStream(id1, xml1); - docMgr.destroyDocument(id1); + std::move(rId1).value().writeToStream(xml1); std::istringstream xml1is{xml1.str()}; - const auto rId2 = docMgr.createFromStream(xml1is); + auto rId2 = MusicXml::fromStream(xml1is); REQUIRE(rId2.ok()); - const int id2 = rId2.value(); - const auto rScore2 = docMgr.getData(id2); + const auto rScore2 = getScore(std::move(rId2).value()); REQUIRE(rScore2.ok()); const auto score2 = rScore2.value(); // The write side always emits version="4.0"; normalize so version fields @@ -145,26 +141,21 @@ TEST(TestPageData, PageData) score1.musicXmlVersion = score2.musicXmlVersion; score1.declaredMusicXmlVersion = score2.declaredMusicXmlVersion; CHECK(score1 == score2); - docMgr.destroyDocument(id2); - const auto rId3 = docMgr.createFromScore(score2); + auto rId3 = fromScore(score2); REQUIRE(rId3.ok()); - const int id3 = rId3.value(); std::stringstream xml3; - docMgr.writeToStream(id3, xml3); + std::move(rId3).value().writeToStream(xml3); CHECK(xml1.str() == xml3.str()); } TEST(LoadFinaleExport, PageData) { const auto filepath = mxtest::MxFileRepository::getFullPath("systems-and-pages.xml"); - auto &docMgr = DocumentManager::getInstance(); - const auto rId = docMgr.createFromFile(filepath); + auto rId = MusicXml::fromFile(filepath); REQUIRE(rId.ok()); - const int id = rId.value(); - const auto rScore = docMgr.getData(id); + const auto rScore = getScore(std::move(rId).value()); REQUIRE(rScore.ok()); const auto score = rScore.value(); - docMgr.destroyDocument(id); // The loaded file has page and system information as follows: // measure number 1 index 0 : page : system diff --git a/src/private/mxtest/api/PartGroupRoundTripTest.cpp b/src/private/mxtest/api/PartGroupRoundTripTest.cpp index 267d3940e..907638939 100644 --- a/src/private/mxtest/api/PartGroupRoundTripTest.cpp +++ b/src/private/mxtest/api/PartGroupRoundTripTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" using namespace mx::api; diff --git a/src/private/mxtest/api/PitchDataTest.cpp b/src/private/mxtest/api/PitchDataTest.cpp index 3c289f4e6..2c2b9cf91 100644 --- a/src/private/mxtest/api/PitchDataTest.cpp +++ b/src/private/mxtest/api/PitchDataTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "pugixml.hpp" #include @@ -75,14 +75,11 @@ Output pitchDataTest(const Input &input) note.pitchData.cents = input.cents; // round trip it through xml - auto &mgr = DocumentManager::getInstance(); - const auto docIdResult = mgr.createFromScore(score); + auto docIdResult = fromScore(score); if (!docIdResult.ok()) return {}; - const int docId = docIdResult.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(docIdResult).value().writeToStream(ss); // check the alter value that was written to xml pugi::xml_document xdoc; @@ -95,12 +92,10 @@ Output pitchDataTest(const Input &input) // deserialize back to ScoreData std::istringstream iss{xml}; - const auto docId2Result = mgr.createFromStream(iss); + auto docId2Result = MusicXml::fromStream(iss); if (!docId2Result.ok()) return {}; - const int docId2 = docId2Result.value(); - const auto oscoreResult = mgr.getData(docId2); - mgr.destroyDocument(docId2); + const auto oscoreResult = getScore(std::move(docId2Result).value()); if (!oscoreResult.ok()) return {}; const auto &oscore = oscoreResult.value(); @@ -115,13 +110,11 @@ Output pitchDataTest(const Input &input) output.accidental = onote.pitchData.accidental; // serialize a second time and check the alter string again - const auto docId3Result = mgr.createFromScore(score); + auto docId3Result = fromScore(score); if (!docId3Result.ok()) return {}; - const int docId3 = docId3Result.value(); std::stringstream ss2; - mgr.writeToStream(docId3, ss2); - mgr.destroyDocument(docId3); + std::move(docId3Result).value().writeToStream(ss2); // check the alter value that was written to xml pugi::xml_document xdoc2; @@ -366,13 +359,10 @@ TEST(AccidentalPresenceAttributesRoundTrip, PitchData) note.pitchData.isAccidentalEditorial = true; note.pitchData.isAccidentalBracketed = true; - auto &mgr = DocumentManager::getInstance(); - const auto r1 = mgr.createFromScore(score); + auto r1 = fromScore(score); REQUIRE(r1.ok()); - const int docId = r1.value(); std::stringstream ss; - mgr.writeToStream(docId, ss); - mgr.destroyDocument(docId); + std::move(r1).value().writeToStream(ss); const std::string xml = ss.str(); CHECK(xml.find(R"(parentheses="yes")") != std::string::npos); CHECK(xml.find(R"(cautionary="yes")") != std::string::npos); @@ -380,12 +370,10 @@ TEST(AccidentalPresenceAttributesRoundTrip, PitchData) CHECK(xml.find(R"(bracket="yes")") != std::string::npos); std::istringstream iss{xml}; - const auto r2 = mgr.createFromStream(iss); + auto r2 = MusicXml::fromStream(iss); REQUIRE(r2.ok()); - const int docId2 = r2.value(); - const auto rd = mgr.getData(docId2); + const auto rd = getScore(std::move(r2).value()); REQUIRE(rd.ok()); - mgr.destroyDocument(docId2); const auto &outNote = rd.value().parts.back().measures.back().staves.back().voices.at(0).notes.back(); CHECK(outNote.pitchData.isAccidentalParenthetical); CHECK(outNote.pitchData.isAccidentalCautionary); diff --git a/src/private/mxtest/api/PrintLayoutRoundTripTest.cpp b/src/private/mxtest/api/PrintLayoutRoundTripTest.cpp index 0ff7f4b9b..89085fda5 100644 --- a/src/private/mxtest/api/PrintLayoutRoundTripTest.cpp +++ b/src/private/mxtest/api/PrintLayoutRoundTripTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" using namespace std; diff --git a/src/private/mxtest/api/RepeatApiTest.cpp b/src/private/mxtest/api/RepeatApiTest.cpp index 10ad1d7c4..c96945915 100644 --- a/src/private/mxtest/api/RepeatApiTest.cpp +++ b/src/private/mxtest/api/RepeatApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/RoundTrip.h b/src/private/mxtest/api/RoundTrip.h index 961aaff6f..0d04be24a 100644 --- a/src/private/mxtest/api/RoundTrip.h +++ b/src/private/mxtest/api/RoundTrip.h @@ -4,7 +4,7 @@ #pragma once -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/control/CompileControl.h" #include "mxtest/file/MxFileRepository.h" #include "mxtest/file/Path.h" @@ -17,46 +17,37 @@ constexpr const char *const roundTripFileName = "k007a_Notations_Dynamics.xml"; inline void roundTrip() { const std::string path{MxFileRepository::getFullPath(roundTripFileName)}; - auto &docMgr = mx::api::DocumentManager::getInstance(); - auto docIdResult = docMgr.createFromFile(path); - if (!docIdResult.ok()) + auto docResult = mx::api::MusicXml::fromFile(path); + if (!docResult.ok()) return; - auto docId = docIdResult.value(); - auto scoreDataResult = docMgr.getData(docId); - docMgr.destroyDocument(docId); + const auto scoreDataResult = mx::api::getScore(std::move(docResult).value()); if (!scoreDataResult.ok()) return; auto scoreData = scoreDataResult.value(); - auto docId2Result = docMgr.createFromScore(scoreData); - if (!docId2Result.ok()) + auto doc2Result = mx::api::fromScore(scoreData); + if (!doc2Result.ok()) return; - auto docId2 = docId2Result.value(); const std::string outputPath = getResourcesDirectoryPath() + "testOutput" + FILE_PATH_SEPARATOR + "output.xml"; - docMgr.writeToFile(docId2, outputPath); - docMgr.destroyDocument(docId2); + std::move(doc2Result).value().writeToFile(outputPath); } inline mx::api::ScoreData roundTrip(const mx::api::ScoreData inScoreData) { - auto &docMgr = mx::api::DocumentManager::getInstance(); - auto docIdResult = docMgr.createFromScore(inScoreData); - if (!docIdResult.ok()) + auto docResult = mx::api::fromScore(inScoreData); + if (!docResult.ok()) return {}; - auto docId = docIdResult.value(); std::stringstream ss; - docMgr.writeToStream(docId, ss); - docMgr.destroyDocument(docId); - auto xmlData = ss.str(); + const auto writeResult = std::move(docResult).value().writeToStream(ss); + if (!writeResult.ok()) + return {}; + const auto xmlData = ss.str(); std::istringstream iss{xmlData}; - auto docId2Result = docMgr.createFromStream(iss); - if (!docId2Result.ok()) + auto doc2Result = mx::api::MusicXml::fromStream(iss); + if (!doc2Result.ok()) return {}; - auto docId2 = docId2Result.value(); - auto outScoreDataResult = docMgr.getData(docId2); - docMgr.destroyDocument(docId2); + const auto outScoreDataResult = mx::api::getScore(std::move(doc2Result).value()); if (!outScoreDataResult.ok()) return {}; - // std::cout << xmlData << std::endl; return outScoreDataResult.value(); } } // namespace mxtest diff --git a/src/private/mxtest/api/ScorePartGroupApiTest.cpp b/src/private/mxtest/api/ScorePartGroupApiTest.cpp index d5ae6a7a8..d4db13484 100644 --- a/src/private/mxtest/api/ScorePartGroupApiTest.cpp +++ b/src/private/mxtest/api/ScorePartGroupApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/SingleNoteSpannerApiTest.cpp b/src/private/mxtest/api/SingleNoteSpannerApiTest.cpp index fdb95c436..990e71d15 100644 --- a/src/private/mxtest/api/SingleNoteSpannerApiTest.cpp +++ b/src/private/mxtest/api/SingleNoteSpannerApiTest.cpp @@ -13,7 +13,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" #include "pugixml.hpp" diff --git a/src/private/mxtest/api/SoundApiTest.cpp b/src/private/mxtest/api/SoundApiTest.cpp index a2efb7ee1..3cd3dca9f 100644 --- a/src/private/mxtest/api/SoundApiTest.cpp +++ b/src/private/mxtest/api/SoundApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/api/ScoreData.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/SpannerIdentityTest.cpp b/src/private/mxtest/api/SpannerIdentityTest.cpp index 0a1ee83b1..efece7cd0 100644 --- a/src/private/mxtest/api/SpannerIdentityTest.cpp +++ b/src/private/mxtest/api/SpannerIdentityTest.cpp @@ -12,7 +12,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/TestHelpers.h" using namespace mx::api; diff --git a/src/private/mxtest/api/StaffCountApiTest.cpp b/src/private/mxtest/api/StaffCountApiTest.cpp index 45aa71623..a1c5cb6bb 100644 --- a/src/private/mxtest/api/StaffCountApiTest.cpp +++ b/src/private/mxtest/api/StaffCountApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/TestHelpers.h b/src/private/mxtest/api/TestHelpers.h index 93786fed6..302b8ef81 100644 --- a/src/private/mxtest/api/TestHelpers.h +++ b/src/private/mxtest/api/TestHelpers.h @@ -4,7 +4,7 @@ #pragma once -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/utility/Throw.h" #include @@ -13,29 +13,24 @@ namespace mxtest inline std::string toXml(const mx::api::ScoreData &inScoreData) { using namespace mx::api; - auto &docMgr = DocumentManager::getInstance(); - const auto docIdResult = docMgr.createFromScore(inScoreData); - if (!docIdResult.ok()) + auto docResult = fromScore(inScoreData); + if (!docResult.ok()) return {}; - const int docId = docIdResult.value(); std::stringstream ss; - docMgr.writeToStream(docId, ss); - docMgr.destroyDocument(docId); - const auto xml = ss.str(); - return xml; + const auto writeResult = std::move(docResult).value().writeToStream(ss); + if (!writeResult.ok()) + return {}; + return ss.str(); } inline mx::api::ScoreData fromXml(const std::string &inXml) { using namespace mx::api; - auto &docMgr = DocumentManager::getInstance(); std::istringstream iss{inXml}; - const auto docIdResult = docMgr.createFromStream(iss); - if (!docIdResult.ok()) + auto docResult = MusicXml::fromStream(iss); + if (!docResult.ok()) return {}; - const int docId = docIdResult.value(); - const auto scoreResult = docMgr.getData(docId); - docMgr.destroyDocument(docId); + const auto scoreResult = getScore(std::move(docResult).value()); if (!scoreResult.ok()) return {}; return scoreResult.value(); diff --git a/src/private/mxtest/api/TimeSignatureApiTest.cpp b/src/private/mxtest/api/TimeSignatureApiTest.cpp index a6cae6974..b30484bfb 100644 --- a/src/private/mxtest/api/TimeSignatureApiTest.cpp +++ b/src/private/mxtest/api/TimeSignatureApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/file/MxFileRepository.h" #include @@ -38,25 +38,20 @@ static ScoreData timeSignatureApiTestScore(const TimeChoice &inTimeSignature, in // serializes the score and reads it back through the api static ScoreData timeSignatureApiTestRoundTrip(const ScoreData &inScore) { - auto &docMgr = DocumentManager::getInstance(); - const auto originalIdResult = docMgr.createFromScore(inScore); + auto originalIdResult = fromScore(inScore); if (!originalIdResult.ok()) { return ScoreData{}; } - const int originalId = originalIdResult.value(); std::stringstream xml; - docMgr.writeToStream(originalId, xml); - docMgr.destroyDocument(originalId); + std::move(originalIdResult).value().writeToStream(xml); std::istringstream iss{xml.str()}; - const auto reloadedIdResult = docMgr.createFromStream(iss); + auto reloadedIdResult = MusicXml::fromStream(iss); if (!reloadedIdResult.ok()) { return ScoreData{}; } - const int reloadedId = reloadedIdResult.value(); - const auto reloadedScoreResult = docMgr.getData(reloadedId); - docMgr.destroyDocument(reloadedId); + const auto reloadedScoreResult = getScore(std::move(reloadedIdResult).value()); if (!reloadedScoreResult.ok()) { return ScoreData{}; diff --git a/src/private/mxtest/api/TranspositionTest.cpp b/src/private/mxtest/api/TranspositionTest.cpp index 8d972721c..a9a0535bb 100644 --- a/src/private/mxtest/api/TranspositionTest.cpp +++ b/src/private/mxtest/api/TranspositionTest.cpp @@ -5,7 +5,7 @@ #include "mxtest/control/CompileControl.h" #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/generated/Attributes.h" #include "mx/core/generated/AttributesChoice.h" #include "mx/core/generated/Document.h" @@ -26,21 +26,16 @@ namespace mxtest // save the file back to disk, load it back up into the API and assert equality. inline mx::api::ScoreData roundtrip(const mx::api::ScoreData &inOriginal) { - auto &docMgr = mx::api::DocumentManager::getInstance(); - const auto r = docMgr.createFromScore(inOriginal); + auto r = mx::api::fromScore(inOriginal); REQUIRE(r.ok()); - const int id = r.value(); std::ostringstream oss; - docMgr.writeToStream(id, oss); + std::move(r).value().writeToStream(oss); std::istringstream iss{oss.str()}; - const auto r2 = docMgr.createFromStream(iss); + auto r2 = mx::api::MusicXml::fromStream(iss); REQUIRE(r2.ok()); - const int id2 = r2.value(); - const auto rd = docMgr.getData(id2); + const auto rd = mx::api::getScore(std::move(r2).value()); REQUIRE(rd.ok()); auto result = rd.value(); - docMgr.destroyDocument(id); - docMgr.destroyDocument(id2); // The write side always emits version="4.0"; normalize the version fields so they // do not prevent a meaningful music-content equality comparison. result.musicXmlVersion = inOriginal.musicXmlVersion; @@ -64,15 +59,12 @@ inline mx::api::ScoreData makeScore(int measures) inline void checkCoreTransposeElement(const mx::api::ScoreData &inScore, int inExpectedChromatic, std::optional inExpectedDiatonic, std::optional inExpectedOctave) { - auto &docMgr = mx::api::DocumentManager::getInstance(); - const auto r = docMgr.createFromScore(inScore); + auto r = mx::api::fromScore(inScore); REQUIRE(r.ok()); - const int id = r.value(); - const auto core = docMgr.getDocument(id); - docMgr.destroyDocument(id); - REQUIRE(core != nullptr); - REQUIRE(core->isScorePartwise()); - const auto &score = core->asScorePartwise(); + const auto doc = std::move(r).value(); + const auto &core = doc.getCoreDocument(); + REQUIRE(core.isScorePartwise()); + const auto &score = core.asScorePartwise(); const auto parts = score.part(); REQUIRE(!parts.empty()); const auto &part = parts[0]; diff --git a/src/private/mxtest/api/VoiceLabelApiTest.cpp b/src/private/mxtest/api/VoiceLabelApiTest.cpp index a60e5b213..af3b58e0b 100644 --- a/src/private/mxtest/api/VoiceLabelApiTest.cpp +++ b/src/private/mxtest/api/VoiceLabelApiTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" diff --git a/src/private/mxtest/api/WavyLineApiTest.cpp b/src/private/mxtest/api/WavyLineApiTest.cpp index 88b00e659..27881e442 100644 --- a/src/private/mxtest/api/WavyLineApiTest.cpp +++ b/src/private/mxtest/api/WavyLineApiTest.cpp @@ -10,7 +10,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" #include "mxtest/file/MxFileRepository.h" diff --git a/src/private/mxtest/file/MxFileRepositoy.cpp b/src/private/mxtest/file/MxFileRepositoy.cpp index 87fabb317..eb131856f 100644 --- a/src/private/mxtest/file/MxFileRepositoy.cpp +++ b/src/private/mxtest/file/MxFileRepositoy.cpp @@ -2,7 +2,7 @@ // Copyright (c) by Matthew James Briggs // Distributed under the MIT License -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mxtest/file/MxFileRepository.h" #include "mxtest/file/Path.h" @@ -123,13 +123,10 @@ void MxFileRepository::initializeTestFiles() mx::api::ScoreData MxFileRepository::loadFile(const std::string &fileName) { const std::string fullPath = getFullPath(fileName); - auto &docMgr = mx::api::DocumentManager::getInstance(); - const auto docIdResult = docMgr.createFromFile(fullPath); - if (!docIdResult.ok()) + auto docResult = mx::api::MusicXml::fromFile(fullPath); + if (!docResult.ok()) return {}; - const int docId = docIdResult.value(); - const auto scoreDataResult = docMgr.getData(docId); - docMgr.destroyDocument(docId); + const auto scoreDataResult = mx::api::getScore(std::move(docResult).value()); if (!scoreDataResult.ok()) return {}; return scoreDataResult.value(); diff --git a/src/private/mxtest/impl/PositionFunctionsTest.cpp b/src/private/mxtest/impl/PositionFunctionsTest.cpp index 0179bb137..0f52616fd 100644 --- a/src/private/mxtest/impl/PositionFunctionsTest.cpp +++ b/src/private/mxtest/impl/PositionFunctionsTest.cpp @@ -6,7 +6,7 @@ #ifdef MX_COMPILE_IMPL_TESTS #include "cpul/cpulTestHarness.h" -#include "mx/api/DocumentManager.h" +#include "mx/api/MusicXml.h" #include "mx/core/generated/Bracket.h" #include "mx/core/generated/Direction.h" #include "mx/core/generated/Stem.h" @@ -180,16 +180,13 @@ TEST(stemDefaultYZeroApiRoundTrip, PositionFunctions) )"; - auto &mgr = api::DocumentManager::getInstance(); std::istringstream iss{xml}; - auto docIdResult = mgr.createFromStream(iss); - CHECK(docIdResult.ok()); - auto docId = docIdResult.value(); + auto docResult = api::MusicXml::fromStream(iss); + CHECK(docResult.ok()); - auto scoreResult = mgr.getData(docId); + auto scoreResult = api::getScore(std::move(docResult).value()); CHECK(scoreResult.ok()); auto scoreData = scoreResult.value(); - mgr.destroyDocument(docId); // Verify the reader populated stemPositionData const auto ¬e = scoreData.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.at(0); @@ -198,12 +195,10 @@ TEST(stemDefaultYZeroApiRoundTrip, PositionFunctions) CHECK_DOUBLES_EQUAL(0.0, note.stemPositionData.defaultY, 0.0001); // Round-trip back to XML - auto docId2Result = mgr.createFromScore(scoreData); - CHECK(docId2Result.ok()); - auto docId2 = docId2Result.value(); + auto doc2Result = api::fromScore(scoreData); + CHECK(doc2Result.ok()); std::stringstream ss; - mgr.writeToStream(docId2, ss); - mgr.destroyDocument(docId2); + std::move(doc2Result).value().writeToStream(ss); const auto output = ss.str(); CHECK(output.find("default-y") != std::string::npos); From 24be1977cae5526ec094cea5e5dea51cfb7f45d5 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sat, 12 Sep 2026 17:42:22 +0200 Subject: [PATCH 2/7] feat: address review of the MusicXml design (#435) Review changes to the MusicXml api: - The move operations now enforce the pimpl invariant: a moved-from MusicXml holds a valid, empty document (a default core::Document) rather than a null pimpl, so every operation on it is safe. The moved-from special cases in writeToFile/writeToStream/getScore are gone; the object guarantees the state. The move constructor allocates the empty replacement so it is not noexcept; move assignment swaps and is. - getCoreDocument is removed from the public api. clone() makes a deep copy instead. The core model is no longer reachable through the public surface at all; mx's own layers and tests reach it through the new private header mx/api/MusicXmlInternal.h (coreDocumentOf), a friend of MusicXml. - Header comments trimmed: the class comment is one sentence, the error variants stay in Result's own documentation, and fromScore keeps only its refusal semantics. - ResultCode::internalError carries a TODO about swallowing exception information. - withWriteVersion's comment reworded to say why it exists without describing the past. All 602 api test cases pass (5495 assertions), including a new clone test and a rewritten moved-from test pinning the empty-document behavior. --- AGENTS.md | 2 +- src/include/mx/api/MusicXml.h | 51 +++++----- src/include/mx/api/Result.h | 4 +- src/private/mx/api/MusicXml.cpp | 96 +++++++++---------- src/private/mx/api/MusicXmlInternal.h | 23 +++++ src/private/mx/impl/ScoreWriter.cpp | 4 +- src/private/mxtest/api/ChordApiTest.cpp | 3 +- .../mxtest/api/ChordDataSaveAndLoadTest.cpp | 3 +- src/private/mxtest/api/ChordTimeTest.cpp | 5 +- src/private/mxtest/api/FreezingRoundTrip.cpp | 53 +++++----- src/private/mxtest/api/KeyDataTest.cpp | 13 +-- src/private/mxtest/api/MusicXmlTest.cpp | 51 ++++++++-- src/private/mxtest/api/NewSystemTest.cpp | 3 +- src/private/mxtest/api/NoteDataTest.cpp | 3 +- src/private/mxtest/api/TranspositionTest.cpp | 3 +- 15 files changed, 187 insertions(+), 130 deletions(-) create mode 100644 src/private/mx/api/MusicXmlInternal.h diff --git a/AGENTS.md b/AGENTS.md index 06bae3e3c..ced0bd6d1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -152,7 +152,7 @@ comment on a PR or the Coverage workflow's "Run workflow" button (`.github/workf | File | What it is | |------|------------| -| `src/include/mx/api/MusicXml.h` | The public API entry point: fromFile, fromStream, fromScore, getScore, intoScore, writeTo* | +| `src/include/mx/api/MusicXml.h` | The public API entry point: fromFile, fromStream, fromScore, getScore, intoScore, clone, writeTo* | | `src/include/mx/api/ScoreData.h` | The primary api data model (ScoreData, PartData, MeasureData, ...) | | `src/private/mx/api/MusicXml.cpp` | API implementation: error channel, parse/serialize orchestration | | `src/private/mx/impl/ScoreReader.cpp` | Translates mx::core -> mx::api ScoreData | diff --git a/src/include/mx/api/MusicXml.h b/src/include/mx/api/MusicXml.h index 14acb4884..b03f8cd80 100644 --- a/src/include/mx/api/MusicXml.h +++ b/src/include/mx/api/MusicXml.h @@ -16,68 +16,63 @@ namespace mx namespace core { class Document; -using DocumentPtr = std::shared_ptr; } // namespace core namespace api { -// A MusicXML document that you own. Parse one from a file or a stream, or -// author one from ScoreData, then read the score back out of it or write it -// to disk. The document is freed automatically when it goes out of scope; -// mx keeps no registry of documents and tracks no ids. A document cannot be -// copied, only moved. +// A MusicXML document, either parsed or constructed from ScoreData. class MusicXml { public: - // parses a .musicxml file. Errors: ioError (file open/read), - // xmlSyntaxError (the bytes are not XML), or the mirrored core parse - // errors. + // Parses a MusicXML file. Logical errors and caught exceptions are + // represented by an error result. static Result fromFile(const std::string &filePath); - // parses from any character stream. Same errors as fromFile, minus the - // file I/O. + // Parses a MusicXML document from a character stream. Logical errors and + // caught exceptions are represented by an error result. static Result fromStream(std::istream &stream); MusicXml(const MusicXml &other) = delete; MusicXml &operator=(const MusicXml &other) = delete; - MusicXml(MusicXml &&other) noexcept; + MusicXml(MusicXml &&other); MusicXml &operator=(MusicXml &&other) noexcept; ~MusicXml(); - // writes the document to a file. Errors: ioError on write failure. + // A deep copy of the document. + MusicXml clone() const; + + // Writes the document to a file. Logical errors and caught exceptions + // are represented by an error result. Result writeToFile(const std::string &filePath) const; - // writes the document to a character stream. Fails only with - // internalError. + // Writes the document to a character stream. Result writeToStream(std::ostream &stream) const; - // access to the underlying core document for requirements that ScoreData - // does not meet. Prefer the score functions above; the core model is a - // much larger interface and it is not frozen the way mx::api is. - const core::Document &getCoreDocument() const; - private: - MusicXml(core::DocumentPtr &&coreDocument, bool writeMxVersion); + MusicXml(); + MusicXml(core::Document document, bool writeMxVersion); class Impl; std::unique_ptr myImpl; friend Result getScore(const MusicXml &document); friend Result fromScore(const ScoreData &score); + friend const core::Document &coreDocumentOf(const MusicXml &document) noexcept; }; -// reads the score out of the document. The document stays alive and can be -// read again or written out. Fails only with internalError. +// Reads the score out of the document. The document stays alive and can be +// read again or written out. Result getScore(const MusicXml &document); -// reads the score out of the document and consumes it: the underlying tree +// Reads the score out of the document and consumes it: the underlying tree // is freed when this function returns rather than when your MusicXml binding // goes out of scope. Pass the document with std::move, or hand over the -// Result's value directly. Fails only with internalError. +// Result's value directly. Result intoScore(MusicXml document); -// authors a new document from ScoreData. CAN fail: when the ScoreData -// describes something the core model will not represent (e.g. more than 8 -// beams), the error is returned rather than silently dropping data. +// Authors a new document from ScoreData. Fails with an error result when the +// ScoreData describes something the core model will not represent (e.g. more +// than 8 beams) rather than silently dropping data. Result fromScore(const ScoreData &score); + } // namespace api } // namespace mx diff --git a/src/include/mx/api/Result.h b/src/include/mx/api/Result.h index 12c7a3dd5..89ef51abc 100644 --- a/src/include/mx/api/Result.h +++ b/src/include/mx/api/Result.h @@ -31,7 +31,9 @@ enum class ResultCode tooManyElements, invalidDocument, unsupportedVersion, // mirrored from the core parse boundary - internalError, // caught exception; nothing escapes (api-level) + // TODO: this badly swallows exception information. we need more variants + // or something like a site and message. + internalError, // caught exception; nothing escapes (api-level) }; struct ApiError diff --git a/src/private/mx/api/MusicXml.cpp b/src/private/mx/api/MusicXml.cpp index 39c9ee01a..a8639315d 100644 --- a/src/private/mx/api/MusicXml.cpp +++ b/src/private/mx/api/MusicXml.cpp @@ -3,6 +3,7 @@ // Distributed under the MIT License #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/Attribution.h" #include "mx/core/Error.h" #include "mx/core/generated/Document.h" @@ -19,10 +20,10 @@ namespace mx { namespace api { -// The write side always emits version="4.0" unconditionally: echoing a -// declared "3.0" (or ScoreData::musicXmlVersion) from a 4.0 model was a -// fiction. Enforced here at the write boundary on a copy, so the owned -// document (and the getCoreDocument escape hatch) keeps what was parsed. +// mx's data model is MusicXML 4.0, so every write states version="4.0" on +// the root element, whatever version a parsed document declared. The +// override happens here on a copy at the write boundary, so the owned +// document keeps what was parsed. core::Document withWriteVersion(const core::Document &document) { core::Document copy = document; @@ -93,28 +94,44 @@ class MusicXml::Impl public: // writeMxVersion governs whether writeTo*() stamps mx's provenance // (see EncodingData::writeMxVersion); it defaults true, - // including for parsed documents (whose source never had the stamp). - Impl(core::DocumentPtr inDocument, bool inWriteMxVersion) + // including for parsed documents (whose source did not have the stamp). + Impl() = default; + + Impl(core::Document inDocument, bool inWriteMxVersion) : document{std::move(inDocument)}, writeMxVersion{inWriteMxVersion} { } - core::DocumentPtr document; - bool writeMxVersion; + // The natural zero: a default-constructed ScorePartwise, i.e. a valid, + // empty document. + core::Document document; + bool writeMxVersion = true; }; -MusicXml::MusicXml(core::DocumentPtr &&coreDocument, bool writeMxVersion) - : myImpl{new MusicXml::Impl{std::move(coreDocument), writeMxVersion}} +MusicXml::MusicXml() : myImpl{new MusicXml::Impl{}} +{ +} + +MusicXml::MusicXml(core::Document document, bool writeMxVersion) + : myImpl{new MusicXml::Impl{std::move(document), writeMxVersion}} { } -MusicXml::MusicXml(MusicXml &&other) noexcept : myImpl{std::move(other.myImpl)} +// The move operations never leave a null pimpl behind. A moved-from MusicXml +// holds a valid, empty document, so it is safe to read and write. The +// constructor allocates the empty replacement first, so it is not noexcept; +// the assignment just swaps and is. +MusicXml::MusicXml(MusicXml &&other) : myImpl{std::make_unique()} { + std::swap(myImpl, other.myImpl); } MusicXml &MusicXml::operator=(MusicXml &&other) noexcept { - myImpl = std::move(other.myImpl); + if (this != &other) + { + std::swap(myImpl, other.myImpl); + } return *this; } @@ -152,8 +169,7 @@ Result MusicXml::fromFile(const std::string &filePath) return mirrorToApiError(parsed.error()); } - core::DocumentPtr mxdoc = std::make_shared(std::move(parsed).value()); - return MusicXml{std::move(mxdoc), true}; + return MusicXml{core::Document{std::move(parsed).value()}, true}; } catch (const std::exception &e) { @@ -182,8 +198,7 @@ Result MusicXml::fromStream(std::istream &stream) return mirrorToApiError(parsed.error()); } - core::DocumentPtr mxdoc = std::make_shared(std::move(parsed).value()); - return MusicXml{std::move(mxdoc), true}; + return MusicXml{core::Document{std::move(parsed).value()}, true}; } catch (const std::exception &e) { @@ -199,13 +214,8 @@ Result MusicXml::writeToFile(const std::string &filePath) const { try { - if (!myImpl) - { - return musicXmlInternalError("MusicXml::writeToFile", "the document has been moved from"); - } - pugi::xml_document xdoc; - const core::Document toWrite = withWriteVersion(*myImpl->document); + const core::Document toWrite = withWriteVersion(myImpl->document); if (myImpl->writeMxVersion) { core::serializeWithAttribution(toWrite, xdoc); @@ -234,13 +244,8 @@ Result MusicXml::writeToStream(std::ostream &stream) const { try { - if (!myImpl) - { - return musicXmlInternalError("MusicXml::writeToStream", "the document has been moved from"); - } - pugi::xml_document xdoc; - const core::Document toWrite = withWriteVersion(*myImpl->document); + const core::Document toWrite = withWriteVersion(myImpl->document); if (myImpl->writeMxVersion) { core::serializeWithAttribution(toWrite, xdoc); @@ -262,16 +267,17 @@ Result MusicXml::writeToStream(std::ostream &stream) const } } -const core::Document &MusicXml::getCoreDocument() const +MusicXml MusicXml::clone() const { - if (myImpl) - { - return *myImpl->document; - } - // a moved-from document holds nothing; reading it yields an empty core - // document rather than a crash - static const core::Document emptyDocument{}; - return emptyDocument; + MusicXml cloned{}; + cloned.myImpl->document = myImpl->document; + cloned.myImpl->writeMxVersion = myImpl->writeMxVersion; + return cloned; +} + +const core::Document &coreDocumentOf(const MusicXml &document) noexcept +{ + return document.myImpl->document; } Result fromScore(const ScoreData &score) @@ -281,17 +287,12 @@ Result fromScore(const ScoreData &score) impl::ScoreWriter writer{score}; core::ScorePartwise scorePartwise = writer.getScorePartwise(); - core::DocumentPtr mxdoc; if (score.musicXmlType == "timewise") { - mxdoc = std::make_shared(impl::partwiseTimewise(scorePartwise)); - } - else - { - mxdoc = std::make_shared(std::move(scorePartwise)); + return MusicXml{core::Document{impl::partwiseTimewise(scorePartwise)}, score.encoding.writeMxVersion}; } - return MusicXml{std::move(mxdoc), score.encoding.writeMxVersion}; + return MusicXml{core::Document{std::move(scorePartwise)}, score.encoding.writeMxVersion}; } catch (const impl::WriteRefusal &refusal) { @@ -313,12 +314,7 @@ Result getScore(const MusicXml &document) { try { - if (!document.myImpl) - { - return musicXmlInternalError("getScore", "the document has been moved from"); - } - - const core::Document &coreDocument = *document.myImpl->document; + const core::Document &coreDocument = document.myImpl->document; // Convert a timewise document into a local partwise copy and read // that; the owned document is untouched. diff --git a/src/private/mx/api/MusicXmlInternal.h b/src/private/mx/api/MusicXmlInternal.h new file mode 100644 index 000000000..84f18877e --- /dev/null +++ b/src/private/mx/api/MusicXmlInternal.h @@ -0,0 +1,23 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#pragma once + +#include "mx/api/MusicXml.h" + +namespace mx +{ +namespace core +{ +class Document; +} // namespace core + +namespace api +{ +// Internal to mx: the core document owned by a MusicXml. The public api does +// not expose the core model; mx's own layers and its tests reach it through +// here. +const core::Document &coreDocumentOf(const MusicXml &document) noexcept; +} // namespace api +} // namespace mx diff --git a/src/private/mx/impl/ScoreWriter.cpp b/src/private/mx/impl/ScoreWriter.cpp index e4ef84e9c..c439857f2 100644 --- a/src/private/mx/impl/ScoreWriter.cpp +++ b/src/private/mx/impl/ScoreWriter.cpp @@ -57,8 +57,8 @@ core::ScorePartwise ScoreWriter::getScorePartwise() const break; // ThreePointZero also represents parsed "4.0" documents (see ScoreReader). The "3.0" - // written here only reaches the getCoreDocument escape hatch -- writeTo*() overrides the - // version to "4.0" via withWriteVersion at the api boundary. + // written here exists only inside the owned core document (coreDocumentOf) -- writeTo*() + // overrides the version to "4.0" via withWriteVersion at the api boundary. case api::MusicXmlVersion::ThreePointZero: { myOutScorePartwise.setVersion(std::string{"3.0"}); } diff --git a/src/private/mxtest/api/ChordApiTest.cpp b/src/private/mxtest/api/ChordApiTest.cpp index 07626a3eb..a064a912f 100644 --- a/src/private/mxtest/api/ChordApiTest.cpp +++ b/src/private/mxtest/api/ChordApiTest.cpp @@ -7,6 +7,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/FullNoteGroup.h" #include "mx/core/generated/MusicDataChoice.h" @@ -245,7 +246,7 @@ TEST(KompChordBug_PIVOTAL_147058063, ChordApi) auto docIdResult = fromScore(originalScore); REQUIRE(docIdResult.ok()); const auto document = std::move(docIdResult).value(); - const auto &coreDoc = document.getCoreDocument(); + const auto &coreDoc = coreDocumentOf(document); REQUIRE(coreDoc.isScorePartwise()); const auto &scorePartwise = coreDoc.asScorePartwise(); const auto xml = mxtest::toXml(originalScore); diff --git a/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp b/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp index ee804dbde..b53d1787a 100644 --- a/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp +++ b/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp @@ -7,6 +7,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Bass.h" #include "mx/core/generated/BassStep.h" #include "mx/core/generated/Document.h" @@ -41,7 +42,7 @@ TEST(Save, ChordDataSaveTest) auto docIdResult = fromScore(scoreData); REQUIRE(docIdResult.ok()); const auto document = std::move(docIdResult).value(); - const auto &coreDoc = document.getCoreDocument(); + const auto &coreDoc = coreDocumentOf(document); REQUIRE(coreDoc.isScorePartwise()); const auto &scorePartwise = coreDoc.asScorePartwise(); diff --git a/src/private/mxtest/api/ChordTimeTest.cpp b/src/private/mxtest/api/ChordTimeTest.cpp index 652013303..2b2097442 100644 --- a/src/private/mxtest/api/ChordTimeTest.cpp +++ b/src/private/mxtest/api/ChordTimeTest.cpp @@ -8,6 +8,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/FullNoteGroup.h" #include "mx/core/generated/MusicDataChoice.h" @@ -80,8 +81,8 @@ TEST(chordTest, Chords) std::stringstream ss; doc.writeToStream(ss); - REQUIRE(doc.getCoreDocument().isScorePartwise()); - const auto &scorePartwise = doc.getCoreDocument().asScorePartwise(); + REQUIRE(coreDocumentOf(doc).isScorePartwise()); + const auto &scorePartwise = coreDocumentOf(doc).asScorePartwise(); const auto parts = scorePartwise.part(); REQUIRE(!parts.empty()); const auto &firstPart = parts[0]; diff --git a/src/private/mxtest/api/FreezingRoundTrip.cpp b/src/private/mxtest/api/FreezingRoundTrip.cpp index c23f7d201..d526b9d98 100644 --- a/src/private/mxtest/api/FreezingRoundTrip.cpp +++ b/src/private/mxtest/api/FreezingRoundTrip.cpp @@ -7,6 +7,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/DirectionTypeChoice.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/DynamicsChoice.h" @@ -74,12 +75,12 @@ struct TestData const mx::core::ScorePartwise &originalScore() const { - return originalDoc.getCoreDocument().asScorePartwise(); + return coreDocumentOf(originalDoc).asScorePartwise(); } const mx::core::ScorePartwise &savedScore() const { - return savedDoc.getCoreDocument().asScorePartwise(); + return coreDocumentOf(savedDoc).asScorePartwise(); } //////////////////////////// original score ///////////////////// saved score /////////////////// @@ -177,8 +178,8 @@ TEST(roundTripViolaDynamicWrongTime, Freezing) const size_t partIndex = 0; const size_t measureIndex = 7; - const auto &originalScore = originalDoc.getCoreDocument().asScorePartwise(); - const auto &savedScore = savedDoc.getCoreDocument().asScorePartwise(); + const auto &originalScore = coreDocumentOf(originalDoc).asScorePartwise(); + const auto &savedScore = coreDocumentOf(savedDoc).asScorePartwise(); const auto originalMdcSpan = originalScore.part()[partIndex].measure()[measureIndex].musicData(); auto originalMdcIter = originalMdcSpan.begin(); @@ -313,8 +314,8 @@ TEST(missingMusicXMLVersion, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - const bool originalScoreHasVersion = originalDoc.getCoreDocument().asScorePartwise().version().has_value(); - const bool savedScoreHasVersion = savedDoc.getCoreDocument().asScorePartwise().version().has_value(); + const bool originalScoreHasVersion = coreDocumentOf(originalDoc).asScorePartwise().version().has_value(); + const bool savedScoreHasVersion = coreDocumentOf(savedDoc).asScorePartwise().version().has_value(); CHECK(originalScoreHasVersion); CHECK(savedScoreHasVersion); } @@ -331,8 +332,8 @@ TEST(HasDefaultsHasAppearance, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - const auto &origHeader = originalDoc.getCoreDocument().asScorePartwise().scoreHeader(); - const auto &savedHeader = savedDoc.getCoreDocument().asScorePartwise().scoreHeader(); + const auto &origHeader = coreDocumentOf(originalDoc).asScorePartwise().scoreHeader(); + const auto &savedHeader = coreDocumentOf(savedDoc).asScorePartwise().scoreHeader(); const bool originalHasDefaults = origHeader.defaults().has_value(); const bool savedHasDefaults = savedHeader.defaults().has_value(); @@ -392,15 +393,15 @@ TEST(appearanceLineWidths, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); const auto &originalAppearance = - originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); + coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto &savedAppearance = - savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); + coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto lineWidthSetSize = savedAppearance.lineWidth().size(); CHECK(lineWidthSetSize > 0); @@ -430,15 +431,15 @@ TEST(appearanceNoteSize, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); const auto &originalAppearance = - originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); + coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto &savedAppearance = - savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); + coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto noteSizeSetSize = savedAppearance.noteSize().size(); CHECK(noteSizeSetSize > 0); @@ -468,15 +469,15 @@ TEST(appearancDistance, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); const auto &originalAppearance = - originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); + coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto &savedAppearance = - savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); + coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto distanceSetSize = savedAppearance.distance().size(); CHECK(distanceSetSize > 0); diff --git a/src/private/mxtest/api/KeyDataTest.cpp b/src/private/mxtest/api/KeyDataTest.cpp index 1e4a9c73f..6ac0ec06d 100644 --- a/src/private/mxtest/api/KeyDataTest.cpp +++ b/src/private/mxtest/api/KeyDataTest.cpp @@ -9,6 +9,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Attributes.h" #include "mx/core/generated/Cancel.h" #include "mx/core/generated/Document.h" @@ -127,7 +128,7 @@ TEST(EMajor, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); + const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -191,7 +192,7 @@ TEST(AbMinor, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); + const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -251,7 +252,7 @@ TEST(NonTraditional1, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); + const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -479,7 +480,7 @@ TEST(CancelLocationBeforeBarline, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); + const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -524,7 +525,7 @@ TEST(CancelLocationUnspecified, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); + const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -726,7 +727,7 @@ TEST(ModeNoneIsNotNonTraditional, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); + const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); const auto &coreKey = getFirstCoreKey(coreDoc); CHECK(coreKey.choice().isTraditionalKey()); diff --git a/src/private/mxtest/api/MusicXmlTest.cpp b/src/private/mxtest/api/MusicXmlTest.cpp index cc8404082..13691e225 100644 --- a/src/private/mxtest/api/MusicXmlTest.cpp +++ b/src/private/mxtest/api/MusicXmlTest.cpp @@ -8,6 +8,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/DefaultsData.h" #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/Attribution.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/MarginType.h" @@ -48,8 +49,8 @@ inline ScoreData roundTripScore(const ScoreData &input) } // --- RAII ownership --------------------------------------------------------- -// A MusicXml owns its document. It cannot be copied, it moves, and a -// moved-from document fails safely on every path rather than crashing. +// A MusicXml owns its document. It cannot be copied (clone makes a deep +// copy), it moves, and a moved-from document is a valid, empty document. TEST(moveTransfersOwnership, MusicXml) { @@ -64,14 +65,46 @@ TEST(moveTransfersOwnership, MusicXml) REQUIRE(scoreResult.ok()); CHECK_EQUAL("Dichterliebe", scoreResult.value().workTitle); - // the moved-from document fails safely on the read and write paths + // the moved-from document is a valid, empty document, safe to read and write const auto movedScoreResult = getScore(doc); - CHECK(!movedScoreResult.ok()); - CHECK(movedScoreResult.error().code == ResultCode::internalError); + REQUIRE(movedScoreResult.ok()); + CHECK(movedScoreResult.value().workTitle.empty()); std::stringstream ss; const auto movedWriteResult = doc.writeToStream(ss); - CHECK(!movedWriteResult.ok()); - CHECK(movedWriteResult.error().code == ResultCode::internalError); + REQUIRE(movedWriteResult.ok()); + CHECK(!ss.str().empty()); +} + +T_END + +TEST(clone, MusicXml) +{ + auto input = ScoreData{}; + input.workTitle = "CloneTest"; + auto docResult = fromScore(input); + REQUIRE(docResult.ok()); + const auto original = std::move(docResult).value(); + + const auto cloned = original.clone(); + + // the copy and the original are both usable and hold the same score + const auto a = getScore(original); + const auto b = getScore(cloned); + REQUIRE(a.ok()); + REQUIRE(b.ok()); + CHECK_EQUAL("CloneTest", a.value().workTitle); + CHECK_EQUAL("CloneTest", b.value().workTitle); + + // the copy is deep: writing it does not disturb the original + auto edited = b.value(); + edited.workTitle = "Edited"; + auto rewritten = fromScore(edited); + REQUIRE(rewritten.ok()); + std::stringstream ss; + std::move(rewritten).value().writeToStream(ss); + const auto reread = getScore(original); + REQUIRE(reread.ok()); + CHECK_EQUAL("CloneTest", reread.value().workTitle); } T_END @@ -347,7 +380,7 @@ TEST(Layout_PageMarginsBoth, MusicXml) auto docResult = fromScore(score); REQUIRE(docResult.ok()); const MusicXml doc = std::move(docResult).value(); - const auto &mxDoc = doc.getCoreDocument(); + const auto &mxDoc = coreDocumentOf(doc); REQUIRE(mxDoc.isScorePartwise()); const auto &defaults = mxDoc.asScorePartwise().scoreHeader().defaults(); REQUIRE(defaults.has_value()); @@ -384,7 +417,7 @@ TEST(Layout_PageMarginsEvenOdd, MusicXml) auto docResult = fromScore(score); REQUIRE(docResult.ok()); const MusicXml doc = std::move(docResult).value(); - const auto &mxDoc = doc.getCoreDocument(); + const auto &mxDoc = coreDocumentOf(doc); REQUIRE(mxDoc.isScorePartwise()); const auto &defaults = mxDoc.asScorePartwise().scoreHeader().defaults(); REQUIRE(defaults.has_value()); diff --git a/src/private/mxtest/api/NewSystemTest.cpp b/src/private/mxtest/api/NewSystemTest.cpp index cd9e16b02..72ff7ff81 100644 --- a/src/private/mxtest/api/NewSystemTest.cpp +++ b/src/private/mxtest/api/NewSystemTest.cpp @@ -7,6 +7,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/MusicDataChoice.h" #include "mx/core/generated/Print.h" @@ -67,7 +68,7 @@ TEST(newSystem, doesItWork) auto rId = fromScore(s); REQUIRE(rId.ok()); const auto doc = std::move(rId).value(); - const auto &sp = doc.getCoreDocument().asScorePartwise(); + const auto &sp = coreDocumentOf(doc).asScorePartwise(); const auto &p = sp.part()[0]; size_t index = 0; diff --git a/src/private/mxtest/api/NoteDataTest.cpp b/src/private/mxtest/api/NoteDataTest.cpp index 7264add70..fe640492b 100644 --- a/src/private/mxtest/api/NoteDataTest.cpp +++ b/src/private/mxtest/api/NoteDataTest.cpp @@ -8,6 +8,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Direction.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/MusicDataChoice.h" @@ -1111,7 +1112,7 @@ TEST(directionOrder, NoteData) std::stringstream ss; doc.writeToStream(ss); - const auto &docPtr = doc.getCoreDocument(); + const auto &docPtr = coreDocumentOf(doc); REQUIRE(docPtr.isScorePartwise()); const auto &partwise = docPtr.asScorePartwise(); const auto partwiseParts = partwise.part(); diff --git a/src/private/mxtest/api/TranspositionTest.cpp b/src/private/mxtest/api/TranspositionTest.cpp index a9a0535bb..416301027 100644 --- a/src/private/mxtest/api/TranspositionTest.cpp +++ b/src/private/mxtest/api/TranspositionTest.cpp @@ -6,6 +6,7 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" +#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Attributes.h" #include "mx/core/generated/AttributesChoice.h" #include "mx/core/generated/Document.h" @@ -62,7 +63,7 @@ inline void checkCoreTransposeElement(const mx::api::ScoreData &inScore, int inE auto r = mx::api::fromScore(inScore); REQUIRE(r.ok()); const auto doc = std::move(r).value(); - const auto &core = doc.getCoreDocument(); + const auto &core = coreDocumentOf(doc); REQUIRE(core.isScorePartwise()); const auto &score = core.asScorePartwise(); const auto parts = score.part(); From 1fbd12703393a059345e9d28cb5c0bc8e7b07c19 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sat, 12 Sep 2026 17:53:24 +0200 Subject: [PATCH 3/7] feat: restore getCoreDocument as a public escape hatch (#435) Reversing my previous take on the review: removing the escape hatch was a mistake. getCoreDocument is back on MusicXml, documented as an escape hatch for when mx::api does not do what you need and you want to edit the core DOM directly. The non-const overload allows editing; the const overload reads. The private header indirection (MusicXmlInternal.h and coreDocumentOf) is removed; the tests that assert on the core tree use the public method again. --- src/include/mx/api/MusicXml.h | 8 ++- src/private/mx/api/MusicXml.cpp | 10 ++-- src/private/mx/api/MusicXmlInternal.h | 23 -------- src/private/mx/impl/ScoreWriter.cpp | 4 +- src/private/mxtest/api/ChordApiTest.cpp | 3 +- .../mxtest/api/ChordDataSaveAndLoadTest.cpp | 3 +- src/private/mxtest/api/ChordTimeTest.cpp | 5 +- src/private/mxtest/api/FreezingRoundTrip.cpp | 53 +++++++++---------- src/private/mxtest/api/KeyDataTest.cpp | 13 +++-- src/private/mxtest/api/MusicXmlTest.cpp | 5 +- src/private/mxtest/api/NewSystemTest.cpp | 3 +- src/private/mxtest/api/NoteDataTest.cpp | 3 +- src/private/mxtest/api/TranspositionTest.cpp | 3 +- 13 files changed, 57 insertions(+), 79 deletions(-) delete mode 100644 src/private/mx/api/MusicXmlInternal.h diff --git a/src/include/mx/api/MusicXml.h b/src/include/mx/api/MusicXml.h index b03f8cd80..83f97fe05 100644 --- a/src/include/mx/api/MusicXml.h +++ b/src/include/mx/api/MusicXml.h @@ -48,6 +48,13 @@ class MusicXml // Writes the document to a character stream. Result writeToStream(std::ostream &stream) const; + // This is an escape hatch in case mx::api does not do what you need and + // you want to edit the core DOM directly. You will need to include the + // private mx::core headers in your header search paths to do so. Not + // recommended, try opening an issue first! + core::Document &getCoreDocument(); + const core::Document &getCoreDocument() const; + private: MusicXml(); MusicXml(core::Document document, bool writeMxVersion); @@ -56,7 +63,6 @@ class MusicXml friend Result getScore(const MusicXml &document); friend Result fromScore(const ScoreData &score); - friend const core::Document &coreDocumentOf(const MusicXml &document) noexcept; }; // Reads the score out of the document. The document stays alive and can be diff --git a/src/private/mx/api/MusicXml.cpp b/src/private/mx/api/MusicXml.cpp index a8639315d..16607d5ce 100644 --- a/src/private/mx/api/MusicXml.cpp +++ b/src/private/mx/api/MusicXml.cpp @@ -3,7 +3,6 @@ // Distributed under the MIT License #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/Attribution.h" #include "mx/core/Error.h" #include "mx/core/generated/Document.h" @@ -275,9 +274,14 @@ MusicXml MusicXml::clone() const return cloned; } -const core::Document &coreDocumentOf(const MusicXml &document) noexcept +core::Document &MusicXml::getCoreDocument() { - return document.myImpl->document; + return myImpl->document; +} + +const core::Document &MusicXml::getCoreDocument() const +{ + return myImpl->document; } Result fromScore(const ScoreData &score) diff --git a/src/private/mx/api/MusicXmlInternal.h b/src/private/mx/api/MusicXmlInternal.h deleted file mode 100644 index 84f18877e..000000000 --- a/src/private/mx/api/MusicXmlInternal.h +++ /dev/null @@ -1,23 +0,0 @@ -// MusicXML Class Library -// Copyright (c) by Matthew James Briggs -// Distributed under the MIT License - -#pragma once - -#include "mx/api/MusicXml.h" - -namespace mx -{ -namespace core -{ -class Document; -} // namespace core - -namespace api -{ -// Internal to mx: the core document owned by a MusicXml. The public api does -// not expose the core model; mx's own layers and its tests reach it through -// here. -const core::Document &coreDocumentOf(const MusicXml &document) noexcept; -} // namespace api -} // namespace mx diff --git a/src/private/mx/impl/ScoreWriter.cpp b/src/private/mx/impl/ScoreWriter.cpp index c439857f2..e4ef84e9c 100644 --- a/src/private/mx/impl/ScoreWriter.cpp +++ b/src/private/mx/impl/ScoreWriter.cpp @@ -57,8 +57,8 @@ core::ScorePartwise ScoreWriter::getScorePartwise() const break; // ThreePointZero also represents parsed "4.0" documents (see ScoreReader). The "3.0" - // written here exists only inside the owned core document (coreDocumentOf) -- writeTo*() - // overrides the version to "4.0" via withWriteVersion at the api boundary. + // written here only reaches the getCoreDocument escape hatch -- writeTo*() overrides the + // version to "4.0" via withWriteVersion at the api boundary. case api::MusicXmlVersion::ThreePointZero: { myOutScorePartwise.setVersion(std::string{"3.0"}); } diff --git a/src/private/mxtest/api/ChordApiTest.cpp b/src/private/mxtest/api/ChordApiTest.cpp index a064a912f..07626a3eb 100644 --- a/src/private/mxtest/api/ChordApiTest.cpp +++ b/src/private/mxtest/api/ChordApiTest.cpp @@ -7,7 +7,6 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/FullNoteGroup.h" #include "mx/core/generated/MusicDataChoice.h" @@ -246,7 +245,7 @@ TEST(KompChordBug_PIVOTAL_147058063, ChordApi) auto docIdResult = fromScore(originalScore); REQUIRE(docIdResult.ok()); const auto document = std::move(docIdResult).value(); - const auto &coreDoc = coreDocumentOf(document); + const auto &coreDoc = document.getCoreDocument(); REQUIRE(coreDoc.isScorePartwise()); const auto &scorePartwise = coreDoc.asScorePartwise(); const auto xml = mxtest::toXml(originalScore); diff --git a/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp b/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp index b53d1787a..ee804dbde 100644 --- a/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp +++ b/src/private/mxtest/api/ChordDataSaveAndLoadTest.cpp @@ -7,7 +7,6 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Bass.h" #include "mx/core/generated/BassStep.h" #include "mx/core/generated/Document.h" @@ -42,7 +41,7 @@ TEST(Save, ChordDataSaveTest) auto docIdResult = fromScore(scoreData); REQUIRE(docIdResult.ok()); const auto document = std::move(docIdResult).value(); - const auto &coreDoc = coreDocumentOf(document); + const auto &coreDoc = document.getCoreDocument(); REQUIRE(coreDoc.isScorePartwise()); const auto &scorePartwise = coreDoc.asScorePartwise(); diff --git a/src/private/mxtest/api/ChordTimeTest.cpp b/src/private/mxtest/api/ChordTimeTest.cpp index 2b2097442..652013303 100644 --- a/src/private/mxtest/api/ChordTimeTest.cpp +++ b/src/private/mxtest/api/ChordTimeTest.cpp @@ -8,7 +8,6 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/FullNoteGroup.h" #include "mx/core/generated/MusicDataChoice.h" @@ -81,8 +80,8 @@ TEST(chordTest, Chords) std::stringstream ss; doc.writeToStream(ss); - REQUIRE(coreDocumentOf(doc).isScorePartwise()); - const auto &scorePartwise = coreDocumentOf(doc).asScorePartwise(); + REQUIRE(doc.getCoreDocument().isScorePartwise()); + const auto &scorePartwise = doc.getCoreDocument().asScorePartwise(); const auto parts = scorePartwise.part(); REQUIRE(!parts.empty()); const auto &firstPart = parts[0]; diff --git a/src/private/mxtest/api/FreezingRoundTrip.cpp b/src/private/mxtest/api/FreezingRoundTrip.cpp index d526b9d98..c23f7d201 100644 --- a/src/private/mxtest/api/FreezingRoundTrip.cpp +++ b/src/private/mxtest/api/FreezingRoundTrip.cpp @@ -7,7 +7,6 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/DirectionTypeChoice.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/DynamicsChoice.h" @@ -75,12 +74,12 @@ struct TestData const mx::core::ScorePartwise &originalScore() const { - return coreDocumentOf(originalDoc).asScorePartwise(); + return originalDoc.getCoreDocument().asScorePartwise(); } const mx::core::ScorePartwise &savedScore() const { - return coreDocumentOf(savedDoc).asScorePartwise(); + return savedDoc.getCoreDocument().asScorePartwise(); } //////////////////////////// original score ///////////////////// saved score /////////////////// @@ -178,8 +177,8 @@ TEST(roundTripViolaDynamicWrongTime, Freezing) const size_t partIndex = 0; const size_t measureIndex = 7; - const auto &originalScore = coreDocumentOf(originalDoc).asScorePartwise(); - const auto &savedScore = coreDocumentOf(savedDoc).asScorePartwise(); + const auto &originalScore = originalDoc.getCoreDocument().asScorePartwise(); + const auto &savedScore = savedDoc.getCoreDocument().asScorePartwise(); const auto originalMdcSpan = originalScore.part()[partIndex].measure()[measureIndex].musicData(); auto originalMdcIter = originalMdcSpan.begin(); @@ -314,8 +313,8 @@ TEST(missingMusicXMLVersion, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - const bool originalScoreHasVersion = coreDocumentOf(originalDoc).asScorePartwise().version().has_value(); - const bool savedScoreHasVersion = coreDocumentOf(savedDoc).asScorePartwise().version().has_value(); + const bool originalScoreHasVersion = originalDoc.getCoreDocument().asScorePartwise().version().has_value(); + const bool savedScoreHasVersion = savedDoc.getCoreDocument().asScorePartwise().version().has_value(); CHECK(originalScoreHasVersion); CHECK(savedScoreHasVersion); } @@ -332,8 +331,8 @@ TEST(HasDefaultsHasAppearance, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - const auto &origHeader = coreDocumentOf(originalDoc).asScorePartwise().scoreHeader(); - const auto &savedHeader = coreDocumentOf(savedDoc).asScorePartwise().scoreHeader(); + const auto &origHeader = originalDoc.getCoreDocument().asScorePartwise().scoreHeader(); + const auto &savedHeader = savedDoc.getCoreDocument().asScorePartwise().scoreHeader(); const bool originalHasDefaults = origHeader.defaults().has_value(); const bool savedHasDefaults = savedHeader.defaults().has_value(); @@ -393,15 +392,15 @@ TEST(appearanceLineWidths, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); const auto &originalAppearance = - coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); + originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto &savedAppearance = - coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); + savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto lineWidthSetSize = savedAppearance.lineWidth().size(); CHECK(lineWidthSetSize > 0); @@ -431,15 +430,15 @@ TEST(appearanceNoteSize, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); const auto &originalAppearance = - coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); + originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto &savedAppearance = - coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); + savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto noteSizeSetSize = savedAppearance.noteSize().size(); CHECK(noteSizeSetSize > 0); @@ -469,15 +468,15 @@ TEST(appearancDistance, Freezing) REQUIRE(rSaved.ok()); MusicXml savedDoc = std::move(rSaved).value(); - REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults().has_value()); - REQUIRE(coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); - REQUIRE(coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults().has_value()); + REQUIRE(originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); + REQUIRE(savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().has_value()); const auto &originalAppearance = - coreDocumentOf(originalDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); + originalDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto &savedAppearance = - coreDocumentOf(savedDoc).asScorePartwise().scoreHeader().defaults()->appearance().value(); + savedDoc.getCoreDocument().asScorePartwise().scoreHeader().defaults()->appearance().value(); const auto distanceSetSize = savedAppearance.distance().size(); CHECK(distanceSetSize > 0); diff --git a/src/private/mxtest/api/KeyDataTest.cpp b/src/private/mxtest/api/KeyDataTest.cpp index 6ac0ec06d..1e4a9c73f 100644 --- a/src/private/mxtest/api/KeyDataTest.cpp +++ b/src/private/mxtest/api/KeyDataTest.cpp @@ -9,7 +9,6 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Attributes.h" #include "mx/core/generated/Cancel.h" #include "mx/core/generated/Document.h" @@ -128,7 +127,7 @@ TEST(EMajor, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -192,7 +191,7 @@ TEST(AbMinor, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -252,7 +251,7 @@ TEST(NonTraditional1, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -480,7 +479,7 @@ TEST(CancelLocationBeforeBarline, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -525,7 +524,7 @@ TEST(CancelLocationUnspecified, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); const auto &coreKey = getFirstCoreKey(coreDoc); const auto &coreKeyChoice = coreKey.choice(); @@ -727,7 +726,7 @@ TEST(ModeNoneIsNotNonTraditional, KeyData) auto originalIdResult = fromScore(original); REQUIRE(originalIdResult.ok()); const auto originalDoc = std::move(originalIdResult).value(); - const mx::core::Document &coreDoc = coreDocumentOf(originalDoc); + const mx::core::Document &coreDoc = originalDoc.getCoreDocument(); const auto &coreKey = getFirstCoreKey(coreDoc); CHECK(coreKey.choice().isTraditionalKey()); diff --git a/src/private/mxtest/api/MusicXmlTest.cpp b/src/private/mxtest/api/MusicXmlTest.cpp index 13691e225..ca8ef5e41 100644 --- a/src/private/mxtest/api/MusicXmlTest.cpp +++ b/src/private/mxtest/api/MusicXmlTest.cpp @@ -8,7 +8,6 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/DefaultsData.h" #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/Attribution.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/MarginType.h" @@ -380,7 +379,7 @@ TEST(Layout_PageMarginsBoth, MusicXml) auto docResult = fromScore(score); REQUIRE(docResult.ok()); const MusicXml doc = std::move(docResult).value(); - const auto &mxDoc = coreDocumentOf(doc); + const auto &mxDoc = doc.getCoreDocument(); REQUIRE(mxDoc.isScorePartwise()); const auto &defaults = mxDoc.asScorePartwise().scoreHeader().defaults(); REQUIRE(defaults.has_value()); @@ -417,7 +416,7 @@ TEST(Layout_PageMarginsEvenOdd, MusicXml) auto docResult = fromScore(score); REQUIRE(docResult.ok()); const MusicXml doc = std::move(docResult).value(); - const auto &mxDoc = coreDocumentOf(doc); + const auto &mxDoc = doc.getCoreDocument(); REQUIRE(mxDoc.isScorePartwise()); const auto &defaults = mxDoc.asScorePartwise().scoreHeader().defaults(); REQUIRE(defaults.has_value()); diff --git a/src/private/mxtest/api/NewSystemTest.cpp b/src/private/mxtest/api/NewSystemTest.cpp index 72ff7ff81..cd9e16b02 100644 --- a/src/private/mxtest/api/NewSystemTest.cpp +++ b/src/private/mxtest/api/NewSystemTest.cpp @@ -7,7 +7,6 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/MusicDataChoice.h" #include "mx/core/generated/Print.h" @@ -68,7 +67,7 @@ TEST(newSystem, doesItWork) auto rId = fromScore(s); REQUIRE(rId.ok()); const auto doc = std::move(rId).value(); - const auto &sp = coreDocumentOf(doc).asScorePartwise(); + const auto &sp = doc.getCoreDocument().asScorePartwise(); const auto &p = sp.part()[0]; size_t index = 0; diff --git a/src/private/mxtest/api/NoteDataTest.cpp b/src/private/mxtest/api/NoteDataTest.cpp index fe640492b..7264add70 100644 --- a/src/private/mxtest/api/NoteDataTest.cpp +++ b/src/private/mxtest/api/NoteDataTest.cpp @@ -8,7 +8,6 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Direction.h" #include "mx/core/generated/Document.h" #include "mx/core/generated/MusicDataChoice.h" @@ -1112,7 +1111,7 @@ TEST(directionOrder, NoteData) std::stringstream ss; doc.writeToStream(ss); - const auto &docPtr = coreDocumentOf(doc); + const auto &docPtr = doc.getCoreDocument(); REQUIRE(docPtr.isScorePartwise()); const auto &partwise = docPtr.asScorePartwise(); const auto partwiseParts = partwise.part(); diff --git a/src/private/mxtest/api/TranspositionTest.cpp b/src/private/mxtest/api/TranspositionTest.cpp index 416301027..a9a0535bb 100644 --- a/src/private/mxtest/api/TranspositionTest.cpp +++ b/src/private/mxtest/api/TranspositionTest.cpp @@ -6,7 +6,6 @@ #ifdef MX_COMPILE_API_TESTS #include "cpul/cpulTestHarness.h" #include "mx/api/MusicXml.h" -#include "mx/api/MusicXmlInternal.h" #include "mx/core/generated/Attributes.h" #include "mx/core/generated/AttributesChoice.h" #include "mx/core/generated/Document.h" @@ -63,7 +62,7 @@ inline void checkCoreTransposeElement(const mx::api::ScoreData &inScore, int inE auto r = mx::api::fromScore(inScore); REQUIRE(r.ok()); const auto doc = std::move(r).value(); - const auto &core = coreDocumentOf(doc); + const auto &core = doc.getCoreDocument(); REQUIRE(core.isScorePartwise()); const auto &score = core.asScorePartwise(); const auto parts = score.part(); From af24e1a5748171f51299ceadc33eaf5cf2706300 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sat, 12 Sep 2026 18:00:22 +0200 Subject: [PATCH 4/7] docs: note getCoreDocument's reference lifetime (#435) The reference getCoreDocument returns is only good while this MusicXml is alive and has not been moved away. Moving into another MusicXml, or into intoScore (which frees the tree when it returns), leaves any reference taken beforehand dangling with no compiler diagnostic. Document this on the method. --- src/include/mx/api/MusicXml.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/include/mx/api/MusicXml.h b/src/include/mx/api/MusicXml.h index 83f97fe05..45af2d11b 100644 --- a/src/include/mx/api/MusicXml.h +++ b/src/include/mx/api/MusicXml.h @@ -52,6 +52,11 @@ class MusicXml // you want to edit the core DOM directly. You will need to include the // private mx::core headers in your header search paths to do so. Not // recommended, try opening an issue first! + // + // The reference is only good for as long as this MusicXml is alive and + // you have not moved it away: do not keep it past a std::move of this + // object into another MusicXml or into intoScore, which destroys the + // document when it returns. core::Document &getCoreDocument(); const core::Document &getCoreDocument() const; From 34e9c32ce982e5e01b014cbc0c26a2b1a5da399a Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Wed, 16 Sep 2026 15:23:12 +0200 Subject: [PATCH 5/7] Update src/include/mx/api/Result.h Signed-off-by: Matthew James Briggs --- src/include/mx/api/Result.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/include/mx/api/Result.h b/src/include/mx/api/Result.h index 89ef51abc..be94b82ac 100644 --- a/src/include/mx/api/Result.h +++ b/src/include/mx/api/Result.h @@ -31,8 +31,6 @@ enum class ResultCode tooManyElements, invalidDocument, unsupportedVersion, // mirrored from the core parse boundary - // TODO: this badly swallows exception information. we need more variants - // or something like a site and message. internalError, // caught exception; nothing escapes (api-level) }; From 183ceefdc5855df245acb07be0ee7486f29da984 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Wed, 16 Sep 2026 15:23:19 +0200 Subject: [PATCH 6/7] Update src/private/mx/examples/Write.cpp Signed-off-by: Matthew James Briggs --- src/private/mx/examples/Write.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/private/mx/examples/Write.cpp b/src/private/mx/examples/Write.cpp index 5877516f9..4758f2051 100644 --- a/src/private/mx/examples/Write.cpp +++ b/src/private/mx/examples/Write.cpp @@ -130,4 +130,4 @@ int main(int argc, const char *argv[]) const auto writeResult = document.writeToFile(outputPath); return writeResult.ok() ? 0 : 1; -} \ No newline at end of file +} From 772c6d255db0323584e2c73fafdbad54ebfac77f Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Wed, 16 Sep 2026 16:47:58 +0200 Subject: [PATCH 7/7] fix: clang-format the trailing comment in ResultCode (#435) --- src/include/mx/api/Result.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/mx/api/Result.h b/src/include/mx/api/Result.h index be94b82ac..12c7a3dd5 100644 --- a/src/include/mx/api/Result.h +++ b/src/include/mx/api/Result.h @@ -31,7 +31,7 @@ enum class ResultCode tooManyElements, invalidDocument, unsupportedVersion, // mirrored from the core parse boundary - internalError, // caught exception; nothing escapes (api-level) + internalError, // caught exception; nothing escapes (api-level) }; struct ApiError