From ce6306c1de55be4e24046d4b6a256b7f138b8131 Mon Sep 17 00:00:00 2001 From: Ralph Wessel Date: Mon, 16 Sep 2024 22:54:55 +0100 Subject: [PATCH] CardMover should validate handler before constructing base Mover ReceiverModelCar constructor should also construct base ModelCard SenderModelCar didn't populate member filter DocumentStoreCore now loads objects when project reopened DocumentStoreCore uses defer for safer handle release DocumentStoreCore::resetStore must reset object ID DocumentStoreEngine must reset cache and DocumentStoreCore --- .../Connector/Record/Model/CardMover.cpp | 32 ++++++++++--------- .../Connector/Record/Model/CardMover.h | 5 --- .../Record/Model/ReceiverModelCard.h | 7 ++-- .../Record/Model/SenderModelCard.cpp | 11 ++++--- .../Connector/Record/Model/SenderModelCard.h | 3 +- .../DocumentStore/DocumentStoreCore.cpp | 24 ++++++++++++-- .../Storage/DocumentStore/DocumentStoreCore.h | 2 +- .../DocumentStore/DocumentStoreEngine.h | 5 ++- 8 files changed, 56 insertions(+), 33 deletions(-) diff --git a/SpeckleConnector/Connector/Record/Model/CardMover.cpp b/SpeckleConnector/Connector/Record/Model/CardMover.cpp index f2c1a60..48e9c94 100644 --- a/SpeckleConnector/Connector/Record/Model/CardMover.cpp +++ b/SpeckleConnector/Connector/Record/Model/CardMover.cpp @@ -21,6 +21,21 @@ namespace { ///Identity for a SenderModelCard const char* senderCardTypeName = "SenderModelCard"; + /*-------------------------------------------------------------------- + Ensure the handler is populated + + handler: The card handler to validate + + return: A reference to the handler + --------------------------------------------------------------------*/ + std::shared_ptr& validateHandler(std::shared_ptr& handler) { + if (!handler->empty()) + return handler; + handler->add(receiverCardTypeName); + handler->add(senderCardTypeName); + return handler; + } //CardMover::validateHandler + } ///The handler for model card packages @@ -32,8 +47,7 @@ std::shared_ptr CardMover::m_handler = std::make_sha handler: A package handler to reconstruct incoming packages --------------------------------------------------------------------*/ -CardMover::CardMover() : Mover{m_handler} { - validateHandler(); +CardMover::CardMover() : Mover{validateHandler(m_handler)} { } //CardMover::CardMover @@ -42,17 +56,5 @@ CardMover::CardMover() : Mover{m_handler} { outgoing: An outgoing package --------------------------------------------------------------------*/ -CardMover::CardMover(const active::serialise::Package& outgoing) : Mover{outgoing, m_handler} { - validateHandler(); +CardMover::CardMover(const active::serialise::Package& outgoing) : Mover{outgoing, validateHandler(m_handler)} { } //CardMover::CardMover - - -/*-------------------------------------------------------------------- - Ensure the handler is populated - --------------------------------------------------------------------*/ -void CardMover::validateHandler() { - if (!m_handler->empty()) - return; - m_handler->add(receiverCardTypeName); - m_handler->add(senderCardTypeName); -} //CardMover::validateHandler diff --git a/SpeckleConnector/Connector/Record/Model/CardMover.h b/SpeckleConnector/Connector/Record/Model/CardMover.h index 7e3fa0f..62bfe7c 100644 --- a/SpeckleConnector/Connector/Record/Model/CardMover.h +++ b/SpeckleConnector/Connector/Record/Model/CardMover.h @@ -27,11 +27,6 @@ namespace connector::record { CardMover(const active::serialise::Package& outgoing); private: - /*! - Ensure the handler is populated - */ - static void validateHandler(); - ///The handler for model card packages static std::shared_ptr m_handler; }; diff --git a/SpeckleConnector/Connector/Record/Model/ReceiverModelCard.h b/SpeckleConnector/Connector/Record/Model/ReceiverModelCard.h index 6357679..95c4217 100644 --- a/SpeckleConnector/Connector/Record/Model/ReceiverModelCard.h +++ b/SpeckleConnector/Connector/Record/Model/ReceiverModelCard.h @@ -33,9 +33,12 @@ namespace connector::record { @param hasDimissedWarning True if the user has already dismissed an alert to update @param bakedObjects The IDs of objects accepted in the receive */ - ReceiverModelCard(const speckle::utility::String& projectName, const speckle::utility::String& modelName, + ReceiverModelCard(const speckle::database::RecordID& projectID, const speckle::utility::String& projectName, + const speckle::database::RecordID& modelID, const speckle::utility::String& modelName, const speckle::database::RecordID& selectedVersion, const speckle::database::RecordID& latestVersion, - bool hasDimissedWarning, database::ElementIDList&& bakedObjects) : + const speckle::database::RecordID& accountID, const speckle::utility::String& serverURL, + bool hasDimissedWarning, database::ElementIDList&& bakedObjects, const SettingList& settings) : + ModelCard{modelID, projectID, accountID, serverURL, settings}, m_projectName{projectName}, m_modelName{modelName}, m_selectedVersionID{selectedVersion}, m_latestVersionID{latestVersion}, m_hasDismissedUpdateWarning{hasDimissedWarning}, m_bakedObjectIDs{bakedObjects} {} /*! diff --git a/SpeckleConnector/Connector/Record/Model/SenderModelCard.cpp b/SpeckleConnector/Connector/Record/Model/SenderModelCard.cpp index af8e31c..e122cc3 100644 --- a/SpeckleConnector/Connector/Record/Model/SenderModelCard.cpp +++ b/SpeckleConnector/Connector/Record/Model/SenderModelCard.cpp @@ -37,8 +37,10 @@ SenderModelCard::SenderModelCard() { filter: The filter applied when the model was sent --------------------------------------------------------------------*/ -SenderModelCard::SenderModelCard(const SendFilter& filter) { - +SenderModelCard::SenderModelCard(const SendFilter& filter, const speckle::database::RecordID& modelID, const speckle::database::RecordID& projectID, + const speckle::database::RecordID& accountID, const speckle::utility::String& serverURL, const SettingList& settings) : + ModelCard{modelID, projectID, accountID, serverURL, settings}, m_filter(clone(filter)) +{ } //SenderModelCard::SenderModelCard @@ -47,8 +49,8 @@ SenderModelCard::SenderModelCard(const SendFilter& filter) { source: The object to copy --------------------------------------------------------------------*/ -SenderModelCard::SenderModelCard(const SenderModelCard& source) { - +SenderModelCard::SenderModelCard(const SenderModelCard& source) : ModelCard{source} { + m_filter = (source.m_filter) ? clone(*source.m_filter) : nullptr; } //SenderModelCard::SenderModelCard @@ -56,7 +58,6 @@ SenderModelCard::SenderModelCard(const SenderModelCard& source) { Destructor --------------------------------------------------------------------*/ SenderModelCard::~SenderModelCard() { - } //SenderModelCard::~SenderModelCard diff --git a/SpeckleConnector/Connector/Record/Model/SenderModelCard.h b/SpeckleConnector/Connector/Record/Model/SenderModelCard.h index 16d3493..0b3e010 100644 --- a/SpeckleConnector/Connector/Record/Model/SenderModelCard.h +++ b/SpeckleConnector/Connector/Record/Model/SenderModelCard.h @@ -27,7 +27,8 @@ namespace connector::record { Constructor @param filter The filter applied when the model was sent */ - SenderModelCard(const SendFilter& filter); + SenderModelCard(const SendFilter& filter, const speckle::database::RecordID& modelID, const speckle::database::RecordID& projectID, + const speckle::database::RecordID& accountID, const speckle::utility::String& serverURL, const SettingList& settings); /*! Copy constructor @param source The object to copy diff --git a/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreCore.cpp b/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreCore.cpp index 7fedccf..84e653e 100644 --- a/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreCore.cpp +++ b/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreCore.cpp @@ -1,5 +1,6 @@ #include "Speckle/Database/Storage/DocumentStore/DocumentStoreCore.h" +#include "Active/Utility/Defer.h" #include "Active/Utility/Memory.h" #include "Active/Utility/String.h" #include "Speckle/Environment/Addon.h" @@ -58,10 +59,27 @@ namespace { bool isExistingStore(active::utility::NameID& storeID) { if (storeID.id) return true; //We must have a store if the ID is populated + GS::UniString storeName{String{storeID.name}}; API_Guid acID; - if (auto statusCode = convertArchicadError(ACAPI_AddOnObject_GetObjectGuidFromName(String{storeID.name}, &acID)); statusCode != nominal) + if (auto statusCode = convertArchicadError(ACAPI_AddOnObject_GetObjectGuidFromName(storeName, &acID)); statusCode != nominal) throw std::system_error(DocumentStoreCore::makeError(statusCode)); storeID.id = Guid{acID}; + if (!storeID.id) { + GS::Array storedObjects; + ACAPI_AddOnObject_GetObjectList(&storedObjects); + for (auto iter = storedObjects.Enumerate(); iter != nullptr; ++iter) { + API_Guid objGuid = *iter; + GS::UniString objName; + GSHandle content = nullptr; + auto scope = active::utility::defer([&content](){ BMKillHandle(&content); }); + if (ACAPI_AddOnObject_GetObjectContent(objGuid, &objName, &content) == NoError) { + if (objName == storeName) { + storeID.id = Guid{objGuid}; + break; + } + } + } + } return storeID.id.operator bool(); //Returns true if the store ID is non-null, i.e. the object exists } //isExistingStore @@ -186,11 +204,11 @@ active::utility::Memory DocumentStoreCore::readStore() const { #ifdef ARCHICAD GS::UniString storeName{String{m_id.name}}; GSHandle storedData; + auto scoped = active::utility::defer([&storedData](){ BMKillHandle(&storedData); }); if (auto statusCode = convertArchicadError(ACAPI_AddOnObject_GetObjectContent(Guid{m_id.id}, &storeName, &storedData)); statusCode != nominal) throw std::system_error(makeError(statusCode)); //Copy the stored data into the result copyHandleToMemory(storedData, result); - BMKillHandle(&storedData); #endif return result; } //DocumentStoreCore::readStore @@ -221,10 +239,10 @@ void DocumentStoreCore::writeStore() { auto toWrite = buildStore(); //Write the new data GSHandle output = BMAllocateHandle(static_cast(toWrite.size()), ALLOCATE_CLEAR, 0); + auto scoped = active::utility::defer([&output](){ BMKillHandle(&output); }); active::utility::Memory::copy(*output, toWrite.data(), toWrite.size(), toWrite.size()); if (auto statusCode = convertArchicadError(ACAPI_AddOnObject_ModifyObject(Guid{m_id.id}, nullptr, &output)); statusCode != nominal) throw std::system_error(makeError(statusCode)); - BMKillHandle(&output); //Release the storage object in TW if (addon()->isSharedDocument()) { if (auto statusCode = convertArchicadError(ACAPI_AddOnObject_ReleaseObjects({Guid{m_id.id}})); statusCode != nominal) diff --git a/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreCore.h b/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreCore.h index 77b98e0..c4a0585 100644 --- a/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreCore.h +++ b/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreCore.h @@ -104,7 +104,7 @@ namespace speckle::database { /*! Reset the stored data (some external change has invalidated previous data, e.g. the document was closed) */ - virtual void resetStore() = 0; + virtual void resetStore() { m_id.id.clear(); } // MARK: Subscriber functions diff --git a/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreEngine.h b/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreEngine.h index 015975c..d911977 100644 --- a/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreEngine.h +++ b/SpeckleLib/Speckle/Database/Storage/DocumentStore/DocumentStoreEngine.h @@ -135,7 +135,10 @@ namespace speckle::database { /*! Reset the stored data (some external change has invalidated previous data, e.g. the document was closed) */ - void resetStore() override { m_cache.reset(); } + void resetStore() override { + DocumentStoreCore::resetStore(); + m_cache.reset(); + } private: //Cached records from the document store