From eba85c4b243fb561b095862e95066a2159f3a9dd Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 15:09:40 +0200 Subject: [PATCH 01/34] Extract property name parsing helpers from GenericPropertyObjectImpl Move isChildProperty, splitOnFirstDot, splitOnLastDot, parseIndex, getPropertyNameInfo, getPropNameWithoutIndex and the PropertyNameInfo struct into daq::details in the new property_object_helpers.h header. These are stateless functions with no dependency on the template parameters, so they no longer need to live inside the class. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_helpers.h | 137 +++++++++++++++ .../coreobjects/property_object_impl.h | 157 +++--------------- core/coreobjects/src/CMakeLists.txt | 2 + 3 files changed, 160 insertions(+), 136 deletions(-) create mode 100644 core/coreobjects/include/coreobjects/property_object_helpers.h diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h new file mode 100644 index 0000000000..ad9e1f4618 --- /dev/null +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -0,0 +1,137 @@ +/* + * Copyright 2022-2026 openDAQ d.o.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once +#include +#include +#include +#include + +BEGIN_NAMESPACE_OPENDAQ + +// Stateless helpers used by GenericPropertyObjectImpl. Internal to openDAQ; not part of the public API surface. +namespace details +{ + +struct PropertyNameInfo +{ + StringPtr name; + Int index{}; +}; + +#if defined(__GNUC__) && __GNUC__ >= 12 + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Wdangling-pointer" +#endif + +// Child property handling - Used when a property is queried in the "parent.child" format + +inline bool isChildProperty(const StringPtr& name) +{ + auto chr = strchr(name.getCharPtr(), '.'); + return chr != nullptr; +} + +inline void splitOnFirstDot(const StringPtr& input, StringPtr& head, StringPtr& tail) +{ + const std::string inputStr = input; + head = input; + + size_t pos = inputStr.find('.'); + if (pos == std::string::npos) + return; + + head = inputStr.substr(0, pos); + tail = inputStr.substr(pos + 1); +} + +inline void splitOnLastDot(const StringPtr& input, StringPtr& head, StringPtr& tail) +{ + const std::string inputStr = input; + head = input; + + size_t pos = inputStr.rfind('.'); + if (pos == std::string::npos) + return; + + head = inputStr.substr(0, pos); + tail = inputStr.substr(pos + 1); +} + +// Gets the index integer value between two square brackets +inline int parseIndex(char const* lBracket) +{ + auto last = strchr(lBracket, ']'); + if (last != nullptr) + { + char* end; + int index = strtol(lBracket + 1, &end, 10); + + if (end != last) + { + DAQ_THROW_EXCEPTION(InvalidParameterException, "Could not parse the property index."); + } + + return index; + } + DAQ_THROW_EXCEPTION(InvalidParameterException, "No matching ] found."); +} + +inline PropertyNameInfo getPropertyNameInfo(const StringPtr& name) +{ + PropertyNameInfo nameInfo; + + auto propNameData = name.getCharPtr(); + auto first = strchr(propNameData, '['); + if (first != nullptr) + { + nameInfo.index = parseIndex(first); + nameInfo.name = String(propNameData, first - propNameData); + } + else + { + nameInfo.index = -1; + nameInfo.name = name; + } + + return nameInfo; +} + +// Gets the property name without the index as the `propName` output parameter +// Returns the index in the form of [index], eg. [0] +inline ConstCharPtr getPropNameWithoutIndex(const StringPtr& name, StringPtr& propName) +{ + auto propNameData = name.getCharPtr(); + auto first = strchr(propNameData, '['); + + if (first == nullptr) + { + propName = String(propNameData); + } + else + { + propName = String(propNameData, first - propNameData); + } + return first; +} + +#if defined(__GNUC__) && __GNUC__ >= 12 + #pragma GCC diagnostic pop +#endif + +} + +END_NAMESPACE_OPENDAQ diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 8b302b94c3..d3163a356f 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -54,17 +54,12 @@ #include #include #include +#include BEGIN_NAMESPACE_OPENDAQ using PropertyOrderedMap = tsl::ordered_map; -struct PropertyNameInfo -{ - StringPtr name; - Int index{}; -}; - namespace config_protocol { class ConfigClientDeviceInfoImpl; @@ -387,11 +382,6 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::getClassName #pragma GCC diagnostic ignored "-Wdangling-pointer" #endif -template -bool GenericPropertyObjectImpl::isChildProperty(const StringPtr& name) -{ - auto chr = strchr(name.getCharPtr(), '.'); - return chr != nullptr; -} - -template -void GenericPropertyObjectImpl::splitOnFirstDot(const StringPtr& input, - StringPtr& head, - StringPtr& tail) -{ - const std::string inputStr = input; - head = input; - - size_t pos = inputStr.find('.'); - if (pos == std::string::npos) - return; - - head = inputStr.substr(0, pos); - tail = inputStr.substr(pos + 1); -} - -template -void GenericPropertyObjectImpl::splitOnLastDot(const StringPtr& input, - StringPtr& head, - StringPtr& tail) -{ - const std::string inputStr = input; - head = input; - - size_t pos = inputStr.rfind('.'); - if (pos == std::string::npos) - return; - - head = inputStr.substr(0, pos); - tail = inputStr.substr(pos + 1); -} - template ErrCode GenericPropertyObjectImpl::getChildPropertyValue(const StringPtr& childName, const StringPtr& subName, @@ -1103,7 +1046,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyV const ErrCode errCode = daqTry([&]() { - const auto isChildProp = isChildProperty(propName); + const auto isChildProp = details::isChildProperty(propName); if (batch && !isChildProp) { @@ -1114,7 +1057,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyV StringPtr subName; if (isChildProp) { - splitOnFirstDot(propName, propName, subName); + details::splitOnFirstDot(propName, propName, subName); } PropertyPtr prop = getUnboundProperty(propName); @@ -1461,7 +1404,7 @@ bool GenericPropertyObjectImpl::hasDuplicateRef template ErrCode GenericPropertyObjectImpl::readLocalValue(const StringPtr& name, BaseObjectPtr& value) const { - PropertyNameInfo info = getPropertyNameInfo(name); + details::PropertyNameInfo info = details::getPropertyNameInfo(name); const auto it = propValues.find(info.name); if (it != propValues.cend()) @@ -1490,69 +1433,11 @@ ErrCode GenericPropertyObjectImpl::readLocalVal return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property value "{}" not found)", name)); } -template -int GenericPropertyObjectImpl::parseIndex(char const* lBracket) -{ - auto last = strchr(lBracket, ']'); - if (last != nullptr) - { - char* end; - int index = strtol(lBracket + 1, &end, 10); - - if (end != last) - { - DAQ_THROW_EXCEPTION(InvalidParameterException, "Could not parse the property index."); - } - - return index; - } - DAQ_THROW_EXCEPTION(InvalidParameterException, "No matching ] found."); -} - #if defined(__GNUC__) && __GNUC__ >= 12 #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdangling-pointer" #endif -template -PropertyNameInfo GenericPropertyObjectImpl::getPropertyNameInfo(const StringPtr& name) -{ - PropertyNameInfo nameInfo; - - auto propNameData = name.getCharPtr(); - auto first = strchr(propNameData, '['); - if (first != nullptr) - { - nameInfo.index = parseIndex(first); - nameInfo.name = String(propNameData, first - propNameData); - } - else - { - nameInfo.index = -1; - nameInfo.name = name; - } - - return nameInfo; -} - -template -ConstCharPtr GenericPropertyObjectImpl::getPropNameWithoutIndex(const StringPtr& name, - StringPtr& propName) -{ - auto propNameData = name.getCharPtr(); - auto first = strchr(propNameData, '['); - - if (first == nullptr) - { - propName = String(propNameData); - } - else - { - propName = String(propNameData, first - propNameData); - } - return first; -} - template void GenericPropertyObjectImpl::triggerCoreEventInternal(const CoreEventArgsPtr& args) { @@ -1568,7 +1453,7 @@ ErrCode GenericPropertyObjectImpl::getPropertyA bool retrieveUpdatingValue) { StringPtr propName; - ConstCharPtr bracket = getPropNameWithoutIndex(name, propName); + ConstCharPtr bracket = details::getPropNameWithoutIndex(name, propName); property = getUnboundPropertyOrNull(propName); @@ -1625,7 +1510,7 @@ ErrCode GenericPropertyObjectImpl::getPropertyA CoreType coreType = value.getCoreType(); if (coreType == ctList && bracket != nullptr) { - int index = parseIndex(bracket); + int index = details::parseIndex(bracket); ListPtr list = value; if (index >= static_cast(list.getCount())) { @@ -1707,11 +1592,11 @@ ErrCode GenericPropertyObjectImpl::setPropertyS const auto propName = StringPtr::Borrow(propertyName); const auto valuePtr = BaseObjectPtr::Borrow(value); - if (isChildProperty(propName)) + if (details::isChildProperty(propName)) { StringPtr childName; StringPtr subName; - splitOnFirstDot(propName, childName, subName); + details::splitOnFirstDot(propName, childName, subName); BaseObjectPtr childProp; const ErrCode err = getPropertyValueInternal(childName, &childProp); @@ -2054,10 +1939,10 @@ ErrCode GenericPropertyObjectImpl::clearPropert } StringPtr subName; - const auto isChildProp = isChildProperty(propName); + const auto isChildProp = details::isChildProperty(propName); if (isChildProp) { - splitOnFirstDot(propName, propName, subName); + details::splitOnFirstDot(propName, propName, subName); } PropertyPtr prop = getUnboundPropertyOrNull(propName); @@ -2168,10 +2053,10 @@ ErrCode GenericPropertyObjectImpl::getPropertyV BaseObjectPtr valuePtr; ErrCode err; - if (isChildProperty(propName)) + if (details::isChildProperty(propName)) { StringPtr subName; - splitOnFirstDot(propName, propName, subName); + details::splitOnFirstDot(propName, propName, subName); err = getChildPropertyValue(propName, subName, valuePtr); } else @@ -2203,7 +2088,7 @@ ErrCode GenericPropertyObjectImpl::getPropertyS BaseObjectPtr valuePtr; PropertyPtr prop; - if (isChildProperty(propName)) + if (details::isChildProperty(propName)) { const ErrCode errCode = getProperty(propName, &prop); OPENDAQ_RETURN_IF_FAILED(errCode, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); @@ -2272,12 +2157,12 @@ ErrCode GenericPropertyObjectImpl::getProperty( StringPtr propName = propertyName; PropertyPtr prop; - if (isChildProperty(propName)) + if (details::isChildProperty(propName)) { StringPtr subName; BaseObjectPtr childProp; - splitOnFirstDot(propName, propName, subName); + details::splitOnFirstDot(propName, propName, subName); const ErrCode err = getPropertyValueInternal(propName, &childProp); OPENDAQ_RETURN_IF_FAILED(err); @@ -2573,10 +2458,10 @@ ErrCode GenericPropertyObjectImpl::getOnPropert StringPtr name = StringPtr::Borrow( propertyName); - if (isChildProperty(name)) + if (details::isChildProperty(name)) { StringPtr subName; - splitOnFirstDot(name, name, subName); + details::splitOnFirstDot(name, name, subName); BaseObjectPtr childProp; ErrCode errCode = getPropertyValueInternal(name, &childProp); @@ -2612,10 +2497,10 @@ ErrCode GenericPropertyObjectImpl::getOnPropert StringPtr name = StringPtr::Borrow(propertyName); - if (isChildProperty(name)) + if (details::isChildProperty(name)) { StringPtr subName; - splitOnFirstDot(name, name, subName); + details::splitOnFirstDot(name, name, subName); BaseObjectPtr childProp; ErrCode errCode = getPropertyValueInternal(name, &childProp); @@ -3577,11 +3462,11 @@ ErrCode GenericPropertyObjectImpl::hasProperty( auto propName = StringPtr::Borrow(propertyName); - if (isChildProperty(propName)) + if (details::isChildProperty(propName)) { BaseObjectPtr val; StringPtr childStr; - splitOnLastDot(propName, propName, childStr); + details::splitOnLastDot(propName, propName, childStr); ErrCode err = getPropertyValue(propName, &val); OPENDAQ_RETURN_IF_FAILED_EXCEPT(err, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Failed to retrieve child object with name {})", propName)); diff --git a/core/coreobjects/src/CMakeLists.txt b/core/coreobjects/src/CMakeLists.txt index 3f5cd9f240..eac1b915ed 100644 --- a/core/coreobjects/src/CMakeLists.txt +++ b/core/coreobjects/src/CMakeLists.txt @@ -112,6 +112,7 @@ source_group("property_object" FILES ${SDK_HEADERS_DIR}/property_object.h ${SDK_HEADERS_DIR}/property_object_core.h ${SDK_HEADERS_DIR}/property_object_core_impl.h ${SDK_HEADERS_DIR}/property_object_utils.h + ${SDK_HEADERS_DIR}/property_object_helpers.h property_object_core_impl.cpp property_object_impl.cpp property_object_utils.cpp @@ -300,6 +301,7 @@ set(SRC_PublicHeaders coreobjects.h mutex_impl.h property_object_core.h property_object_utils.h + property_object_helpers.h permissions_internal.h ) From 62da18b4cdd44580503e74c3fbcc5a6c06fb8f5b Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 15:15:38 +0200 Subject: [PATCH 02/34] Extract remaining pure statics from GenericPropertyObjectImpl Move the type-check group (checkContainerType, checkStructType, checkEnumerationType, checkSelectionValues, checkPropertyTypeAndConvert), the coercion/validation group (coercePropertyWrite, validatePropertyWrite, coerceMinMax) and the reference-resolution group (checkForRefPropAndGetBoundProp, hasDuplicateReferences, checkIsReferenced) into daq::details in property_object_helpers.h. All are stateless functions of their arguments with no dependency on the class template parameters. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_helpers.h | 345 +++++++++++++++ .../coreobjects/property_object_impl.h | 399 +----------------- 2 files changed, 361 insertions(+), 383 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h index ad9e1f4618..0c9d3db333 100644 --- a/core/coreobjects/include/coreobjects/property_object_helpers.h +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -16,9 +16,22 @@ #pragma once #include +#include +#include +#include +#include +#include +#include +#include #include +#include +#include +#include +#include +#include #include #include +#include BEGIN_NAMESPACE_OPENDAQ @@ -132,6 +145,338 @@ inline ConstCharPtr getPropNameWithoutIndex(const StringPtr& name, StringPtr& pr #pragma GCC diagnostic pop #endif +// Checks if the value is a container type, or base `IPropertyObject`. Only such values can be set in `setProperty` +inline ErrCode checkContainerType(const PropertyPtr& prop, const BaseObjectPtr& value) +{ + if (!value.assigned()) + return OPENDAQ_SUCCESS; + + auto coreType = value.getCoreType(); + if (coreType == ctObject) + { + auto inspect = value.asPtrOrNull(true); + if (inspect.assigned() && !inspect.getInterfaceIds().empty()) + { + return inspect.getInterfaceIds()[0] == IPropertyObject::Id; + } + + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, "Only base Property Object object-type values are allowed"); + } + + auto iterate = [](const IterablePtr& it, CoreType type) + { + for (const auto& key : it) + { + if (key.getCoreType() != type) + return false; + } + return true; + }; + + const auto propInternal = prop.asPtr(true); + if (coreType == ctDict) + { + const auto dict = value.asPtr(); + const auto keyType = propInternal.getKeyTypeNoLock(); + const auto itemType = propInternal.getItemTypeNoLock(); + + IterablePtr it; + dict->getKeys(&it); + if (!iterate(it, keyType)) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(Invalid dictionary key type for property "{}")", prop.getName())); + + dict->getValues(&it); + if (!iterate(it, itemType)) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(Invalid dictionary item type for property "{}")", prop.getName())); + } + else if (coreType == ctList) + { + const auto itemType = propInternal.getItemTypeNoLock(); + + if (itemType != ctUndefined && !iterate(value, itemType)) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(Invalid list item type for property "{}")", prop.getName())); + } + + return OPENDAQ_SUCCESS; +} + +// Checks if the property is a struct type, and checks its fields for type/name compatibility +inline ErrCode checkStructType(const PropertyPtr& prop, const BaseObjectPtr& value) +{ + if (prop.getValueType() != ctStruct) + return OPENDAQ_SUCCESS; + + auto structPtr = value.asPtrOrNull(); + if (!structPtr.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Set value is not a struct for property "{}")", prop.getName())); + + StructTypePtr structType = prop.asPtr().getStructTypeNoLock(); + StructTypePtr valueStructType = structPtr.getStructType(); + + if (structType != valueStructType) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Set value StructureType is different from the default for property "{}")", prop.getName())); + + return OPENDAQ_SUCCESS; +} + +// Checks if the property is a enumeration type and checks for type/name compatibility +inline ErrCode checkEnumerationType(const PropertyPtr& prop, const BaseObjectPtr& value) +{ + const auto propInternal = prop.asPtr(); + if (propInternal.getValueTypeNoLock() != ctEnumeration) + return OPENDAQ_SUCCESS; + + auto enumerationPtr = value.asPtrOrNull(); + if (!enumerationPtr.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Set value is not an enumeration for property "{}")", prop.getName())); + + auto propEnumerationPtr = propInternal.getDefaultValueNoLock().asPtrOrNull(); + if (!propEnumerationPtr.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Property default value is not an enumeration for property "{}")", prop.getName())); + + EnumerationTypePtr valueEnumerationType = enumerationPtr.getEnumerationType(); + EnumerationTypePtr propEnumerationType = propEnumerationPtr.getEnumerationType(); + + if (propEnumerationType != valueEnumerationType) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Set value EnumerationType is different from the default for property "{}")", prop.getName())); + + return OPENDAQ_SUCCESS; +} + +// Checks if value is a correct key into the list/dictionary of selection values +inline ErrCode checkSelectionValues(const PropertyPtr& prop, const BaseObjectPtr& value) +{ + const auto selectionValues = prop.asPtr(true).getSelectionValuesNoLock(); + if (selectionValues.assigned()) + { + const PropertyType propType = prop.getPropertyType(); + if (propType == PropertyType::IndexSelection) + { + if (const auto list = selectionValues.asPtrOrNull(true); list.assigned()) + { + const SizeT key = value; + if (key < list.getCount()) + return OPENDAQ_SUCCESS; + } + } + else if (propType == PropertyType::Selection) + { + if (const auto list = selectionValues.asPtrOrNull(true); list.assigned()) + { + if (prop.getValueType() == ctFloat) + { + const double valueDouble = value; + const double preScale = std::max({1.0, std::abs(valueDouble)}); + for (const double& item : list) + { + const double scale = std::max({preScale, std::abs(item)}); + if (std::abs(item - valueDouble) <= std::numeric_limits::epsilon() * scale) + return OPENDAQ_SUCCESS; + } + } + else + { + for (const auto& item : list) + { + if (item == value) + return OPENDAQ_SUCCESS; + } + } + } + } + else if (propType == PropertyType::SparseSelection) + { + if (const auto dict = selectionValues.asPtrOrNull(true); dict.assigned() && dict.hasKey(value)) + return OPENDAQ_SUCCESS; + } + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Value is not a key/index of selection values for property "{}")", prop.getName())); + } + + return OPENDAQ_SUCCESS; +} + +// Checks if property and value type match. If not, attempts to convert the value +inline ErrCode checkPropertyTypeAndConvert(const PropertyPtr& prop, BaseObjectPtr& value) +{ + if (!prop.assigned() || !value.assigned()) + return OPENDAQ_SUCCESS; + + if (value.supportsInterface()) + return OPENDAQ_SUCCESS; + + const ErrCode errCode = daqTry([&]() + { + const auto propInternal = prop.asPtr(); + const auto propCoreType = propInternal.getValueTypeNoLock(); + const auto valueCoreType = value.getCoreType(); + + if (propCoreType != valueCoreType) + { + if (propCoreType == ctEnumeration) + { + const auto enumVal = propInternal.getDefaultValueNoLock().asPtrOrNull(); + if (!enumVal.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, + fmt::format(R"(Default value of enumeration property {} is not assigned)", prop.getName())); + + const auto type = enumVal.getEnumerationType(); + const Int intVal = value.convertTo(ctInt); + value = EnumerationWithIntValueAndType(type, intVal); + } + else + value = value.convertTo(propCoreType); + } + return OPENDAQ_SUCCESS; + }); + + OPENDAQ_RETURN_IF_FAILED(errCode, fmt::format(R"(Value type is different than Property type and conversion failed for property "{}")", prop.getName())); + return errCode; +} + +// Coercion/Validation + +inline void coercePropertyWrite(const PropertyPtr& prop, ObjectPtr& valuePtr, const PropertyObjectPtr& objPtr) +{ + if (prop.assigned() && valuePtr.assigned()) + { + const auto coercer = prop.asPtr().getCoercerNoLock(); + if (coercer.assigned()) + { + try + { + valuePtr = coercer.coerceNoLock(objPtr, valuePtr); + } + catch (const DaqException&) + { + throw; + } + catch (...) + { + DAQ_THROW_EXCEPTION(CoerceFailedException); + } + } + } +} + +inline void validatePropertyWrite(const PropertyPtr& prop, ObjectPtr& valuePtr, const PropertyObjectPtr& objPtr) +{ + if (prop.assigned() && valuePtr.assigned()) + { + const auto validator = prop.asPtr().getValidatorNoLock(); + if (validator.assigned()) + { + try + { + validator.validateNoLock(objPtr, valuePtr); + } + catch (const DaqException&) + { + throw; + } + catch (...) + { + DAQ_THROW_EXCEPTION(ValidateFailedException); + } + } + } +} + +inline void coerceMinMax(const PropertyPtr& prop, ObjectPtr& valuePtr) +{ + if (!prop.assigned() || !valuePtr.assigned()) + return; + + const auto propInternal = prop.asPtr(); + const auto min = propInternal.getMinValueNoLock(); + if (min.assigned()) + { + try + { + if (valuePtr < min) + valuePtr = min; + } + catch (...) + { + } + } + + const auto max = propInternal.getMaxValueNoLock(); + if (max.assigned()) + { + try + { + if (valuePtr > max) + valuePtr = max; + } + catch (...) + { + } + } +} + +// Reference property handling + +inline PropertyPtr checkForRefPropAndGetBoundProp(PropertyPtr& prop, const PropertyObjectPtr& objPtr, bool* isReferenced = nullptr) +{ + if (!prop.assigned()) + { + return prop; + } + + PropertyInternalPtr boundProp = prop.asPtr(true).cloneWithOwner(objPtr); + auto refProp = boundProp.getReferencedPropertyNoLock(); + if (refProp.assigned()) + { + CoreType ct = refProp.getCoreType(); + + if (ct != ctObject) + throw std::invalid_argument("Invalid reference to property"); + + if (isReferenced) + *isReferenced = true; + + return checkForRefPropAndGetBoundProp(refProp, objPtr); + } + + if (isReferenced) + *isReferenced = false; + return boundProp; +} + +// Checks whether the property is a reference property that references an already referenced property +inline bool hasDuplicateReferences(const PropertyPtr& prop, const PropertyObjectPtr& objPtr) +{ + auto refEval = prop.asPtr().getReferencedPropertyUnresolved(); + if (refEval.assigned()) + { + auto refNames = refEval.getPropertyReferences(); + for (auto refPropName : refNames) + { + if (objPtr.hasProperty(refPropName) && objPtr.getProperty(refPropName).getIsReferenced()) + return true; + } + } + + return false; +} + +inline Bool checkIsReferenced(const StringPtr& referencedPropName, const PropertyInternalPtr& prop) +{ + const auto refProp = prop.getReferencedPropertyUnresolved(); + if (!refProp.assigned()) + return false; + + for (auto propName : refProp.getPropertyReferences()) + { + if (propName == referencedPropName) + { + return true; + } + } + + return false; +} + } END_NAMESPACE_OPENDAQ diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index d3163a356f..5f5a0ec685 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -397,18 +397,6 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak& valuePtr, const PropertyObjectPtr& objPtr); - static void validatePropertyWrite(const PropertyPtr& prop, ObjectPtr& valuePtr, const PropertyObjectPtr& objPtr); - static void coerceMinMax(const PropertyPtr& prop, ObjectPtr& valuePtr); - static Bool checkIsReferenced(const StringPtr& referencedPropName, const PropertyInternalPtr& prop); // Update ErrCode updateObjectProperties(const PropertyObjectPtr& propObj, @@ -622,7 +597,7 @@ ErrCode GenericPropertyObjectImpl::getChildProp { prop = getUnboundProperty(childName); - prop = checkForRefPropAndGetBoundProp(prop, objPtr); + prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); name = prop.getName(); return OPENDAQ_SUCCESS; }); @@ -763,92 +738,6 @@ BaseObjectPtr GenericPropertyObjectImpl::callPr return args.getValue(); } -template -void GenericPropertyObjectImpl::coercePropertyWrite(const PropertyPtr& prop, - ObjectPtr& valuePtr, - const PropertyObjectPtr& objPtr) -{ - if (prop.assigned() && valuePtr.assigned()) - { - const auto coercer = prop.asPtr().getCoercerNoLock(); - if (coercer.assigned()) - { - try - { - valuePtr = coercer.coerceNoLock(objPtr, valuePtr); - } - catch (const DaqException&) - { - throw; - } - catch (...) - { - DAQ_THROW_EXCEPTION(CoerceFailedException); - } - } - } -} - -template -void GenericPropertyObjectImpl::validatePropertyWrite(const PropertyPtr& prop, - ObjectPtr& valuePtr, - const PropertyObjectPtr& objPtr) -{ - if (prop.assigned() && valuePtr.assigned()) - { - const auto validator = prop.asPtr().getValidatorNoLock(); - if (validator.assigned()) - { - try - { - validator.validateNoLock(objPtr, valuePtr); - } - catch (const DaqException&) - { - throw; - } - catch (...) - { - DAQ_THROW_EXCEPTION(ValidateFailedException); - } - } - } -} - -template -void GenericPropertyObjectImpl::coerceMinMax(const PropertyPtr& prop, ObjectPtr& valuePtr) -{ - if (!prop.assigned() || !valuePtr.assigned()) - return; - - const auto propInternal = prop.asPtr(); - const auto min = propInternal.getMinValueNoLock(); - if (min.assigned()) - { - try - { - if (valuePtr < min) - valuePtr = min; - } - catch (...) - { - } - } - - const auto max = propInternal.getMaxValueNoLock(); - if (max.assigned()) - { - try - { - if (valuePtr > max) - valuePtr = max; - } - catch (...) - { - } - } -} - template ErrCode GenericPropertyObjectImpl::setProtectedPropertyValue(IString* propertyName, IBaseObject* value) { @@ -875,158 +764,6 @@ ErrCode GenericPropertyObjectImpl::setPropertyV return setPropertyValueInternal(propertyName, value, true, false, updateCount > 0); } -template -ErrCode GenericPropertyObjectImpl::checkContainerType(const PropertyPtr& prop, const BaseObjectPtr& value) -{ - if (!value.assigned()) - return OPENDAQ_SUCCESS; - - auto coreType = value.getCoreType(); - if (coreType == ctObject) - { - auto inspect = value.asPtrOrNull(true); - if (inspect.assigned() && !inspect.getInterfaceIds().empty()) - { - return inspect.getInterfaceIds()[0] == IPropertyObject::Id; - } - - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, "Only base Property Object object-type values are allowed"); - } - - auto iterate = [](const IterablePtr& it, CoreType type) - { - for (const auto& key : it) - { - if (key.getCoreType() != type) - return false; - } - return true; - }; - - const auto propInternal = prop.asPtr(true); - if (coreType == ctDict) - { - const auto dict = value.asPtr(); - const auto keyType = propInternal.getKeyTypeNoLock(); - const auto itemType = propInternal.getItemTypeNoLock(); - - IterablePtr it; - dict->getKeys(&it); - if (!iterate(it, keyType)) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(Invalid dictionary key type for property "{}")", prop.getName())); - - dict->getValues(&it); - if (!iterate(it, itemType)) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(Invalid dictionary item type for property "{}")", prop.getName())); - } - else if (coreType == ctList) - { - const auto itemType = propInternal.getItemTypeNoLock(); - - if (itemType != ctUndefined && !iterate(value, itemType)) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(Invalid list item type for property "{}")", prop.getName())); - } - - return OPENDAQ_SUCCESS; -} - -template -ErrCode GenericPropertyObjectImpl::checkStructType(const PropertyPtr& prop, const BaseObjectPtr& value) -{ - if (prop.getValueType() != ctStruct) - return OPENDAQ_SUCCESS; - - auto structPtr = value.asPtrOrNull(); - if (!structPtr.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Set value is not a struct for property "{}")", prop.getName())); - - StructTypePtr structType = prop.asPtr().getStructTypeNoLock(); - StructTypePtr valueStructType = structPtr.getStructType(); - - if (structType != valueStructType) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Set value StructureType is different from the default for property "{}")", prop.getName())); - - return OPENDAQ_SUCCESS; -} - -template -ErrCode GenericPropertyObjectImpl::checkEnumerationType(const PropertyPtr& prop, - const BaseObjectPtr& value) -{ - const auto propInternal = prop.asPtr(); - if (propInternal.getValueTypeNoLock() != ctEnumeration) - return OPENDAQ_SUCCESS; - - auto enumerationPtr = value.asPtrOrNull(); - if (!enumerationPtr.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Set value is not an enumeration for property "{}")", prop.getName())); - - auto propEnumerationPtr = propInternal.getDefaultValueNoLock().asPtrOrNull(); - if (!propEnumerationPtr.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Property default value is not an enumeration for property "{}")", prop.getName())); - - EnumerationTypePtr valueEnumerationType = enumerationPtr.getEnumerationType(); - EnumerationTypePtr propEnumerationType = propEnumerationPtr.getEnumerationType(); - - if (propEnumerationType != valueEnumerationType) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Set value EnumerationType is different from the default for property "{}")", prop.getName())); - - return OPENDAQ_SUCCESS; -} - -template -ErrCode GenericPropertyObjectImpl::checkSelectionValues(const PropertyPtr& prop, - const BaseObjectPtr& value) -{ - const auto selectionValues = prop.asPtr(true).getSelectionValuesNoLock(); - if (selectionValues.assigned()) - { - const PropertyType propType = prop.getPropertyType(); - if (propType == PropertyType::IndexSelection) - { - if (const auto list = selectionValues.asPtrOrNull(true); list.assigned()) - { - const SizeT key = value; - if (key < list.getCount()) - return OPENDAQ_SUCCESS; - } - } - else if (propType == PropertyType::Selection) - { - if (const auto list = selectionValues.asPtrOrNull(true); list.assigned()) - { - if (prop.getValueType() == ctFloat) - { - const double valueDouble = value; - const double preScale = std::max({1.0, std::abs(valueDouble)}); - for (const double& item : list) - { - const double scale = std::max({preScale, std::abs(item)}); - if (std::abs(item - valueDouble) <= std::numeric_limits::epsilon() * scale) - return OPENDAQ_SUCCESS; - } - } - else - { - for (const auto& item : list) - { - if (item == value) - return OPENDAQ_SUCCESS; - } - } - } - } - else if (propType == PropertyType::SparseSelection) - { - if (const auto dict = selectionValues.asPtrOrNull(true); dict.assigned() && dict.hasKey(value)) - return OPENDAQ_SUCCESS; - } - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Value is not a key/index of selection values for property "{}")", prop.getName())); - } - - return OPENDAQ_SUCCESS; -} - template ErrCode GenericPropertyObjectImpl::setPropertyValueInternal(IString* name, IBaseObject* value, @@ -1061,7 +798,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyV } PropertyPtr prop = getUnboundProperty(propName); - prop = checkForRefPropAndGetBoundProp(prop, objPtr); + prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); if (!prop.assigned()) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" not found.)", propName)); @@ -1098,15 +835,15 @@ ErrCode GenericPropertyObjectImpl::setPropertyV } } - OPENDAQ_RETURN_IF_FAILED(checkPropertyTypeAndConvert(prop, valuePtr)); - OPENDAQ_RETURN_IF_FAILED(checkContainerType(prop, valuePtr)); - OPENDAQ_RETURN_IF_FAILED(checkSelectionValues(prop, valuePtr)); - OPENDAQ_RETURN_IF_FAILED(checkStructType(prop, valuePtr)); - OPENDAQ_RETURN_IF_FAILED(checkEnumerationType(prop, valuePtr)); + OPENDAQ_RETURN_IF_FAILED(details::checkPropertyTypeAndConvert(prop, valuePtr)); + OPENDAQ_RETURN_IF_FAILED(details::checkContainerType(prop, valuePtr)); + OPENDAQ_RETURN_IF_FAILED(details::checkSelectionValues(prop, valuePtr)); + OPENDAQ_RETURN_IF_FAILED(details::checkStructType(prop, valuePtr)); + OPENDAQ_RETURN_IF_FAILED(details::checkEnumerationType(prop, valuePtr)); - coercePropertyWrite(prop, valuePtr, objPtr); - validatePropertyWrite(prop, valuePtr, objPtr); - coerceMinMax(prop, valuePtr); + details::coercePropertyWrite(prop, valuePtr, objPtr); + details::validatePropertyWrite(prop, valuePtr, objPtr); + details::coerceMinMax(prop, valuePtr); const auto ct = propInternal.getValueTypeNoLock(); if (ct == ctList || ct == ctDict) @@ -1153,44 +890,6 @@ ErrCode GenericPropertyObjectImpl::setPropertyV return errCode; } -template -ErrCode GenericPropertyObjectImpl::checkPropertyTypeAndConvert(const PropertyPtr& prop, - BaseObjectPtr& value) -{ - if (!prop.assigned() || !value.assigned()) - return OPENDAQ_SUCCESS; - - if (value.supportsInterface()) - return OPENDAQ_SUCCESS; - - const ErrCode errCode = daqTry([&]() - { - const auto propInternal = prop.asPtr(); - const auto propCoreType = propInternal.getValueTypeNoLock(); - const auto valueCoreType = value.getCoreType(); - - if (propCoreType != valueCoreType) - { - if (propCoreType == ctEnumeration) - { - const auto enumVal = propInternal.getDefaultValueNoLock().asPtrOrNull(); - if (!enumVal.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, - fmt::format(R"(Default value of enumeration property {} is not assigned)", prop.getName())); - - const auto type = enumVal.getEnumerationType(); - const Int intVal = value.convertTo(ctInt); - value = EnumerationWithIntValueAndType(type, intVal); - } - else - value = value.convertTo(propCoreType); - } - return OPENDAQ_SUCCESS; - }); - - OPENDAQ_RETURN_IF_FAILED(errCode, fmt::format(R"(Value type is different than Property type and conversion failed for property "{}")", prop.getName())); - return errCode; -} template bool GenericPropertyObjectImpl::shouldWriteLocalValue(const StringPtr& name, const BaseObjectPtr& value) const @@ -1302,36 +1001,6 @@ PropertyPtr GenericPropertyObjectImpl::getUnbou return property; } -template -PropertyPtr GenericPropertyObjectImpl::checkForRefPropAndGetBoundProp(PropertyPtr& prop, - const PropertyObjectPtr& objPtr, - bool* isReferenced) -{ - if (!prop.assigned()) - { - return prop; - } - - PropertyInternalPtr boundProp = prop.asPtr(true).cloneWithOwner(objPtr); - auto refProp = boundProp.getReferencedPropertyNoLock(); - if (refProp.assigned()) - { - CoreType ct = refProp.getCoreType(); - - if (ct != ctObject) - throw std::invalid_argument("Invalid reference to property"); - - if (isReferenced) - *isReferenced = true; - - return checkForRefPropAndGetBoundProp(refProp, objPtr); - } - - if (isReferenced) - *isReferenced = false; - return boundProp; -} - template PropertyObjectPtr GenericPropertyObjectImpl::cloneChildPropertyObject(const PropertyPtr& prop) { @@ -1384,23 +1053,6 @@ void GenericPropertyObjectImpl::configureCloned } } -template -bool GenericPropertyObjectImpl::hasDuplicateReferences(const PropertyPtr& prop, const PropertyObjectPtr& objPtr) -{ - auto refEval = prop.asPtr().getReferencedPropertyUnresolved(); - if (refEval.assigned()) - { - auto refNames = refEval.getPropertyReferences(); - for (auto refPropName : refNames) - { - if (objPtr.hasProperty(refPropName) && objPtr.getProperty(refPropName).getIsReferenced()) - return true; - } - } - - return false; -} - template ErrCode GenericPropertyObjectImpl::readLocalValue(const StringPtr& name, BaseObjectPtr& value) const { @@ -1463,7 +1115,7 @@ ErrCode GenericPropertyObjectImpl::getPropertyA } bool isRef; - property = checkForRefPropAndGetBoundProp(property, objPtr, &isRef); + property = details::checkForRefPropAndGetBoundProp(property, objPtr, &isRef); // TODO: Extract this to own function if (bracket != nullptr) @@ -1615,7 +1267,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyS } PropertyPtr prop = getUnboundProperty(propName); - prop = checkForRefPropAndGetBoundProp(prop, objPtr); + prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); if (!prop.assigned()) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" not found)", propName)); @@ -1946,7 +1598,7 @@ ErrCode GenericPropertyObjectImpl::clearPropert } PropertyPtr prop = getUnboundPropertyOrNull(propName); - prop = checkForRefPropAndGetBoundProp(prop, objPtr); + prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); if (!prop.assigned()) { @@ -2206,7 +1858,7 @@ ErrCode GenericPropertyObjectImpl::addPropertyI if (!propName.assigned()) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDVALUE, fmt::format(R"(Property "{}" does not have an assigned name.)", propName)); - if (hasDuplicateReferences(propPtr, objPtr)) + if (details::hasDuplicateReferences(propPtr, objPtr)) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDVALUE, fmt::format(R"(Reference property "{}" references a property that is already referenced by another.)", propName)); @@ -2805,25 +2457,6 @@ ErrCode GenericPropertyObjectImpl::findProperti return errCode; } -template -Bool GenericPropertyObjectImpl::checkIsReferenced(const StringPtr& referencedPropName, - const PropertyInternalPtr& prop) -{ - const auto refProp = prop.getReferencedPropertyUnresolved(); - if (!refProp.assigned()) - return false; - - for (auto propName : refProp.getPropertyReferences()) - { - if (propName == referencedPropName) - { - return true; - } - } - - return false; -} - template ErrCode GenericPropertyObjectImpl::checkForReferencesInternal(IProperty* property, Bool* isReferenced) { @@ -2839,14 +2472,14 @@ ErrCode GenericPropertyObjectImpl::checkForRefe { for (const auto& prop : objectClass.getProperties(True)) { - if (*isReferenced = checkIsReferenced(name, prop); *isReferenced) + if (*isReferenced = details::checkIsReferenced(name, prop); *isReferenced) return OPENDAQ_SUCCESS; } } for (const auto& prop : localProperties) { - if (*isReferenced = checkIsReferenced(name, prop.second); *isReferenced) + if (*isReferenced = details::checkIsReferenced(name, prop.second); *isReferenced) return OPENDAQ_SUCCESS; } return OPENDAQ_SUCCESS; From 317b30fc0dff7630b4f7f1aa53032daeba2ff7a4 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 16:04:32 +0200 Subject: [PATCH 03/34] Replace recursive child-property dispatch with iterative getParentObject Child property access (a.b.c.prop) previously recursed through the full operation at every path level - each hop re-entered the internal set/get/clear, re-parsed the remaining path, and re-acquired locks. getParentObject now walks the dot-separated path iteratively: it splits off the leaf name, resolves one segment per hop (first hop through the object's own internal getter, further hops through the child's public getPropertyValue), and returns the direct parent of the leaf property. The operation-specific machinery then runs exactly once, at the leaf. All nine dispatch sites (set, protected set, clear, get, selection set, getProperty, both event getters) now share the resolver and keep only their operation-specific leaf action. getChildPropertyValue is removed. hasProperty and selection get inherit the iterative walk through getPropertyValue/getProperty. Per-hop semantics (ref-property resolution, read events, permission checks, error codes) are unchanged. Intermediate objects' locks are no longer held across the leaf operation. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 200 ++++++++---------- 1 file changed, 83 insertions(+), 117 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 5f5a0ec685..6625eff085 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -410,8 +410,10 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::getClassName return OPENDAQ_SUCCESS; } -#if defined(__GNUC__) && __GNUC__ >= 12 - #pragma GCC diagnostic push - #pragma GCC diagnostic ignored "-Wdangling-pointer" -#endif - template -ErrCode GenericPropertyObjectImpl::getChildPropertyValue(const StringPtr& childName, - const StringPtr& subName, - BaseObjectPtr& value) +ErrCode GenericPropertyObjectImpl::getParentObject(const StringPtr& path, + PropertyObjectPtr& parentObj, + StringPtr& leafName) { - PropertyPtr prop; - StringPtr name; + StringPtr parentPath; + details::splitOnLastDot(path, parentPath, leafName); - auto err = daqTry([&]() -> auto + const std::string parentPathStr = parentPath; + PropertyObjectPtr current; + size_t start = 0; + + while (true) { - prop = getUnboundProperty(childName); + const size_t pos = parentPathStr.find('.', start); + const SizeT segmentLength = (pos == std::string::npos ? parentPathStr.size() : pos) - start; + const StringPtr segment = String(parentPathStr.c_str() + start, segmentLength); - prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); - name = prop.getName(); - return OPENDAQ_SUCCESS; - }); + BaseObjectPtr childValue; + const ErrCode err = current.assigned() ? current->getPropertyValue(segment, &childValue) + : getPropertyValueInternal(segment, &childValue); + OPENDAQ_RETURN_IF_FAILED(err); - OPENDAQ_RETURN_IF_FAILED(err); + current = childValue.template asPtrOrNull(true); + if (!current.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOINTERFACE, fmt::format(R"(Property "{}" is not a property object)", segment)); - if (!prop.assigned()) - { - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", name)); + if (pos == std::string::npos) + break; + start = pos + 1; } - BaseObjectPtr childProp; - err = getPropertyValueInternal(name, &childProp); - OPENDAQ_RETURN_IF_FAILED(err); - - err = daqTry([&]() -> auto - { - const auto childPropAsPropertyObject = childProp.template asPtr(true); - value = childPropAsPropertyObject.getPropertyValue(subName); - }); - OPENDAQ_RETURN_IF_FAILED(err); - return err; + parentObj = current; + return OPENDAQ_SUCCESS; } -#if defined(__GNUC__) && __GNUC__ >= 12 - #pragma GCC diagnostic pop -#endif - template ErrCode GenericPropertyObjectImpl::callPropertyValueWrite(const PropertyPtr& prop, BaseObjectPtr& newValue, @@ -791,40 +783,33 @@ ErrCode GenericPropertyObjectImpl::setPropertyV return OPENDAQ_SUCCESS; } - StringPtr subName; if (isChildProp) { - details::splitOnFirstDot(propName, propName, subName); - } - - PropertyPtr prop = getUnboundProperty(propName); - prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); - - if (!prop.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" not found.)", propName)); - - propName = prop.getName(); - - if (isChildProp) - { - BaseObjectPtr childProp; - const ErrCode err = getPropertyValueInternal(propName, &childProp); - OPENDAQ_RETURN_IF_FAILED(err); + PropertyObjectPtr parentObj; + StringPtr leafName; + OPENDAQ_RETURN_IF_FAILED(getParentObject(propName, parentObj, leafName)); if (protectedAccess) { - const auto childPropAsPropertyObject = childProp.template asPtr(true); - childPropAsPropertyObject.setProtectedPropertyValue(subName, valuePtr); + const auto parentObjProtected = parentObj.template asPtr(true); + parentObjProtected.setProtectedPropertyValue(leafName, valuePtr); } else { - const auto childPropAsPropertyObject = childProp.template asPtr(true); - childPropAsPropertyObject.setPropertyValue(subName, valuePtr); + parentObj.setPropertyValue(leafName, valuePtr); } return OPENDAQ_SUCCESS; } + PropertyPtr prop = getUnboundProperty(propName); + prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); + + if (!prop.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" not found.)", propName)); + + propName = prop.getName(); + const auto propInternal = prop.asPtr(); // TODO: If function type, check if return value is correct type. if (!protectedAccess) @@ -1246,23 +1231,18 @@ ErrCode GenericPropertyObjectImpl::setPropertyS if (details::isChildProperty(propName)) { - StringPtr childName; - StringPtr subName; - details::splitOnFirstDot(propName, childName, subName); - - BaseObjectPtr childProp; - const ErrCode err = getPropertyValueInternal(childName, &childProp); - OPENDAQ_RETURN_IF_FAILED(err); + PropertyObjectPtr parentObj; + StringPtr leafName; + OPENDAQ_RETURN_IF_FAILED(getParentObject(propName, parentObj, leafName)); if (protectedAccess) { - const auto childPropAsPropertyObject = childProp.template asPtr(true); - return childPropAsPropertyObject->setProtectedPropertySelectionValue(subName, value); + const auto parentObjProtected = parentObj.template asPtr(true); + return parentObjProtected->setProtectedPropertySelectionValue(leafName, value); } else { - const auto childPropAsPropertyObject = childProp.template asPtr(true); - return childPropAsPropertyObject->setPropertySelectionValue(subName, value); + return parentObj->setPropertySelectionValue(leafName, value); } } @@ -1590,11 +1570,23 @@ ErrCode GenericPropertyObjectImpl::clearPropert return OPENDAQ_SUCCESS; } - StringPtr subName; - const auto isChildProp = details::isChildProperty(propName); - if (isChildProp) + if (details::isChildProperty(propName)) { - details::splitOnFirstDot(propName, propName, subName); + PropertyObjectPtr parentObj; + StringPtr leafName; + OPENDAQ_RETURN_IF_FAILED(getParentObject(propName, parentObj, leafName)); + + if (protectedAccess) + { + const auto parentObjProtected = parentObj.template asPtr(true); + parentObjProtected.clearProtectedPropertyValue(leafName); + } + else + { + parentObj.clearPropertyValue(leafName); + } + + return OPENDAQ_SUCCESS; } PropertyPtr prop = getUnboundPropertyOrNull(propName); @@ -1610,30 +1602,12 @@ ErrCode GenericPropertyObjectImpl::clearPropert if (!protectedAccess) { - if (propInternal.getReadOnlyNoLock() && !isChildProp) + if (propInternal.getReadOnlyNoLock()) { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_ACCESSDENIED, fmt::format(R"(Property "{}" is read only)", propName)); } } - if (isChildProp) - { - BaseObjectPtr childProp; - const ErrCode err = getPropertyValueInternal(propName, &childProp); - OPENDAQ_RETURN_IF_FAILED(err); - - if (protectedAccess) - { - const auto childPropAsPropertyObject = childProp.template asPtr(true); - childPropAsPropertyObject.clearProtectedPropertyValue(subName); - } - else - { - const auto childPropAsPropertyObject = childProp.template asPtr(true); - childPropAsPropertyObject.clearPropertyValue(subName); - } - } - else { if (propValues.find(prop.getName()) == propValues.end()) return OPENDAQ_IGNORED; @@ -1707,9 +1681,11 @@ ErrCode GenericPropertyObjectImpl::getPropertyV if (details::isChildProperty(propName)) { - StringPtr subName; - details::splitOnFirstDot(propName, propName, subName); - err = getChildPropertyValue(propName, subName, valuePtr); + PropertyObjectPtr parentObj; + StringPtr leafName; + err = getParentObject(propName, parentObj, leafName); + OPENDAQ_RETURN_IF_FAILED(err); + err = parentObj->getPropertyValue(leafName, &valuePtr); } else { @@ -1811,15 +1787,11 @@ ErrCode GenericPropertyObjectImpl::getProperty( if (details::isChildProperty(propName)) { - StringPtr subName; - BaseObjectPtr childProp; + PropertyObjectPtr parentObj; + StringPtr leafName; + OPENDAQ_RETURN_IF_FAILED(getParentObject(propName, parentObj, leafName)); - details::splitOnFirstDot(propName, propName, subName); - const ErrCode err = getPropertyValueInternal(propName, &childProp); - OPENDAQ_RETURN_IF_FAILED(err); - - const auto childPropAsPropertyObject = childProp.template asPtr(true); - prop = childPropAsPropertyObject.getProperty(subName); + prop = parentObj.getProperty(leafName); } else { @@ -2112,15 +2084,12 @@ ErrCode GenericPropertyObjectImpl::getOnPropert if (details::isChildProperty(name)) { - StringPtr subName; - details::splitOnFirstDot(name, name, subName); - - BaseObjectPtr childProp; - ErrCode errCode = getPropertyValueInternal(name, &childProp); + PropertyObjectPtr parentObj; + StringPtr leafName; + ErrCode errCode = getParentObject(name, parentObj, leafName); OPENDAQ_RETURN_IF_FAILED(errCode); - const auto childPropAsPropertyObject = childProp.template asPtr(true); - errCode = childPropAsPropertyObject->getOnPropertyValueWrite(subName, event); + errCode = parentObj->getOnPropertyValueWrite(leafName, event); OPENDAQ_RETURN_IF_FAILED(errCode); return errCode; } @@ -2151,15 +2120,12 @@ ErrCode GenericPropertyObjectImpl::getOnPropert if (details::isChildProperty(name)) { - StringPtr subName; - details::splitOnFirstDot(name, name, subName); - - BaseObjectPtr childProp; - ErrCode errCode = getPropertyValueInternal(name, &childProp); + PropertyObjectPtr parentObj; + StringPtr leafName; + ErrCode errCode = getParentObject(name, parentObj, leafName); OPENDAQ_RETURN_IF_FAILED(errCode); - const auto childPropAsPropertyObject = childProp.template asPtr(true); - errCode = childPropAsPropertyObject->getOnPropertyValueRead(subName, event); + errCode = parentObj->getOnPropertyValueRead(leafName, event); OPENDAQ_RETURN_IF_FAILED(errCode); return errCode; } From 40ca68b749643b14d35474c03ce3a485acef0d86 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 16:13:09 +0200 Subject: [PATCH 04/34] Extract selection key/value conversion into shared helpers setPropertySelectionValueInternal and getPropertySelectionValueInternal carried ~135 lines of selection-type branching. The conversions now live in details::selectionValueToKey (user value -> stored index/key) and details::selectionKeyToValue (stored key -> selection value), next to checkSelectionValues. Both internals reduce to: resolve the bound property, convert, and delegate to the normal value get/set path. Error codes are unchanged; helper messages now name the resolved property instead of the raw input path. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_helpers.h | 112 ++++++++++++++++++ .../coreobjects/property_object_impl.h | 100 +--------------- 2 files changed, 115 insertions(+), 97 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h index 0c9d3db333..d2c1fe5b90 100644 --- a/core/coreobjects/include/coreobjects/property_object_helpers.h +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -333,6 +333,118 @@ inline ErrCode checkPropertyTypeAndConvert(const PropertyPtr& prop, BaseObjectPt return errCode; } +// Maps a user-facing selection value to the index/key that is stored as the property value +inline ErrCode selectionValueToKey(const PropertyPtr& prop, const BaseObjectPtr& valuePtr, BaseObjectPtr& indexOrKey) +{ + const auto propInternal = prop.asPtr(true); + const auto selectionValues = propInternal.getSelectionValuesNoLock(); + const PropertyType propType = prop.getPropertyType(); + const auto propName = prop.getName(); + + if (propType == PropertyType::IndexSelection) + { + if (!selectionValues.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, + fmt::format(R"(Index selection property "{}" has no selection values assigned)", propName)); + + const auto valuesList = selectionValues.template asPtrOrNull(true); + if (!valuesList.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, + fmt::format(R"(Index selection property "{}" values is not a list)", propName)); + + for (SizeT i = 0; i < valuesList.getCount(); ++i) + { + if (valuesList.getItemAt(i) == valuePtr) + { + indexOrKey = Int(i); + break; + } + } + + if (!indexOrKey.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Value not found in selection values of property "{}")", propName)); + } + else if (propType == PropertyType::SparseSelection) + { + if (!selectionValues.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, + fmt::format(R"(Sparse selection property "{}" has no selection values assigned)", propName)); + + const auto valuesDict = selectionValues.template asPtrOrNull(true); + if (!valuesDict.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, + fmt::format(R"(Sparse selection property "{}" values is not a dictionary)", propName)); + + for (const auto& [key, value] : valuesDict) + { + if (value == valuePtr) + { + indexOrKey = key; + break; + } + } + + if (!indexOrKey.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Value not found in sparse selection values of property "{}")", propName)); + } + else if (propType == PropertyType::Selection) + { + indexOrKey = valuePtr; + } + else + { + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Property "{}" is not an index selection or sparse selection property)", propName)); + } + + return OPENDAQ_SUCCESS; +} + +// Resolves the stored index/key of a selection property to the corresponding selection value, in place +inline ErrCode selectionKeyToValue(const PropertyPtr& prop, BaseObjectPtr& valuePtr) +{ + const auto propInternal = prop.asPtr(true); + const auto values = propInternal.getSelectionValuesNoLock(); + const auto propName = prop.getName(); + + if (!values.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Selection property "{}" has no selection values assigned)", propName)); + + const PropertyType propType = prop.getPropertyType(); + if (propType == PropertyType::IndexSelection) + { + const auto valuesList = values.asPtrOrNull(true); + if (!valuesList.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, + fmt::format(R"(Index selection property "{}" values is not a list)", propName)); + valuePtr = valuesList.getItemAt(valuePtr); + } + else if (propType == PropertyType::SparseSelection) + { + const auto valuesDict = values.asPtrOrNull(true); + if (!valuesDict.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, + fmt::format(R"(Sparse selection property "{}" values is not a dictionary)", propName)); + valuePtr = valuesDict.get(valuePtr); + } + else if (propType == PropertyType::Selection) + { + if (propInternal.getValueTypeNoLock() != valuePtr.getCoreType()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(Selection item type mismatch for property "{}")", propName)); + + return OPENDAQ_SUCCESS; + } + else + { + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, + fmt::format(R"(Property "{}" is not an index selection or sparse selection property)", propName)); + } + + if (propInternal.getItemTypeNoLock() != valuePtr.getCoreType()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(List item type mismatch for property "{}")", propName)); + + return OPENDAQ_SUCCESS; +} + // Coercion/Validation inline void coercePropertyWrite(const PropertyPtr& prop, ObjectPtr& valuePtr, const PropertyObjectPtr& objPtr) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 6625eff085..1dd2573524 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -1251,66 +1251,9 @@ ErrCode GenericPropertyObjectImpl::setPropertyS if (!prop.assigned()) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" not found)", propName)); - - const auto propInternal = prop.asPtr(true); - const auto selectionValues = propInternal.getSelectionValuesNoLock(); - BaseObjectPtr indexOrKey; - PropertyType propType = prop.getPropertyType(); - - if (propType == PropertyType::IndexSelection) - { - if (!selectionValues.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, - fmt::format(R"(Index selection property "{}" has no selection values assigned)", propName)); - - const auto valuesList = selectionValues.template asPtrOrNull(true); - if (!valuesList.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, - fmt::format(R"(Index selection property "{}" values is not a list)", propName)); - - for (SizeT i = 0; i < valuesList.getCount(); ++i) - { - if (valuesList.getItemAt(i) == valuePtr) - { - indexOrKey = Int(i); - break; - } - } - - if (!indexOrKey.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Value not found in selection values of property "{}")", propName)); - } - else if (propType == PropertyType::SparseSelection) - { - if (!selectionValues.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, - fmt::format(R"(Sparse selection property "{}" has no selection values assigned)", propName)); - - const auto valuesDict = selectionValues.template asPtrOrNull(true); - if (!valuesDict.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, - fmt::format(R"(Sparse selection property "{}" values is not a dictionary)", propName)); - - for (const auto& [key, value] : valuesDict) - { - if (value == valuePtr) - { - indexOrKey = key; - break; - } - } - if (!indexOrKey.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Value not found in sparse selection values of property "{}")", propName)); - } - else if (propType == PropertyType::Selection) - { - indexOrKey = valuePtr; - } - else - { - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Property "{}" is not an index selection or sparse selection property)", propName)); - } + BaseObjectPtr indexOrKey; + OPENDAQ_RETURN_IF_FAILED(details::selectionValueToKey(prop, valuePtr, indexOrKey)); return setPropertyValueInternal(propertyName, indexOrKey, true, protectedAccess, updateCount > 0); }); @@ -1728,44 +1671,7 @@ ErrCode GenericPropertyObjectImpl::getPropertyS OPENDAQ_RETURN_IF_FAILED(errCode, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); } - const auto propInternal = prop.asPtr(true); - auto values = propInternal.getSelectionValuesNoLock(); - if (!values.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Selection property "{}" has no selection values assigned)", propName)); - - const PropertyType propType = prop.getPropertyType(); - if (propType == PropertyType::IndexSelection) - { - const auto valuesList = values.asPtrOrNull(true); - if (!valuesList.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, - fmt::format(R"(Index selection property "{}" values is not a list)", propName)); - valuePtr = valuesList.getItemAt(valuePtr); - } - else if (propType == PropertyType::SparseSelection) - { - const auto valuesDict = values.asPtrOrNull(true); - if (!valuesDict.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, - fmt::format(R"(Sparse selection property "{}" values is not a dictionary)", propName)); - valuePtr = valuesDict.get(valuePtr); - } - else if (propType == PropertyType::Selection) - { - if (propInternal.getValueTypeNoLock() != valuePtr.getCoreType()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(Selection item type mismatch for property "{}")", propName)); - - *value = valuePtr.detach(); - return OPENDAQ_SUCCESS; - } - else - { - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, - fmt::format(R"(Property "{}" is not an index selection or sparse selection property)", propName)); - } - - if (propInternal.getItemTypeNoLock() != valuePtr.getCoreType()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(List item type mismatch for property "{}")", propName)); + OPENDAQ_RETURN_IF_FAILED(details::selectionKeyToValue(prop, valuePtr)); *value = valuePtr.detach(); return OPENDAQ_SUCCESS; From 4bac6ba44e49c2b310b9acebf0b911a7e50724e5 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 17:53:31 +0200 Subject: [PATCH 05/34] Decompose getPropertyAndValueInternal The 90-line method mixed name parsing, ref resolution, effective-name computation, updating-stack reads, default-value fallback with list indexing, container cloning, and read-event dispatch. Three of those concerns move to pure helpers in property_object_helpers.h: - details::buildEffectivePropertyName - the storage/lookup name for a resolved property, preserving [index] suffixes (resolves the long-standing TODO to extract this) - details::readDefaultPropertyValue - default-value fallback, including indexing into list defaults - details::cloneContainerValue - defensive clone of list/dict values on read What remains is a linear orchestrator: parse -> bind -> read (updating stack or local value) -> default fallback -> clone -> read event. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_helpers.h | 59 +++++++++++++++++++ .../coreobjects/property_object_impl.h | 44 +------------- 2 files changed, 62 insertions(+), 41 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h index d2c1fe5b90..9daa562660 100644 --- a/core/coreobjects/include/coreobjects/property_object_helpers.h +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -333,6 +334,64 @@ inline ErrCode checkPropertyTypeAndConvert(const PropertyPtr& prop, BaseObjectPt return errCode; } +// Computes the name under which a property's value is stored/looked up, given the resolved +// property and the raw queried name. Reference properties store under the referenced name; +// a "[index]" suffix from the raw name is preserved. +inline StringPtr buildEffectivePropertyName(const StringPtr& parsedName, + const StringPtr& rawName, + const PropertyPtr& resolvedProperty, + bool isReferenced, + ConstCharPtr bracket) +{ + if (bracket != nullptr) + return isReferenced ? StringPtr(resolvedProperty.getName() + std::string(bracket)) : rawName; + + if (isReferenced) + return resolvedProperty.getName(); + + return parsedName; +} + +// Reads the property's default value, indexing into it when a "[index]" suffix was queried. +// A missing default is not an error; value is simply left unassigned. +inline ErrCode readDefaultPropertyValue(const PropertyPtr& property, const StringPtr& propName, ConstCharPtr bracket, BaseObjectPtr& value) +{ + const auto propInternal = property.asPtr(); + const ErrCode res = propInternal->getDefaultValueNoLock(&value); + + if (OPENDAQ_FAILED(res)) + daqClearErrorInfo(); + + if (!value.assigned()) + return OPENDAQ_SUCCESS; + + if (value.getCoreType() == ctList && bracket != nullptr) + { + const int index = parseIndex(bracket); + ListPtr list = value; + if (index >= static_cast(list.getCount())) + { + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_OUTOFRANGE, fmt::format(R"(The index parameter is out of bounds of the list for property "{}")", propName)); + } + value = list[std::size_t(index)]; + } + + return OPENDAQ_SUCCESS; +} + +// Container values are cloned on read so the stored value cannot be mutated through the returned reference +inline BaseObjectPtr cloneContainerValue(const BaseObjectPtr& value) +{ + const CoreType coreType = value.getCoreType(); + if (coreType == ctList || coreType == ctDict) + { + BaseObjectPtr clonedValue; + value.asPtr()->clone(&clonedValue); + return clonedValue; + } + return value; +} + // Maps a user-facing selection value to the index/key that is stored as the property value inline ErrCode selectionValueToKey(const PropertyPtr& prop, const BaseObjectPtr& valuePtr, BaseObjectPtr& indexOrKey) { diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 1dd2573524..d4b10a825b 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -1101,23 +1101,7 @@ ErrCode GenericPropertyObjectImpl::getPropertyA bool isRef; property = details::checkForRefPropAndGetBoundProp(property, objPtr, &isRef); - - // TODO: Extract this to own function - if (bracket != nullptr) - { - if (isRef) - { - propName = property.getName() + std::string(bracket); - } - else - { - propName = name; - } - } - else if (isRef) - { - propName = property.getName(); - } + propName = details::buildEffectivePropertyName(propName, name, property, isRef, bracket); ErrCode res = OPENDAQ_SUCCESS; @@ -1135,35 +1119,13 @@ ErrCode GenericPropertyObjectImpl::getPropertyA if (res == OPENDAQ_ERR_NOTFOUND) { daqClearErrorInfo(); - const auto propInternal = property.asPtr(); - res = propInternal->getDefaultValueNoLock(&value); - - if (OPENDAQ_FAILED(res)) - daqClearErrorInfo(); + OPENDAQ_RETURN_IF_FAILED(details::readDefaultPropertyValue(property, propName, bracket, value)); if (!value.assigned()) return OPENDAQ_SUCCESS; - - CoreType coreType = value.getCoreType(); - if (coreType == ctList && bracket != nullptr) - { - int index = details::parseIndex(bracket); - ListPtr list = value; - if (index >= static_cast(list.getCount())) - { - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_OUTOFRANGE, fmt::format(R"(The index parameter is out of bounds of the list for property "{}")", propName)); - } - value = list[std::size_t(index)]; - } } - CoreType coreType = value.getCoreType(); - if (coreType == ctList || coreType == ctDict) - { - BaseObjectPtr clonedValue; - value.asPtr()->clone(&clonedValue); - value = clonedValue.detach(); - } + value = details::cloneContainerValue(value); if (triggerEvent) value = callPropertyValueRead(property, value); From d3223fe8191d8e49353ef133a3779fdeeec86f23 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 18:00:38 +0200 Subject: [PATCH 06/34] Rename cloneContainerValue to cloneIfContainerValue The function is a no-op for non-container values; the name now says so. Co-Authored-By: Claude Fable 5 --- core/coreobjects/include/coreobjects/property_object_helpers.h | 2 +- core/coreobjects/include/coreobjects/property_object_impl.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h index 9daa562660..ee704051bf 100644 --- a/core/coreobjects/include/coreobjects/property_object_helpers.h +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -380,7 +380,7 @@ inline ErrCode readDefaultPropertyValue(const PropertyPtr& property, const Strin } // Container values are cloned on read so the stored value cannot be mutated through the returned reference -inline BaseObjectPtr cloneContainerValue(const BaseObjectPtr& value) +inline BaseObjectPtr cloneIfContainerValue(const BaseObjectPtr& value) { const CoreType coreType = value.getCoreType(); if (coreType == ctList || coreType == ctDict) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index d4b10a825b..231cb41d9c 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -1125,7 +1125,7 @@ ErrCode GenericPropertyObjectImpl::getPropertyA return OPENDAQ_SUCCESS; } - value = details::cloneContainerValue(value); + value = details::cloneIfContainerValue(value); if (triggerEvent) value = callPropertyValueRead(property, value); From 3cdde9ef961ab0f43da602580d36b2e7ab0218da Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 18:13:41 +0200 Subject: [PATCH 07/34] Split getPropertyAndValueInternal into bind and read helpers getPropertyAndValueInternal conflated three concerns behind two bool flags: binding the property (name parse, lookup, reference resolution), reading its value, and firing read events. Split it into: - getBoundPropertyInternal: lookup + reference resolution, outputs the bound property, effective storage name, and bracket suffix - readPropertyValueInternal: updating-stack/local/default value read, never fires events Callers now invoke callPropertyValueRead explicitly where read events are wanted, making endApplyUpdate's read-event firing during update application visible (kept for behavior parity, marked TODO). Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 75 ++++++++++++++----- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 231cb41d9c..08dbcaa341 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -389,8 +389,13 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::triggerCoreEven } template -ErrCode GenericPropertyObjectImpl::getPropertyAndValueInternal(const StringPtr& name, - BaseObjectPtr& value, - PropertyPtr& property, - bool triggerEvent, - bool retrieveUpdatingValue) +ErrCode GenericPropertyObjectImpl::getBoundPropertyInternal(const StringPtr& name, + PropertyPtr& property, + StringPtr& effectiveName, + ConstCharPtr& bracket) { StringPtr propName; - ConstCharPtr bracket = details::getPropNameWithoutIndex(name, propName); + bracket = details::getPropNameWithoutIndex(name, propName); property = getUnboundPropertyOrNull(propName); @@ -1101,35 +1105,40 @@ ErrCode GenericPropertyObjectImpl::getPropertyA bool isRef; property = details::checkForRefPropAndGetBoundProp(property, objPtr, &isRef); - propName = details::buildEffectivePropertyName(propName, name, property, isRef, bracket); + effectiveName = details::buildEffectivePropertyName(propName, name, property, isRef, bracket); + return OPENDAQ_SUCCESS; +} +template +ErrCode GenericPropertyObjectImpl::readPropertyValueInternal(const PropertyPtr& property, + const StringPtr& effectiveName, + ConstCharPtr bracket, + bool retrieveUpdatingValue, + BaseObjectPtr& value) +{ ErrCode res = OPENDAQ_SUCCESS; - if (retrieveUpdatingValue && updatePropertyStack.getPropertyValue(propName, value)) + if (retrieveUpdatingValue && updatePropertyStack.getPropertyValue(effectiveName, value)) { if (!value.assigned()) value = property.getDefaultValue(); } else { - res = readLocalValue(propName, value); + res = readLocalValue(effectiveName, value); } OPENDAQ_RETURN_IF_FAILED_EXCEPT(res, OPENDAQ_ERR_NOTFOUND); if (res == OPENDAQ_ERR_NOTFOUND) { daqClearErrorInfo(); - OPENDAQ_RETURN_IF_FAILED(details::readDefaultPropertyValue(property, propName, bracket, value)); + OPENDAQ_RETURN_IF_FAILED(details::readDefaultPropertyValue(property, effectiveName, bracket, value)); if (!value.assigned()) return OPENDAQ_SUCCESS; } value = details::cloneIfContainerValue(value); - - if (triggerEvent) - value = callPropertyValueRead(property, value); - return OPENDAQ_SUCCESS; } @@ -1421,9 +1430,14 @@ ErrCode GenericPropertyObjectImpl::clearPropert if (prop.getValueType() == ctObject) { + const StringPtr propName = prop.getName(); PropertyPtr propPtr; + StringPtr effectiveName; + ConstCharPtr bracket; BaseObjectPtr valuePtr; - ErrCode err = getPropertyAndValueInternal(prop.getName(), valuePtr, propPtr, false); + ErrCode err = getBoundPropertyInternal(propName, propPtr, effectiveName, bracket); + OPENDAQ_RETURN_IF_FAILED(err); + err = readPropertyValueInternal(propPtr, effectiveName, bracket, false, valuePtr); OPENDAQ_RETURN_IF_FAILED(err); if (const auto freezable = valuePtr.asPtrOrNull(true); freezable.assigned() && freezable.isFrozen()) @@ -1595,7 +1609,14 @@ ErrCode GenericPropertyObjectImpl::getPropertyV else { PropertyPtr prop; - err = getPropertyAndValueInternal(propName, valuePtr, prop, true, retrieveUpdatingValue); + StringPtr effectiveName; + ConstCharPtr bracket; + err = getBoundPropertyInternal(propName, prop, effectiveName, bracket); + OPENDAQ_RETURN_IF_FAILED(err); + err = readPropertyValueInternal(prop, effectiveName, bracket, retrieveUpdatingValue, valuePtr); + OPENDAQ_RETURN_IF_FAILED(err); + if (valuePtr.assigned()) + valuePtr = callPropertyValueRead(prop, valuePtr); } OPENDAQ_RETURN_IF_FAILED(err); @@ -1629,8 +1650,14 @@ ErrCode GenericPropertyObjectImpl::getPropertyS } else { - const ErrCode errCode = getPropertyAndValueInternal(propName, valuePtr, prop, true, retrieveUpdatingValue); + StringPtr effectiveName; + ConstCharPtr bracket; + ErrCode errCode = getBoundPropertyInternal(propName, prop, effectiveName, bracket); OPENDAQ_RETURN_IF_FAILED(errCode, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); + errCode = readPropertyValueInternal(prop, effectiveName, bracket, retrieveUpdatingValue, valuePtr); + OPENDAQ_RETURN_IF_FAILED(errCode, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); + if (valuePtr.assigned()) + valuePtr = callPropertyValueRead(prop, valuePtr); } OPENDAQ_RETURN_IF_FAILED(details::selectionKeyToValue(prop, valuePtr)); @@ -2195,7 +2222,15 @@ void GenericPropertyObjectImpl::endApplyUpdate( if (err != OPENDAQ_IGNORED) { PropertyPtr prop; - getPropertyAndValueInternal(name, action.value, prop); + StringPtr effectiveName; + ConstCharPtr bracket; + if (OPENDAQ_SUCCEEDED(getBoundPropertyInternal(name, prop, effectiveName, bracket)) && + OPENDAQ_SUCCEEDED(readPropertyValueInternal(prop, effectiveName, bracket, false, action.value)) && + action.value.assigned()) + { + // TODO: firing read events while applying updates is likely unintended; kept for behavior parity + action.value = callPropertyValueRead(prop, action.value); + } appliedUpdates.emplace_back(name, action); } } From 61752831c078b05ffc21fb8886e453db7bf20237 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 18:20:55 +0200 Subject: [PATCH 08/34] Deduplicate default-value comparison in local value writes shouldWriteLocalValue and writeLocalValue each hand-rolled the same compare-against-property-default check; extract it into differsFromDefaultValue and flatten writeLocalValue branching. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 53 ++++++++----------- 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 08dbcaa341..33a4068bcb 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -378,6 +378,8 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::setPropertyV template -bool GenericPropertyObjectImpl::shouldWriteLocalValue(const StringPtr& name, const BaseObjectPtr& value) const +bool GenericPropertyObjectImpl::differsFromDefaultValue(const StringPtr& name, const BaseObjectPtr& value) const { - auto it = propValues.find(name); - if (it != propValues.end()) + try { - return it->second != value; + return objPtr.getProperty(name).template asPtr().getDefaultValueNoLock() != value; } - else + catch (...) { - try - { - return objPtr.getProperty(name).template asPtr().getDefaultValueNoLock() != value; - } - catch(...) - { - } } return true; } +template +bool GenericPropertyObjectImpl::shouldWriteLocalValue(const StringPtr& name, const BaseObjectPtr& value) const +{ + const auto it = propValues.find(name); + if (it != propValues.end()) + return it->second != value; + + return differsFromDefaultValue(name, value); +} + template bool GenericPropertyObjectImpl::writeLocalValue(const StringPtr& name, const BaseObjectPtr& value, bool forceWrite) { - auto it = propValues.find(name); + const auto it = propValues.find(name); if (it != propValues.end()) { if (it->second == value) return false; it->second = value; + return true; } - else if (forceWrite) + + if (forceWrite || differsFromDefaultValue(name, value)) { propValues.emplace(name, value); - } - else - { - bool shouldWrite = true; - try - { - shouldWrite = objPtr.getProperty(name).template asPtr().getDefaultValueNoLock() != value; - } - catch (...) - { - } - - if (shouldWrite) - propValues.emplace(name, value); - else - return false; + return true; } - return true; + return false; } template From 72e05c37a707e25083ee05408bed3bc3c42b8133 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 18:24:52 +0200 Subject: [PATCH 09/34] Add combined bind+read overload; drop redundant re-bind in clearPropertyValues All read-path callers were calling getBoundPropertyInternal and readPropertyValueInternal back to back; add a name-based readPropertyValueInternal overload that does both so call sites lose the effectiveName/bracket boilerplate. clearPropertyValuesInternal already holds a bound, non-reference property from the getAllProperties loop, so re-looking it up and re-resolving it was pure waste (its property out-param was never used); it now reads the value directly via the bound-property overload. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 33a4068bcb..c521c04edf 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -398,6 +398,8 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::readProperty return OPENDAQ_SUCCESS; } +template +ErrCode GenericPropertyObjectImpl::readPropertyValueInternal(const StringPtr& name, + bool retrieveUpdatingValue, + PropertyPtr& property, + BaseObjectPtr& value) +{ + StringPtr effectiveName; + ConstCharPtr bracket; + OPENDAQ_RETURN_IF_FAILED(getBoundPropertyInternal(name, property, effectiveName, bracket)); + return readPropertyValueInternal(property, effectiveName, bracket, retrieveUpdatingValue, value); +} + #if defined(__GNUC__) && __GNUC__ >= 12 #pragma GCC diagnostic pop #endif @@ -1421,14 +1435,9 @@ ErrCode GenericPropertyObjectImpl::clearPropert if (prop.getValueType() == ctObject) { - const StringPtr propName = prop.getName(); - PropertyPtr propPtr; - StringPtr effectiveName; - ConstCharPtr bracket; + // `prop` is already bound and cannot be a reference here; read the value directly BaseObjectPtr valuePtr; - ErrCode err = getBoundPropertyInternal(propName, propPtr, effectiveName, bracket); - OPENDAQ_RETURN_IF_FAILED(err); - err = readPropertyValueInternal(propPtr, effectiveName, bracket, false, valuePtr); + const ErrCode err = readPropertyValueInternal(prop, prop.getName(), nullptr, false, valuePtr); OPENDAQ_RETURN_IF_FAILED(err); if (const auto freezable = valuePtr.asPtrOrNull(true); freezable.assigned() && freezable.isFrozen()) @@ -1600,11 +1609,7 @@ ErrCode GenericPropertyObjectImpl::getPropertyV else { PropertyPtr prop; - StringPtr effectiveName; - ConstCharPtr bracket; - err = getBoundPropertyInternal(propName, prop, effectiveName, bracket); - OPENDAQ_RETURN_IF_FAILED(err); - err = readPropertyValueInternal(prop, effectiveName, bracket, retrieveUpdatingValue, valuePtr); + err = readPropertyValueInternal(propName, retrieveUpdatingValue, prop, valuePtr); OPENDAQ_RETURN_IF_FAILED(err); if (valuePtr.assigned()) valuePtr = callPropertyValueRead(prop, valuePtr); @@ -1641,11 +1646,7 @@ ErrCode GenericPropertyObjectImpl::getPropertyS } else { - StringPtr effectiveName; - ConstCharPtr bracket; - ErrCode errCode = getBoundPropertyInternal(propName, prop, effectiveName, bracket); - OPENDAQ_RETURN_IF_FAILED(errCode, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); - errCode = readPropertyValueInternal(prop, effectiveName, bracket, retrieveUpdatingValue, valuePtr); + const ErrCode errCode = readPropertyValueInternal(propName, retrieveUpdatingValue, prop, valuePtr); OPENDAQ_RETURN_IF_FAILED(errCode, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); if (valuePtr.assigned()) valuePtr = callPropertyValueRead(prop, valuePtr); @@ -2213,11 +2214,7 @@ void GenericPropertyObjectImpl::endApplyUpdate( if (err != OPENDAQ_IGNORED) { PropertyPtr prop; - StringPtr effectiveName; - ConstCharPtr bracket; - if (OPENDAQ_SUCCEEDED(getBoundPropertyInternal(name, prop, effectiveName, bracket)) && - OPENDAQ_SUCCEEDED(readPropertyValueInternal(prop, effectiveName, bracket, false, action.value)) && - action.value.assigned()) + if (OPENDAQ_SUCCEEDED(readPropertyValueInternal(name, false, prop, action.value)) && action.value.assigned()) { // TODO: firing read events while applying updates is likely unintended; kept for behavior parity action.value = callPropertyValueRead(prop, action.value); From 89335a9b15527f798199ed930efce0ec4004f068 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 18:26:54 +0200 Subject: [PATCH 10/34] Rename updatingPropsAndValues to batchedUpdates The member holds the begin/endUpdate batch queue, but its old name read as if it belonged to updatePropertyStack (the write-event reentrancy guard), which is an unrelated mechanism. Also updates the references in the commented-out config client block. Co-Authored-By: Claude Fable 5 --- .../include/coreobjects/property_object_impl.h | 10 +++++----- .../config_client_property_object_impl.h | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index c521c04edf..069cf28ec9 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -336,7 +336,7 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak owner; int updateCount; - UpdatingActions updatingPropsAndValues; + UpdatingActions batchedUpdates; WeakRefPtr manager; std::vector customOrder; StringPtr path; @@ -788,7 +788,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyV if (batch && !isChildProp) { - updatingPropsAndValues.emplace_back(std::make_pair(propName, UpdatingAction{true, protectedAccess, valuePtr})); + batchedUpdates.emplace_back(std::make_pair(propName, UpdatingAction{true, protectedAccess, valuePtr})); return OPENDAQ_SUCCESS; } @@ -1485,7 +1485,7 @@ ErrCode GenericPropertyObjectImpl::clearPropert if (batch) { - updatingPropsAndValues.emplace_back(std::make_pair(propName, UpdatingAction{false, protectedAccess, nullptr})); + batchedUpdates.emplace_back(std::make_pair(propName, UpdatingAction{false, protectedAccess, nullptr})); return OPENDAQ_SUCCESS; } @@ -2186,13 +2186,13 @@ std::mutex* GenericPropertyObjectImpl::getLocal template void GenericPropertyObjectImpl::beginApplyUpdate() { - beginApplyProperties(updatingPropsAndValues, isParentUpdating()); + beginApplyProperties(batchedUpdates, isParentUpdating()); } template void GenericPropertyObjectImpl::endApplyUpdate() { - UpdatingActions localUpdates = std::move(updatingPropsAndValues); + UpdatingActions localUpdates = std::move(batchedUpdates); UpdatingActions appliedUpdates; appliedUpdates.reserve(localUpdates.size()); diff --git a/shared/libraries/config_protocol/include/config_protocol/config_client_property_object_impl.h b/shared/libraries/config_protocol/include/config_protocol/config_client_property_object_impl.h index acbf76851a..ee38274cf7 100644 --- a/shared/libraries/config_protocol/include/config_protocol/config_client_property_object_impl.h +++ b/shared/libraries/config_protocol/include/config_protocol/config_client_property_object_impl.h @@ -810,7 +810,7 @@ void ConfigClientPropertyObjectBaseImpl::endApplyUpdate() propsAndValuesEx = List(); auto ignoredProps = List(); - for (auto& item : this->updatingPropsAndValues) + for (auto& item : this->batchedUpdates) { auto itemEx = Dict(); itemEx.set("Name", String(item.first)); @@ -823,7 +823,7 @@ void ConfigClientPropertyObjectBaseImpl::endApplyUpdate() else applyUpdatingPropsAndValuesProtocolVer0(); - this->updatingPropsAndValues.clear(); + this->batchedUpdates.clear(); clientComm->endUpdate(remoteGlobalId, getPathInternal(), propsAndValuesEx); } @@ -831,7 +831,7 @@ void ConfigClientPropertyObjectBaseImpl::endApplyUpdate() template void ConfigClientPropertyObjectBaseImpl::applyUpdatingPropsAndValuesProtocolVer0() { - for (const auto& item : this->updatingPropsAndValues) + for (const auto& item : this->batchedUpdates) { if (item.second.setValue) { From 6a5751ba47812b0940e43614081e4fd552d1d78a Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 18:28:20 +0200 Subject: [PATCH 11/34] Share the child-object iteration between begin/endUpdate propagation callBeginUpdateOnChildren and callEndUpdateOnChildren duplicated the same skip-non-objects, skip-frozen loop; extract it into forEachUnfrozenChildObject. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 069cf28ec9..e0952e3b0e 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -302,6 +302,24 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak + void forEachUnfrozenChildObject(Handler&& handler) + { + for (const auto& [_, propValue] : propValues) + { + const auto propObj = propValue.template asPtrOrNull(true); + if (!propObj.assigned()) + continue; + + const auto freezable = propObj.template asPtrOrNull(true); + if (freezable.assigned() && freezable.isFrozen()) + continue; + + handler(propObj); + } + } + virtual PropertyObjectPtr getPropertyObjectParent(); virtual PropertyObjectPtr cloneChildPropertyObject(const PropertyPtr& prop); @@ -2063,35 +2081,13 @@ ErrCode GenericPropertyObjectImpl::beginUpdate( template void GenericPropertyObjectImpl::callBeginUpdateOnChildren() { - for (const auto& [_, propValue] : propValues) - { - const auto propObj = propValue.template asPtrOrNull(true); - if (!propObj.assigned()) - continue; - - auto freezable = propObj.template asPtrOrNull(true); - if (freezable.assigned() && freezable.isFrozen()) - continue; - - propObj.beginUpdate(); - } + forEachUnfrozenChildObject([](const PropertyObjectPtr& propObj) { propObj.beginUpdate(); }); } template void GenericPropertyObjectImpl::callEndUpdateOnChildren() { - for (const auto& [_, propValue] : propValues) - { - const auto propObj = propValue.template asPtrOrNull(true); - if (!propObj.assigned()) - continue; - - auto freezable = propObj.template asPtrOrNull(true); - if (freezable.assigned() && freezable.isFrozen()) - continue; - - propObj.endUpdate(); - } + forEachUnfrozenChildObject([](const PropertyObjectPtr& propObj) { propObj.endUpdate(); }); } template From 8d0e06841551b3a679c92d1331afb6a4435b3587 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 18:30:09 +0200 Subject: [PATCH 12/34] Deduplicate getOnPropertyValueWrite/getOnPropertyValueRead Both were ~35 identical lines differing only in the event map, the child-dispatch target, and the error string; fold them into getPropertyValueEventInternal. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 64 +++++++------------ 1 file changed, 22 insertions(+), 42 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index e0952e3b0e..bc04833f91 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -434,6 +434,9 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::getPropertie } template -ErrCode GenericPropertyObjectImpl::getOnPropertyValueWrite(IString* propertyName, IEvent** event) +ErrCode GenericPropertyObjectImpl::getPropertyValueEventInternal(IString* propertyName, IEvent** event, bool valueWrite) { OPENDAQ_PARAM_NOT_NULL(propertyName); OPENDAQ_PARAM_NOT_NULL(event); - StringPtr name = StringPtr::Borrow( propertyName); + StringPtr name = StringPtr::Borrow(propertyName); if (details::isChildProperty(name)) { PropertyObjectPtr parentObj; StringPtr leafName; - ErrCode errCode = getParentObject(name, parentObj, leafName); + const ErrCode errCode = getParentObject(name, parentObj, leafName); OPENDAQ_RETURN_IF_FAILED(errCode); - errCode = parentObj->getOnPropertyValueWrite(leafName, event); - OPENDAQ_RETURN_IF_FAILED(errCode); - return errCode; + return valueWrite ? parentObj->getOnPropertyValueWrite(leafName, event) + : parentObj->getOnPropertyValueRead(leafName, event); } Bool hasProp; - ErrCode err = this->hasProperty(name, &hasProp); + const ErrCode err = this->hasProperty(name, &hasProp); OPENDAQ_RETURN_IF_FAILED(err); if (!hasProp) @@ -2008,49 +2010,27 @@ ErrCode GenericPropertyObjectImpl::getOnPropert PropertyInternalPtr prop = getUnboundProperty(name); if (prop.getReferencedPropertyUnresolved().assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALID_OPERATION, fmt::format(R"(getOnPropertyValueWrite is not allowed for the reference properties "{}")", name)); + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALID_OPERATION, + fmt::format(R"({} is not allowed for the reference properties "{}")", + valueWrite ? "getOnPropertyValueWrite" : "getOnPropertyValueRead", + name)); - auto [it, _] = valueWriteEvents.try_emplace(name); + auto& events = valueWrite ? valueWriteEvents : valueReadEvents; + auto [it, _] = events.try_emplace(name); *event = it->second.addRefAndReturn(); return OPENDAQ_SUCCESS; } template -ErrCode GenericPropertyObjectImpl::getOnPropertyValueRead(IString* propertyName, IEvent** event) +ErrCode GenericPropertyObjectImpl::getOnPropertyValueWrite(IString* propertyName, IEvent** event) { - OPENDAQ_PARAM_NOT_NULL(propertyName); - OPENDAQ_PARAM_NOT_NULL(event); - - StringPtr name = StringPtr::Borrow(propertyName); - - if (details::isChildProperty(name)) - { - PropertyObjectPtr parentObj; - StringPtr leafName; - ErrCode errCode = getParentObject(name, parentObj, leafName); - OPENDAQ_RETURN_IF_FAILED(errCode); - - errCode = parentObj->getOnPropertyValueRead(leafName, event); - OPENDAQ_RETURN_IF_FAILED(errCode); - return errCode; - } - - Bool hasProp; - ErrCode err = this->hasProperty(name, &hasProp); - OPENDAQ_RETURN_IF_FAILED(err); - - if (!hasProp) - { - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", name)); - } - - PropertyInternalPtr prop = getUnboundProperty(name); - if (prop.getReferencedPropertyUnresolved().assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALID_OPERATION, fmt::format(R"(getOnPropertyValueRead is not allowed for the reference properties "{}")", name)); + return getPropertyValueEventInternal(propertyName, event, true); +} - auto [it, _] = valueReadEvents.try_emplace(name); - *event = it->second.addRefAndReturn(); - return OPENDAQ_SUCCESS; +template +ErrCode GenericPropertyObjectImpl::getOnPropertyValueRead(IString* propertyName, IEvent** event) +{ + return getPropertyValueEventInternal(propertyName, event, false); } template From bbe5db85db758bde593960ab1ecd0100ec2a0e48 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 18:31:40 +0200 Subject: [PATCH 13/34] Route hasProperty child paths through getParentObject hasProperty was the last site hand-rolling parent resolution (splitOnLastDot plus a full getPropertyValue on the parent path). NOTFOUND and NOINTERFACE now uniformly map to hasProperty = False; previously a non-object segment in the middle of a deep path errored while one at the end reported False. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index bc04833f91..16aab3ec8c 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -2930,26 +2930,18 @@ ErrCode GenericPropertyObjectImpl::hasProperty( if (details::isChildProperty(propName)) { - BaseObjectPtr val; - StringPtr childStr; - details::splitOnLastDot(propName, propName, childStr); - - ErrCode err = getPropertyValue(propName, &val); - OPENDAQ_RETURN_IF_FAILED_EXCEPT(err, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Failed to retrieve child object with name {})", propName)); - if (err == OPENDAQ_ERR_NOTFOUND) - { - *hasProperty = False; - return OPENDAQ_SUCCESS; - } - - PropertyObjectPtr obj = val.asPtrOrNull(true); - if (!obj.assigned()) + PropertyObjectPtr parentObj; + StringPtr leafName; + const ErrCode err = getParentObject(propName, parentObj, leafName); + if (err == OPENDAQ_ERR_NOTFOUND || err == OPENDAQ_ERR_NOINTERFACE) { + daqClearErrorInfo(); *hasProperty = False; return OPENDAQ_SUCCESS; } + OPENDAQ_RETURN_IF_FAILED(err, fmt::format(R"(Failed to retrieve child object for property "{}")", propName)); - return obj->hasProperty(childStr, hasProperty); + return parentObj->hasProperty(leafName, hasProperty); } if (localProperties.find(propertyName) != localProperties.cend()) From e9ff145304ce2f9484d0868147ebac3a14aba9c1 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 18:34:01 +0200 Subject: [PATCH 14/34] Converge property lookup on getUnboundPropertyOrNull Delete the throwing getUnboundProperty variant; all sites now use the null-returning lookup with an explicit NOTFOUND error. This makes the set path consistent with the clear path (its post-resolution not-found check was previously unreachable for missing properties, since the throwing lookup fired first with a different message) and normalizes the error text to a single wording. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 16aab3ec8c..dbc65fd4f2 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -393,7 +393,6 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::setPropertyV return OPENDAQ_SUCCESS; } - PropertyPtr prop = getUnboundProperty(propName); + PropertyPtr prop = getUnboundPropertyOrNull(propName); prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); if (!prop.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" not found.)", propName)); + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", propName)); propName = prop.getName(); @@ -968,21 +967,6 @@ void GenericPropertyObjectImpl::setOwnerToPrope } } -template -PropertyPtr GenericPropertyObjectImpl::getUnboundProperty(const StringPtr& name) -{ - const auto res = localProperties.find(name); - if (res == localProperties.end()) - { - if (objectClass == nullptr) - DAQ_THROW_EXCEPTION(NotFoundException, R"(Property with name {} does not exist.)", name); - - return objectClass.getProperty(name); - } - - return res->second; -} - template PropertyPtr GenericPropertyObjectImpl::getUnboundPropertyOrNull(const StringPtr& name) const { @@ -1243,11 +1227,11 @@ ErrCode GenericPropertyObjectImpl::setPropertyS } } - PropertyPtr prop = getUnboundProperty(propName); + PropertyPtr prop = getUnboundPropertyOrNull(propName); prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); if (!prop.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" not found)", propName)); + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", propName)); BaseObjectPtr indexOrKey; OPENDAQ_RETURN_IF_FAILED(details::selectionValueToKey(prop, valuePtr, indexOrKey)); @@ -1703,7 +1687,10 @@ ErrCode GenericPropertyObjectImpl::getProperty( } else { - prop = getUnboundProperty(propName); + prop = getUnboundPropertyOrNull(propName); + if (!prop.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", propName)); + prop = prop.asPtr().cloneWithOwner(objPtr); } @@ -2008,7 +1995,10 @@ ErrCode GenericPropertyObjectImpl::getPropertyV if (!hasProp) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", name)); - PropertyInternalPtr prop = getUnboundProperty(name); + PropertyInternalPtr prop = getUnboundPropertyOrNull(name); + if (!prop.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", name)); + if (prop.getReferencedPropertyUnresolved().assigned()) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALID_OPERATION, fmt::format(R"({} is not allowed for the reference properties "{}")", From f38cc83a5739c211068312c6dbc1652f42947018 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 19:28:26 +0200 Subject: [PATCH 15/34] Drop redundant hasProperty pre-check in getPropertyValueEventInternal The pre-check existed to guard the old throwing lookup; since the convergence on getUnboundPropertyOrNull it duplicated the same localProperties/objectClass lookup with the identical error result. Co-Authored-By: Claude Fable 5 --- .../coreobjects/include/coreobjects/property_object_impl.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index dbc65fd4f2..387d44b824 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -1988,13 +1988,6 @@ ErrCode GenericPropertyObjectImpl::getPropertyV : parentObj->getOnPropertyValueRead(leafName, event); } - Bool hasProp; - const ErrCode err = this->hasProperty(name, &hasProp); - OPENDAQ_RETURN_IF_FAILED(err); - - if (!hasProp) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", name)); - PropertyInternalPtr prop = getUnboundPropertyOrNull(name); if (!prop.assigned()) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", name)); From f923950ff89ffcac0388000667b7a5df9fdb5658 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 19:30:41 +0200 Subject: [PATCH 16/34] Collapse protected/plain duplication in clearPropertyValueInternal object branch The nested-object clear loop existed twice, differing only in which clear call it made per child property. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 387d44b824..4782e9eede 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -1545,24 +1545,18 @@ ErrCode GenericPropertyObjectImpl::clearPropert freezable.assigned() && freezable.isFrozen()) return OPENDAQ_IGNORED; + const auto obj = it->second.template asPtr(true); + PropertyObjectProtectedPtr objProtected; if (protectedAccess) + objProtected = it->second.template asPtr(true); + + for (const auto& childProp : obj.getAllProperties()) { - auto objProtected = it->second.template asPtr(true); - auto obj = it->second.template asPtr(true); - for (const auto& childProp: obj.getAllProperties()) - { + if (protectedAccess) objProtected.clearProtectedPropertyValue(childProp.getName()); - } - } - else - { - auto obj = it->second.template asPtr(true); - for (const auto& childProp: obj.getAllProperties()) - { + else obj.clearPropertyValue(childProp.getName()); - } } - } } else From baab34021510301fc9ca73730adc97c6edc3cc4a Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 19:35:51 +0200 Subject: [PATCH 17/34] Extract shared three-tier event dispatch from callPropertyValueRead/Write Both fired the same class-level, per-property, and any-property event sequence; fold it into firePropertyValueEvents. Guarding is unchanged: only the per-property write tier runs under daqTry, class-level and any-property handlers still propagate exceptions (the resulting update-stack leak on throw is a known issue to fix separately). Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 79 +++++++++---------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 4782e9eede..452873c222 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -433,6 +433,11 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::callProperty daqClearErrorInfo(); oldValue = defaultValue; } - errCode = OPENDAQ_SUCCESS; - PropertyValueEventArgsPtr args; if (changeType == PropertyEventType::Clear) args = PropertyValueEventArgs(prop, defaultValue, oldValue, changeType, isUpdating); else args = PropertyValueEventArgs(prop, newValue, oldValue, changeType, isUpdating); - if (!localProperties.count(name)) - { - const PropertyValueEventEmitter propEvent{prop.asPtr(true).getClassOnPropertyValueWrite()}; - if (propEvent.hasListeners()) - propEvent(objPtr, args); - } - - if (valueWriteEvents.find(name) != valueWriteEvents.end()) - { - if (valueWriteEvents[name].hasListeners()) - errCode = daqTry([&] { valueWriteEvents[name](objPtr, args); }); - } - - if (valueWriteEvents[AnyWriteEventName].hasListeners()) - { - valueWriteEvents[AnyWriteEventName](objPtr, args); - } + errCode = firePropertyValueEvents(prop, args, true); bool shouldUpdate = updatePropertyStack.unregisterPropertyUpdating(name); // If the event execution failed, forward the error code @@ -723,39 +710,49 @@ ErrCode GenericPropertyObjectImpl::callProperty } template -BaseObjectPtr GenericPropertyObjectImpl::callPropertyValueRead(const PropertyPtr& prop, - const BaseObjectPtr& readValue) +ErrCode GenericPropertyObjectImpl::firePropertyValueEvents(const PropertyPtr& prop, + PropertyValueEventArgsPtr& args, + bool valueWrite) { - if (!prop.assigned()) - { - return readValue; - } - - auto args = PropertyValueEventArgs(prop, readValue, readValue, PropertyEventType::Read, False); + ErrCode errCode = OPENDAQ_SUCCESS; + const auto name = prop.getName(); + auto& events = valueWrite ? valueWriteEvents : valueReadEvents; - if (!localProperties.count(prop.getName())) + if (!localProperties.count(name)) { - const PropertyValueEventEmitter propEvent{prop.asPtr().getClassOnPropertyValueRead()}; - if (propEvent.hasListeners()) - { - propEvent(objPtr, args); - } + const auto propInternal = prop.asPtr(true); + const PropertyValueEventEmitter classEvent{valueWrite ? propInternal.getClassOnPropertyValueWrite() + : propInternal.getClassOnPropertyValueRead()}; + if (classEvent.hasListeners()) + classEvent(objPtr, args); } - const auto name = prop.getName(); - if (valueReadEvents.find(name) != valueReadEvents.end()) + if (const auto it = events.find(name); it != events.end() && it->second.hasListeners()) { - if (valueReadEvents[name].hasListeners()) - { - valueReadEvents[name](objPtr, args); - } + if (valueWrite) + errCode = daqTry([&] { it->second(objPtr, args); }); + else + it->second(objPtr, args); } - if (valueReadEvents[AnyReadEventName].hasListeners()) + auto& anyEvent = events[valueWrite ? AnyWriteEventName : AnyReadEventName]; + if (anyEvent.hasListeners()) + anyEvent(objPtr, args); + + return errCode; +} + +template +BaseObjectPtr GenericPropertyObjectImpl::callPropertyValueRead(const PropertyPtr& prop, + const BaseObjectPtr& readValue) +{ + if (!prop.assigned()) { - valueReadEvents[AnyReadEventName](objPtr, args); + return readValue; } + auto args = PropertyValueEventArgs(prop, readValue, readValue, PropertyEventType::Read, False); + firePropertyValueEvents(prop, args, false); return args.getValue(); } From 2e9035db6fb5b5efdecc544319efe0a919f1f353 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 19:40:43 +0200 Subject: [PATCH 18/34] Merge configureClonedMembers overloads via namespace-scoped CloneParameters The 9-argument overload existed only because CloneParameters was a nested type, so each template instantiation had its own and clone() could not pass one across instantiations. Hoist the struct (and the event emitter aliases it needs) to namespace scope as PropertyObjectCloneParameters, keep the in-class CloneParameters alias, and fold the 9-argument body into the single overload. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 96 ++++++------------- 1 file changed, 30 insertions(+), 66 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 452873c222..10d8edce4a 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -60,6 +60,24 @@ BEGIN_NAMESPACE_OPENDAQ using PropertyOrderedMap = tsl::ordered_map; +using PropertyValueEventEmitter = EventEmitter; +using EndUpdateEventEmitter = EventEmitter; + +// Snapshot of the members a clone copies from its source. Namespace-scoped (not nested) so that +// all GenericPropertyObjectImpl instantiations share the type and can configure clones of one another. +struct PropertyObjectCloneParameters +{ + const std::unordered_map& valueWriteEvents; + const std::unordered_map& valueReadEvents; + const EndUpdateEventEmitter& endUpdateEvent; + const ProcedurePtr& triggerCoreEvent; + const PropertyOrderedMap& localProperties; + const std::unordered_map& propValues; + const std::vector& customOrder; + const PermissionManagerPtr& permissionManager; + const std::set& corePropertyNames; +}; + namespace config_protocol { class ConfigClientDeviceInfoImpl; @@ -171,32 +189,9 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak; - using EndUpdateEventEmitter = EventEmitter; - - struct CloneParameters - { - const std::unordered_map& valueWriteEvents; - const std::unordered_map& valueReadEvents; - const EndUpdateEventEmitter& endUpdateEvent; - const ProcedurePtr& triggerCoreEvent; - const PropertyOrderedMap& localProperties; - const std::unordered_map& propValues; - const std::vector& customOrder; - const PermissionManagerPtr& permissionManager; - const std::set& corePropertyNames; - }; + using CloneParameters = PropertyObjectCloneParameters; void configureClonedMembers(const CloneParameters& parameters); - void configureClonedMembers(const std::unordered_map& valueWriteEvents, - const std::unordered_map& valueReadEvents, - const EndUpdateEventEmitter& endUpdateEvent, - const ProcedurePtr& triggerCoreEvent, - const PropertyOrderedMap& localProperties, - const std::unordered_map& propValues, - const std::vector& customOrder, - const PermissionManagerPtr& permissionManager, - const std::set& corePropertyNames); // TODO: Make remove friend classes once private methods are properly exposed in protected scope. template @@ -1248,40 +1243,17 @@ ErrCode GenericPropertyObjectImpl::clearProtect template void GenericPropertyObjectImpl::configureClonedMembers(const CloneParameters& parameters) -{ - configureClonedMembers(parameters.valueWriteEvents, - parameters.valueReadEvents, - parameters.endUpdateEvent, - parameters.triggerCoreEvent, - parameters.localProperties, - parameters.propValues, - parameters.customOrder, - parameters.permissionManager, - parameters.corePropertyNames); -} - -template -void GenericPropertyObjectImpl::configureClonedMembers( - const std::unordered_map& valueWriteEvents, - const std::unordered_map& valueReadEvents, - const EndUpdateEventEmitter& endUpdateEvent, - const ProcedurePtr& triggerCoreEvent, - const PropertyOrderedMap& localProperties, - const std::unordered_map& propValues, - const std::vector& customOrder, - const PermissionManagerPtr& permissionManager, - const std::set& corePropertyNames) { this->valueWriteEvents.clear(); - for (const auto& [name, srcEmitter] : valueWriteEvents) + for (const auto& [name, srcEmitter] : parameters.valueWriteEvents) { BaseObjectPtr cloned; srcEmitter.template asPtr(true)->clone(&cloned); this->valueWriteEvents.emplace(name, cloned); } - + this->valueReadEvents.clear(); - for (const auto& [name, srcEmitter] : valueReadEvents) + for (const auto& [name, srcEmitter] : parameters.valueReadEvents) { BaseObjectPtr cloned; srcEmitter.template asPtr(true)->clone(&cloned); @@ -1289,19 +1261,19 @@ void GenericPropertyObjectImpl::configureCloned } BaseObjectPtr cloned; - endUpdateEvent.template asPtr(true)->clone(&cloned); + parameters.endUpdateEvent.template asPtr(true)->clone(&cloned); this->endUpdateEvent = cloned; - this->triggerCoreEvent = triggerCoreEvent; - this->localProperties = localProperties; - this->customOrder = customOrder; - this->corePropertyNames = corePropertyNames; + this->triggerCoreEvent = parameters.triggerCoreEvent; + this->localProperties = parameters.localProperties; + this->customOrder = parameters.customOrder; + this->corePropertyNames = parameters.corePropertyNames; BaseObjectPtr permissionManagerClone; - permissionManager.template asPtr()->clone(&permissionManagerClone); + parameters.permissionManager.template asPtr()->clone(&permissionManagerClone); this->permissionManager = permissionManagerClone; - for (const auto& val : propValues) + for (const auto& val : parameters.propValues) { const auto& propName = val.first; const auto& prop = val.second; @@ -2391,15 +2363,7 @@ ErrCode GenericPropertyObjectImpl::clone(IPrope const ErrCode errCode = daqTry([this, &obj, &cloned]() { auto implPtr = static_cast(obj.getObject()); - implPtr->configureClonedMembers(valueWriteEvents, - valueReadEvents, - endUpdateEvent, - triggerCoreEvent, - localProperties, - propValues, - customOrder, - permissionManager, - corePropertyNames); + implPtr->configureClonedMembers(getCloneParameters()); *cloned = obj.detach(); return OPENDAQ_SUCCESS; From 6acc25be0e59a451f3626dedecfec01ee5eaec37 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 19:42:13 +0200 Subject: [PATCH 19/34] Simplify getPropertiesInternal ordering tail The custom-order branch duplicated the append-the-remaining-properties loop; with an empty customOrder the first loop is a no-op, so the branch is unnecessary. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 10d8edce4a..4f3208d29f 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -1901,31 +1901,21 @@ ErrCode GenericPropertyObjectImpl::getPropertie } auto properties = List(); - if (!customOrder.empty()) - { - // Add properties with explicit order - for (auto& propName : customOrder) - { - const auto iter = lookup.find(propName); - if (iter != lookup.cend()) - { - properties.unsafePushBack(iter->second); - lookup.erase(iter); - } - } - // Add the rest of without set order - for (auto& prop : lookup) + // Add properties with explicit order first, then the rest in default order + for (const auto& propName : customOrder) + { + const auto iter = lookup.find(propName); + if (iter != lookup.cend()) { - properties.unsafePushBack(prop.second); + properties.unsafePushBack(iter->second); + lookup.erase(iter); } } - else + + for (const auto& prop : lookup) { - for (auto& prop : lookup) - { - properties.unsafePushBack(prop.second); - } + properties.unsafePushBack(prop.second); } *list = properties.detach(); From ea0233a5cbe129518d02fd2846b9e7be4ae444b6 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 19:56:23 +0200 Subject: [PATCH 20/34] Rename effectiveName to resolvedName The name is the queried name rewritten onto the resolved property after reference resolution; resolved states the transformation, effective did not. Also renames buildEffectivePropertyName accordingly. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_helpers.h | 2 +- .../coreobjects/property_object_impl.h | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h index ee704051bf..534642e3d7 100644 --- a/core/coreobjects/include/coreobjects/property_object_helpers.h +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -337,7 +337,7 @@ inline ErrCode checkPropertyTypeAndConvert(const PropertyPtr& prop, BaseObjectPt // Computes the name under which a property's value is stored/looked up, given the resolved // property and the raw queried name. Reference properties store under the referenced name; // a "[index]" suffix from the raw name is preserved. -inline StringPtr buildEffectivePropertyName(const StringPtr& parsedName, +inline StringPtr buildResolvedPropertyName(const StringPtr& parsedName, const StringPtr& rawName, const PropertyPtr& resolvedProperty, bool isReferenced, diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 4f3208d29f..48ed72e8f8 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -404,12 +404,12 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::triggerCoreEven template ErrCode GenericPropertyObjectImpl::getBoundPropertyInternal(const StringPtr& name, PropertyPtr& property, - StringPtr& effectiveName, + StringPtr& resolvedName, ConstCharPtr& bracket) { StringPtr propName; @@ -1095,34 +1095,34 @@ ErrCode GenericPropertyObjectImpl::getBoundProp bool isRef; property = details::checkForRefPropAndGetBoundProp(property, objPtr, &isRef); - effectiveName = details::buildEffectivePropertyName(propName, name, property, isRef, bracket); + resolvedName = details::buildResolvedPropertyName(propName, name, property, isRef, bracket); return OPENDAQ_SUCCESS; } template ErrCode GenericPropertyObjectImpl::readPropertyValueInternal(const PropertyPtr& property, - const StringPtr& effectiveName, + const StringPtr& resolvedName, ConstCharPtr bracket, bool retrieveUpdatingValue, BaseObjectPtr& value) { ErrCode res = OPENDAQ_SUCCESS; - if (retrieveUpdatingValue && updatePropertyStack.getPropertyValue(effectiveName, value)) + if (retrieveUpdatingValue && updatePropertyStack.getPropertyValue(resolvedName, value)) { if (!value.assigned()) value = property.getDefaultValue(); } else { - res = readLocalValue(effectiveName, value); + res = readLocalValue(resolvedName, value); } OPENDAQ_RETURN_IF_FAILED_EXCEPT(res, OPENDAQ_ERR_NOTFOUND); if (res == OPENDAQ_ERR_NOTFOUND) { daqClearErrorInfo(); - OPENDAQ_RETURN_IF_FAILED(details::readDefaultPropertyValue(property, effectiveName, bracket, value)); + OPENDAQ_RETURN_IF_FAILED(details::readDefaultPropertyValue(property, resolvedName, bracket, value)); if (!value.assigned()) return OPENDAQ_SUCCESS; @@ -1138,10 +1138,10 @@ ErrCode GenericPropertyObjectImpl::readProperty PropertyPtr& property, BaseObjectPtr& value) { - StringPtr effectiveName; + StringPtr resolvedName; ConstCharPtr bracket; - OPENDAQ_RETURN_IF_FAILED(getBoundPropertyInternal(name, property, effectiveName, bracket)); - return readPropertyValueInternal(property, effectiveName, bracket, retrieveUpdatingValue, value); + OPENDAQ_RETURN_IF_FAILED(getBoundPropertyInternal(name, property, resolvedName, bracket)); + return readPropertyValueInternal(property, resolvedName, bracket, retrieveUpdatingValue, value); } #if defined(__GNUC__) && __GNUC__ >= 12 From 071bafa90a6a828403b504463e8a2173a464bbc9 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 20:00:57 +0200 Subject: [PATCH 21/34] Move forEachUnfrozenChildObject to private scope It is an implementation detail of the base begin/endUpdate propagation; subclass overrides extend to components, not propValues children, and have no use for it. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 48ed72e8f8..f8a1b7f6e4 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -297,24 +297,6 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak - void forEachUnfrozenChildObject(Handler&& handler) - { - for (const auto& [_, propValue] : propValues) - { - const auto propObj = propValue.template asPtrOrNull(true); - if (!propObj.assigned()) - continue; - - const auto freezable = propObj.template asPtrOrNull(true); - if (freezable.assigned() && freezable.isFrozen()) - continue; - - handler(propObj); - } - } - virtual PropertyObjectPtr getPropertyObjectParent(); virtual PropertyObjectPtr cloneChildPropertyObject(const PropertyPtr& prop); @@ -364,6 +346,24 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak + void forEachUnfrozenChildObject(Handler&& handler) + { + for (const auto& [_, propValue] : propValues) + { + const auto propObj = propValue.template asPtrOrNull(true); + if (!propObj.assigned()) + continue; + + const auto freezable = propObj.template asPtrOrNull(true); + if (freezable.assigned() && freezable.isFrozen()) + continue; + + handler(propObj); + } + } + static void DeserializePropertyValues(const SerializedObjectPtr& serialized, const BaseObjectPtr& context, const FunctionPtr& factoryCallback, From b3bfd3df8a98100b77bddf219a6204d970aa4073 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 20:08:44 +0200 Subject: [PATCH 22/34] Remove dead default-value walk from disableCoreEventTrigger Since child object properties are materialized into propValues at add time (with non-core defaults replaced by inert clones and core-property defaults aliasing the propValues entry), the defaults iterated by the second loop either never had their trigger enabled or were already disabled by the propValues loop. Leftover from the pre-rework value-on-demand semantics; enableCoreEventTrigger never had a counterpart walk. Co-Authored-By: Claude Fable 5 --- .../include/coreobjects/property_object_impl.h | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index f8a1b7f6e4..aa77845325 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -2303,24 +2303,6 @@ ErrCode GenericPropertyObjectImpl::disableCoreE } } - for (const auto& item : localProperties) - { - if (item.second.assigned()) - { - const auto propInternal = item.second.template asPtr(); - if (propInternal.getValueTypeUnresolved() == ctObject) - { - const auto defaultVal = item.second.getDefaultValue(); - if (defaultVal.assigned()) - { - const auto objInternal = defaultVal.template asPtrOrNull(); - if (objInternal.assigned()) - objInternal.disableCoreEventTrigger(); - } - } - } - } - return OPENDAQ_SUCCESS; } From f97c0d997b2dc5578bfc676aec3d09f5b361f84a Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 21:13:02 +0200 Subject: [PATCH 23/34] Make property write events exception-safe A handler throwing from the class-level or any-property write tier unwound past unregisterPropertyUpdating, leaking the update-stack entry. Since all public reads consult the updating stack first, the leaked entry became a phantom value: every read returned the never-committed value while propValues (the state that serializes and clones) stayed empty, and re-setting that value was silently ignored. All write tiers now run under daqTry (all tiers still fire, first error wins) and callPropertyValueWrite unregisters on unwind as a backstop. Pinned by a test verified to fail against the previous implementation. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 100 +++++++++++------- .../tests/test_value_changed_events.cpp | 28 +++++ 2 files changed, 89 insertions(+), 39 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index aa77845325..c0d9e9c83d 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -669,39 +669,51 @@ ErrCode GenericPropertyObjectImpl::callProperty } } - BaseObjectPtr oldValue; - ErrCode errCode = readLocalValue(name, oldValue); - if (errCode == OPENDAQ_ERR_NOTFOUND) + bool unregistered = false; + try { - daqClearErrorInfo(); - oldValue = defaultValue; - } - PropertyValueEventArgsPtr args; - if (changeType == PropertyEventType::Clear) - args = PropertyValueEventArgs(prop, defaultValue, oldValue, changeType, isUpdating); - else - args = PropertyValueEventArgs(prop, newValue, oldValue, changeType, isUpdating); + BaseObjectPtr oldValue; + ErrCode errCode = readLocalValue(name, oldValue); + if (errCode == OPENDAQ_ERR_NOTFOUND) + { + daqClearErrorInfo(); + oldValue = defaultValue; + } + PropertyValueEventArgsPtr args; + if (changeType == PropertyEventType::Clear) + args = PropertyValueEventArgs(prop, defaultValue, oldValue, changeType, isUpdating); + else + args = PropertyValueEventArgs(prop, newValue, oldValue, changeType, isUpdating); - errCode = firePropertyValueEvents(prop, args, true); + errCode = firePropertyValueEvents(prop, args, true); - bool shouldUpdate = updatePropertyStack.unregisterPropertyUpdating(name); - // If the event execution failed, forward the error code - OPENDAQ_RETURN_IF_FAILED(errCode); + const bool shouldUpdate = updatePropertyStack.unregisterPropertyUpdating(name); + unregistered = true; + // If the event execution failed, forward the error code + OPENDAQ_RETURN_IF_FAILED(errCode); - if (shouldUpdate) - { - // setting the final value is only done in the top level of the stack - if (changeType == PropertyEventType::Clear && args.getValue() == defaultValue) - return OPENDAQ_SUCCESS; + if (shouldUpdate) + { + // setting the final value is only done in the top level of the stack + if (changeType == PropertyEventType::Clear && args.getValue() == defaultValue) + return OPENDAQ_SUCCESS; - if (newValue == args.getValue()) - return OPENDAQ_SUCCESS; - - // if the value changed, we have to validate new value before setting it - newValue = args.getValue(); - return setPropertyValueInternal(name, newValue, false, true, false); + if (newValue == args.getValue()) + return OPENDAQ_SUCCESS; + + // if the value changed, we have to validate new value before setting it + newValue = args.getValue(); + return setPropertyValueInternal(name, newValue, false, true, false); + } + return OPENDAQ_IGNORED; + } + catch (...) + { + // A registration leaked past a throw would permanently poison the update stack for this property + if (!unregistered) + updatePropertyStack.unregisterPropertyUpdating(name); + throw; } - return OPENDAQ_IGNORED; } template @@ -713,26 +725,36 @@ ErrCode GenericPropertyObjectImpl::fireProperty const auto name = prop.getName(); auto& events = valueWrite ? valueWriteEvents : valueReadEvents; + // Write tiers are guarded so a throwing handler surfaces as an error instead of unwinding + // past the caller's update-stack bookkeeping; all tiers still fire, the first error wins. + const auto fire = [&](const PropertyValueEventEmitter& emitter) + { + if (!emitter.hasListeners()) + return; + + if (!valueWrite) + { + emitter(objPtr, args); + return; + } + + const ErrCode err = daqTry([&] { emitter(objPtr, args); }); + if (OPENDAQ_FAILED(err) && OPENDAQ_SUCCEEDED(errCode)) + errCode = err; + }; + if (!localProperties.count(name)) { const auto propInternal = prop.asPtr(true); const PropertyValueEventEmitter classEvent{valueWrite ? propInternal.getClassOnPropertyValueWrite() : propInternal.getClassOnPropertyValueRead()}; - if (classEvent.hasListeners()) - classEvent(objPtr, args); + fire(classEvent); } - if (const auto it = events.find(name); it != events.end() && it->second.hasListeners()) - { - if (valueWrite) - errCode = daqTry([&] { it->second(objPtr, args); }); - else - it->second(objPtr, args); - } + if (const auto it = events.find(name); it != events.end()) + fire(it->second); - auto& anyEvent = events[valueWrite ? AnyWriteEventName : AnyReadEventName]; - if (anyEvent.hasListeners()) - anyEvent(objPtr, args); + fire(events[valueWrite ? AnyWriteEventName : AnyReadEventName]); return errCode; } diff --git a/core/coreobjects/tests/test_value_changed_events.cpp b/core/coreobjects/tests/test_value_changed_events.cpp index 851a39df9d..6b26c7419a 100644 --- a/core/coreobjects/tests/test_value_changed_events.cpp +++ b/core/coreobjects/tests/test_value_changed_events.cpp @@ -506,3 +506,31 @@ TEST_F(PropertyValueChangedEventsTest, AnyEventClone) ASSERT_EQ(callCount, 3); } + +TEST_F(PropertyValueChangedEventsTest, ThrowingWriteHandlerDoesNotPoisonProperty) +{ + const PropertyObjectPtr obj = PropertyObject(); + obj.addProperty(IntProperty("int", 0)); + + bool shouldThrow = true; + obj.getOnAnyPropertyValueWrite() += + [&shouldThrow](const PropertyObjectPtr&, const PropertyValueEventArgsPtr&) + { + if (shouldThrow) + throw GeneralErrorException("Handler failure"); + }; + + ASSERT_ANY_THROW(obj.setPropertyValue("int", 1)); + + // A failed write must not leave a phantom value on the update stack: reads consult + // the stack first, so a leaked entry makes every read return the never-written value + ASSERT_EQ(obj.getPropertyValue("int"), 0); + + // The same value must still be settable afterwards, and for real (visible to clones) + shouldThrow = false; + ASSERT_NO_THROW(obj.setPropertyValue("int", 1)); + ASSERT_EQ(obj.getPropertyValue("int"), 1); + + const PropertyObjectPtr objClone = obj.asPtr().clone(); + ASSERT_EQ(objClone.getPropertyValue("int"), 1); +} From 3029dcbe2578f51bde3b5264c24c1e5301cfe90b Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 21:26:51 +0200 Subject: [PATCH 24/34] Dispatch child-path clears to the child during updates, same as sets clearPropertyValue(child.Prop) during begin/endUpdate was queued on the initiating object and applied only after the child had already ended its own update: the clear ran with isUpdating=false, outside the child's update scope, and the child's endUpdate event never reported it. Sets already dispatch child paths immediately so the operation lands in the updating child's own batch (behavior pinned by ConfigProtocolIntegrationTest.BeginEndUpdateNestedPropertyObjectOrder); clears now do the same. Also clears the ignored error info when endApplyUpdate cannot re-read an applied value on this object. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 13 ++++-- .../tests/test_property_object.cpp | 40 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index c0d9e9c83d..9656d6db90 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -1478,14 +1478,15 @@ ErrCode GenericPropertyObjectImpl::clearPropert const ErrCode errCode = daqTry([&]() { auto propName = StringPtr::Borrow(name); + const auto isChildProp = details::isChildProperty(propName); - if (batch) + if (batch && !isChildProp) { batchedUpdates.emplace_back(std::make_pair(propName, UpdatingAction{false, protectedAccess, nullptr})); return OPENDAQ_SUCCESS; } - if (details::isChildProperty(propName)) + if (isChildProp) { PropertyObjectPtr parentObj; StringPtr leafName; @@ -2148,7 +2149,13 @@ void GenericPropertyObjectImpl::endApplyUpdate( if (err != OPENDAQ_IGNORED) { PropertyPtr prop; - if (OPENDAQ_SUCCEEDED(readPropertyValueInternal(name, false, prop, action.value)) && action.value.assigned()) + const ErrCode refreshErr = readPropertyValueInternal(name, false, prop, action.value); + if (OPENDAQ_FAILED(refreshErr)) + { + // Child-path names cannot be refreshed on this object; keep the queued value + daqClearErrorInfo(); + } + else if (action.value.assigned()) { // TODO: firing read events while applying updates is likely unintended; kept for behavior parity action.value = callPropertyValueRead(prop, action.value); diff --git a/core/coreobjects/tests/test_property_object.cpp b/core/coreobjects/tests/test_property_object.cpp index c2f0f58672..c7101687f9 100644 --- a/core/coreobjects/tests/test_property_object.cpp +++ b/core/coreobjects/tests/test_property_object.cpp @@ -2197,6 +2197,46 @@ TEST_F(PropertyObjectTest, BeginEndUpdateNestedFromPropertyValueWrite) ASSERT_EQ(propObj.getPropertyValue("Property4"), "valuefromprop2"); } +TEST_F(PropertyObjectTest, ChildPropertyClearInChildUpdateScope) +{ + const auto childTemplate = PropertyObject(); + childTemplate.addProperty(StringProperty("Str", "-")); + + auto propObj = PropertyObject(); + propObj.addProperty(ObjectProperty("Child", childTemplate)); + propObj.setPropertyValue("Child.Str", "Value"); + + const PropertyObjectPtr child = propObj.getPropertyValue("Child"); + + bool clearSeen = false; + child.getOnPropertyValueWrite("Str") += [&clearSeen](PropertyObjectPtr&, PropertyValueEventArgsPtr& args) + { + if (args.getPropertyEventType() == PropertyEventType::Clear) + { + clearSeen = true; + // A child-path clear during an update applies within the child's own update scope, + // matching the behavior of child-path sets + ASSERT_TRUE(args.getIsUpdating()); + } + }; + + int childEndUpdateCount = 0; + child.getOnEndUpdate() += [&childEndUpdateCount](PropertyObjectPtr&, EndUpdateEventArgsPtr& args) + { + childEndUpdateCount++; + ASSERT_THAT(args.getProperties(), testing::ElementsAre("Str")); + }; + + propObj.beginUpdate(); + propObj.clearPropertyValue("Child.Str"); + ASSERT_EQ(propObj.getPropertyValue("Child.Str"), "Value"); + propObj.endUpdate(); + + ASSERT_TRUE(clearSeen); + ASSERT_EQ(childEndUpdateCount, 1); + ASSERT_EQ(propObj.getPropertyValue("Child.Str"), "-"); +} + TEST_F(PropertyObjectTest, TestContainerClone) { const auto propObj = PropertyObject(); From 8de161133d99bea9f2105280b4516f86779e7c3a Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 21:35:19 +0200 Subject: [PATCH 25/34] Delegate object-property clears to the nested object clearPropertyValue on an object-type property hand-rolled a per-child loop that cleared every child unconditionally: with a read-only child present the non-protected clear threw ACCESSDENIED, and reference properties were not skipped - while clearPropertyValues on the same nested object politely skips both. Delegate to the nested object''s clearPropertyValues/clearProtectedPropertyValues so there is one clear semantics. Pinned by a test verified to fail against the previous implementation. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 18 +++++++-------- .../tests/test_property_object.cpp | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 9656d6db90..a0e01b68d5 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -1537,17 +1537,17 @@ ErrCode GenericPropertyObjectImpl::clearPropert freezable.assigned() && freezable.isFrozen()) return OPENDAQ_IGNORED; - const auto obj = it->second.template asPtr(true); - PropertyObjectProtectedPtr objProtected; + // Delegate to the nested object so the semantics (skipping read-only and + // reference properties) match clearPropertyValuesInternal if (protectedAccess) - objProtected = it->second.template asPtr(true); - - for (const auto& childProp : obj.getAllProperties()) { - if (protectedAccess) - objProtected.clearProtectedPropertyValue(childProp.getName()); - else - obj.clearPropertyValue(childProp.getName()); + const auto nested = it->second.template asPtr(true); + OPENDAQ_RETURN_IF_FAILED(nested->clearProtectedPropertyValues()); + } + else + { + const auto nested = it->second.template asPtr(true); + OPENDAQ_RETURN_IF_FAILED(nested->clearPropertyValues()); } } } diff --git a/core/coreobjects/tests/test_property_object.cpp b/core/coreobjects/tests/test_property_object.cpp index c7101687f9..02f0115020 100644 --- a/core/coreobjects/tests/test_property_object.cpp +++ b/core/coreobjects/tests/test_property_object.cpp @@ -2237,6 +2237,28 @@ TEST_F(PropertyObjectTest, ChildPropertyClearInChildUpdateScope) ASSERT_EQ(propObj.getPropertyValue("Child.Str"), "-"); } +TEST_F(PropertyObjectTest, ClearObjectPropertyWithReadOnlyChild) +{ + const auto childTemplate = PropertyObject(); + childTemplate.addProperty(IntPropertyBuilder("RO", 1).setReadOnly(true).build()); + childTemplate.addProperty(IntProperty("RW", 1)); + + auto propObj = PropertyObject(); + propObj.addProperty(ObjectProperty("Child", childTemplate)); + + propObj.asPtr().setProtectedPropertyValue("Child.RO", 5); + propObj.setPropertyValue("Child.RW", 5); + + // Read-only children are skipped, matching clearPropertyValues on the nested object + ASSERT_NO_THROW(propObj.clearPropertyValue("Child")); + ASSERT_EQ(propObj.getPropertyValue("Child.RW"), 1); + ASSERT_EQ(propObj.getPropertyValue("Child.RO"), 5); + + // The protected clear still resets read-only children + propObj.asPtr().clearProtectedPropertyValue("Child"); + ASSERT_EQ(propObj.getPropertyValue("Child.RO"), 1); +} + TEST_F(PropertyObjectTest, TestContainerClone) { const auto propObj = PropertyObject(); From 570bd63a63f99c3ee33cae6c40defa97326f095a Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 21:37:33 +0200 Subject: [PATCH 26/34] Extract bindForWrite for the write-path property lookup The lookup + reference-resolution + not-found-error + name-rebind sequence appeared verbatim in setPropertyValueInternal, clearPropertyValueInternal, and setPropertySelectionValueInternal. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index a0e01b68d5..f203d30761 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -407,6 +407,9 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::setPropertyV return OPENDAQ_SUCCESS; } - PropertyPtr prop = getUnboundPropertyOrNull(propName); - prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); - - if (!prop.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", propName)); - - propName = prop.getName(); + PropertyPtr prop; + OPENDAQ_RETURN_IF_FAILED(bindForWrite(propName, prop)); const auto propInternal = prop.asPtr(); // TODO: If function type, check if return value is correct type. @@ -1099,6 +1097,19 @@ void GenericPropertyObjectImpl::triggerCoreEven triggerCoreEvent(args); } +template +ErrCode GenericPropertyObjectImpl::bindForWrite(StringPtr& propName, PropertyPtr& prop) +{ + prop = getUnboundPropertyOrNull(propName); + prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); + + if (!prop.assigned()) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", propName)); + + propName = prop.getName(); + return OPENDAQ_SUCCESS; +} + template ErrCode GenericPropertyObjectImpl::getBoundPropertyInternal(const StringPtr& name, PropertyPtr& property, @@ -1241,11 +1252,9 @@ ErrCode GenericPropertyObjectImpl::setPropertyS } } - PropertyPtr prop = getUnboundPropertyOrNull(propName); - prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); - - if (!prop.assigned()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", propName)); + StringPtr boundName = propName; + PropertyPtr prop; + OPENDAQ_RETURN_IF_FAILED(bindForWrite(boundName, prop)); BaseObjectPtr indexOrKey; OPENDAQ_RETURN_IF_FAILED(details::selectionValueToKey(prop, valuePtr, indexOrKey)); @@ -1505,15 +1514,9 @@ ErrCode GenericPropertyObjectImpl::clearPropert return OPENDAQ_SUCCESS; } - PropertyPtr prop = getUnboundPropertyOrNull(propName); - prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); - - if (!prop.assigned()) - { - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", propName)); - } + PropertyPtr prop; + OPENDAQ_RETURN_IF_FAILED(bindForWrite(propName, prop)); - propName = prop.getName(); const auto propInternal = prop.asPtr(); if (!protectedAccess) From 884a4397053f1f6253f09ff3c842b34e7066f428 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Mon, 10 Aug 2026 21:39:20 +0200 Subject: [PATCH 27/34] Extract the write validation pipeline into details::checkAndCoerceWrite The five type/compatibility checks, the three coercion/validation steps, and the defensive container clone move to the helpers header as one function, leaving only the object-configuration arm (which needs member state) in setPropertyValueInternal. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_helpers.h | 26 +++++++++++++++++++ .../coreobjects/property_object_impl.h | 22 ++-------------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h index 534642e3d7..845afc4d7f 100644 --- a/core/coreobjects/include/coreobjects/property_object_helpers.h +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -585,6 +585,32 @@ inline void coerceMinMax(const PropertyPtr& prop, ObjectPtr& valueP } } +// Validates and converts `value` for writing to `prop`: type conversion and compatibility +// checks, write coercion/validation, min/max clamping, and a defensive clone of container +// values so the caller stores a private copy. +inline ErrCode checkAndCoerceWrite(const PropertyPtr& prop, ObjectPtr& value, const PropertyObjectPtr& objPtr) +{ + OPENDAQ_RETURN_IF_FAILED(checkPropertyTypeAndConvert(prop, value)); + OPENDAQ_RETURN_IF_FAILED(checkContainerType(prop, value)); + OPENDAQ_RETURN_IF_FAILED(checkSelectionValues(prop, value)); + OPENDAQ_RETURN_IF_FAILED(checkStructType(prop, value)); + OPENDAQ_RETURN_IF_FAILED(checkEnumerationType(prop, value)); + + coercePropertyWrite(prop, value, objPtr); + validatePropertyWrite(prop, value, objPtr); + coerceMinMax(prop, value); + + const auto ct = prop.asPtr(true).getValueTypeNoLock(); + if (ct == ctList || ct == ctDict) + { + BaseObjectPtr cloned; + OPENDAQ_RETURN_IF_FAILED(value.asPtr()->clone(&cloned)); + value = cloned.detach(); + } + + return OPENDAQ_SUCCESS; +} + // Reference property handling inline PropertyPtr checkForRefPropAndGetBoundProp(PropertyPtr& prop, const PropertyObjectPtr& objPtr, bool* isReferenced = nullptr) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index f203d30761..8d7d41be20 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -861,28 +861,10 @@ ErrCode GenericPropertyObjectImpl::setPropertyV } } - OPENDAQ_RETURN_IF_FAILED(details::checkPropertyTypeAndConvert(prop, valuePtr)); - OPENDAQ_RETURN_IF_FAILED(details::checkContainerType(prop, valuePtr)); - OPENDAQ_RETURN_IF_FAILED(details::checkSelectionValues(prop, valuePtr)); - OPENDAQ_RETURN_IF_FAILED(details::checkStructType(prop, valuePtr)); - OPENDAQ_RETURN_IF_FAILED(details::checkEnumerationType(prop, valuePtr)); + OPENDAQ_RETURN_IF_FAILED(details::checkAndCoerceWrite(prop, valuePtr, objPtr)); - details::coercePropertyWrite(prop, valuePtr, objPtr); - details::validatePropertyWrite(prop, valuePtr, objPtr); - details::coerceMinMax(prop, valuePtr); - - const auto ct = propInternal.getValueTypeNoLock(); - if (ct == ctList || ct == ctDict) - { - BaseObjectPtr clonedValue; - OPENDAQ_RETURN_IF_FAILED(valuePtr.asPtr()->clone(&clonedValue)); - - valuePtr = clonedValue.detach(); - } - else if (ct == ctObject) - { + if (propInternal.getValueTypeNoLock() == ctObject) configureClonedObj(propName, valuePtr); - } if (triggerEvent) { From 416e3d31641fed81c7bc1ac70c25ff0ee530113b Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Tue, 11 Aug 2026 06:35:26 +0200 Subject: [PATCH 28/34] Clean up property object helpers Behavior-neutral batch: delete the dead splitOnFirstDot (unused since the getParentObject rewrite), express getPropertyNameInfo via getPropNameWithoutIndex instead of a second bracket scan, default PropertyNameInfo::index to the -1 sentinel, flatten checkSelectionValues and coerce/validatePropertyWrite with guard clauses, add a null guard to cloneIfContainerValue, drop unnecessary .template disambiguators, make checkForRefPropAndGetBoundProp take the property by const reference, avoid StringPtr copies in reference loops, unify bool return types, fix the misleading dict error text in selectionKeyToValue, and order the includes. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_helpers.h | 216 ++++++++---------- 1 file changed, 95 insertions(+), 121 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h index 845afc4d7f..0d372128bc 100644 --- a/core/coreobjects/include/coreobjects/property_object_helpers.h +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -23,16 +23,16 @@ #include #include #include -#include #include #include #include #include #include #include -#include #include +#include #include +#include BEGIN_NAMESPACE_OPENDAQ @@ -43,7 +43,7 @@ namespace details struct PropertyNameInfo { StringPtr name; - Int index{}; + Int index = -1; // -1 when the queried name carries no "[index]" suffix }; #if defined(__GNUC__) && __GNUC__ >= 12 @@ -55,21 +55,7 @@ struct PropertyNameInfo inline bool isChildProperty(const StringPtr& name) { - auto chr = strchr(name.getCharPtr(), '.'); - return chr != nullptr; -} - -inline void splitOnFirstDot(const StringPtr& input, StringPtr& head, StringPtr& tail) -{ - const std::string inputStr = input; - head = input; - - size_t pos = inputStr.find('.'); - if (pos == std::string::npos) - return; - - head = inputStr.substr(0, pos); - tail = inputStr.substr(pos + 1); + return strchr(name.getCharPtr(), '.') != nullptr; } inline void splitOnLastDot(const StringPtr& input, StringPtr& head, StringPtr& tail) @@ -104,26 +90,6 @@ inline int parseIndex(char const* lBracket) DAQ_THROW_EXCEPTION(InvalidParameterException, "No matching ] found."); } -inline PropertyNameInfo getPropertyNameInfo(const StringPtr& name) -{ - PropertyNameInfo nameInfo; - - auto propNameData = name.getCharPtr(); - auto first = strchr(propNameData, '['); - if (first != nullptr) - { - nameInfo.index = parseIndex(first); - nameInfo.name = String(propNameData, first - propNameData); - } - else - { - nameInfo.index = -1; - nameInfo.name = name; - } - - return nameInfo; -} - // Gets the property name without the index as the `propName` output parameter // Returns the index in the form of [index], eg. [0] inline ConstCharPtr getPropNameWithoutIndex(const StringPtr& name, StringPtr& propName) @@ -142,6 +108,15 @@ inline ConstCharPtr getPropNameWithoutIndex(const StringPtr& name, StringPtr& pr return first; } +inline PropertyNameInfo getPropertyNameInfo(const StringPtr& name) +{ + PropertyNameInfo nameInfo; + const ConstCharPtr bracket = getPropNameWithoutIndex(name, nameInfo.name); + if (bracket != nullptr) + nameInfo.index = parseIndex(bracket); + return nameInfo; +} + #if defined(__GNUC__) && __GNUC__ >= 12 #pragma GCC diagnostic pop #endif @@ -248,52 +223,51 @@ inline ErrCode checkEnumerationType(const PropertyPtr& prop, const BaseObjectPtr inline ErrCode checkSelectionValues(const PropertyPtr& prop, const BaseObjectPtr& value) { const auto selectionValues = prop.asPtr(true).getSelectionValuesNoLock(); - if (selectionValues.assigned()) + if (!selectionValues.assigned()) + return OPENDAQ_SUCCESS; + + const PropertyType propType = prop.getPropertyType(); + if (propType == PropertyType::IndexSelection) { - const PropertyType propType = prop.getPropertyType(); - if (propType == PropertyType::IndexSelection) + if (const auto list = selectionValues.asPtrOrNull(true); list.assigned()) { - if (const auto list = selectionValues.asPtrOrNull(true); list.assigned()) - { - const SizeT key = value; - if (key < list.getCount()) - return OPENDAQ_SUCCESS; - } + const SizeT key = value; + if (key < list.getCount()) + return OPENDAQ_SUCCESS; } - else if (propType == PropertyType::Selection) + } + else if (propType == PropertyType::Selection) + { + if (const auto list = selectionValues.asPtrOrNull(true); list.assigned()) { - if (const auto list = selectionValues.asPtrOrNull(true); list.assigned()) + if (prop.getValueType() == ctFloat) { - if (prop.getValueType() == ctFloat) + const double valueDouble = value; + const double preScale = std::max(1.0, std::abs(valueDouble)); + for (const double& item : list) { - const double valueDouble = value; - const double preScale = std::max({1.0, std::abs(valueDouble)}); - for (const double& item : list) - { - const double scale = std::max({preScale, std::abs(item)}); - if (std::abs(item - valueDouble) <= std::numeric_limits::epsilon() * scale) - return OPENDAQ_SUCCESS; - } + const double scale = std::max(preScale, std::abs(item)); + if (std::abs(item - valueDouble) <= std::numeric_limits::epsilon() * scale) + return OPENDAQ_SUCCESS; } - else + } + else + { + for (const auto& item : list) { - for (const auto& item : list) - { - if (item == value) - return OPENDAQ_SUCCESS; - } + if (item == value) + return OPENDAQ_SUCCESS; } } } - else if (propType == PropertyType::SparseSelection) - { - if (const auto dict = selectionValues.asPtrOrNull(true); dict.assigned() && dict.hasKey(value)) - return OPENDAQ_SUCCESS; - } - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Value is not a key/index of selection values for property "{}")", prop.getName())); + } + else if (propType == PropertyType::SparseSelection) + { + if (const auto dict = selectionValues.asPtrOrNull(true); dict.assigned() && dict.hasKey(value)) + return OPENDAQ_SUCCESS; } - return OPENDAQ_SUCCESS; + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Value is not a key/index of selection values for property "{}")", prop.getName())); } // Checks if property and value type match. If not, attempts to convert the value @@ -382,6 +356,9 @@ inline ErrCode readDefaultPropertyValue(const PropertyPtr& property, const Strin // Container values are cloned on read so the stored value cannot be mutated through the returned reference inline BaseObjectPtr cloneIfContainerValue(const BaseObjectPtr& value) { + if (!value.assigned()) + return value; + const CoreType coreType = value.getCoreType(); if (coreType == ctList || coreType == ctDict) { @@ -406,7 +383,7 @@ inline ErrCode selectionValueToKey(const PropertyPtr& prop, const BaseObjectPtr& return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Index selection property "{}" has no selection values assigned)", propName)); - const auto valuesList = selectionValues.template asPtrOrNull(true); + const auto valuesList = selectionValues.asPtrOrNull(true); if (!valuesList.assigned()) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Index selection property "{}" values is not a list)", propName)); @@ -429,7 +406,7 @@ inline ErrCode selectionValueToKey(const PropertyPtr& prop, const BaseObjectPtr& return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Sparse selection property "{}" has no selection values assigned)", propName)); - const auto valuesDict = selectionValues.template asPtrOrNull(true); + const auto valuesDict = selectionValues.asPtrOrNull(true); if (!valuesDict.assigned()) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Sparse selection property "{}" values is not a dictionary)", propName)); @@ -499,7 +476,7 @@ inline ErrCode selectionKeyToValue(const PropertyPtr& prop, BaseObjectPtr& value } if (propInternal.getItemTypeNoLock() != valuePtr.getCoreType()) - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(List item type mismatch for property "{}")", propName)); + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, fmt::format(R"(Selection value type mismatch for property "{}")", propName)); return OPENDAQ_SUCCESS; } @@ -508,47 +485,47 @@ inline ErrCode selectionKeyToValue(const PropertyPtr& prop, BaseObjectPtr& value inline void coercePropertyWrite(const PropertyPtr& prop, ObjectPtr& valuePtr, const PropertyObjectPtr& objPtr) { - if (prop.assigned() && valuePtr.assigned()) + if (!prop.assigned() || !valuePtr.assigned()) + return; + + const auto coercer = prop.asPtr().getCoercerNoLock(); + if (!coercer.assigned()) + return; + + try { - const auto coercer = prop.asPtr().getCoercerNoLock(); - if (coercer.assigned()) - { - try - { - valuePtr = coercer.coerceNoLock(objPtr, valuePtr); - } - catch (const DaqException&) - { - throw; - } - catch (...) - { - DAQ_THROW_EXCEPTION(CoerceFailedException); - } - } + valuePtr = coercer.coerceNoLock(objPtr, valuePtr); + } + catch (const DaqException&) + { + throw; + } + catch (...) + { + DAQ_THROW_EXCEPTION(CoerceFailedException); } } inline void validatePropertyWrite(const PropertyPtr& prop, ObjectPtr& valuePtr, const PropertyObjectPtr& objPtr) { - if (prop.assigned() && valuePtr.assigned()) + if (!prop.assigned() || !valuePtr.assigned()) + return; + + const auto validator = prop.asPtr().getValidatorNoLock(); + if (!validator.assigned()) + return; + + try { - const auto validator = prop.asPtr().getValidatorNoLock(); - if (validator.assigned()) - { - try - { - validator.validateNoLock(objPtr, valuePtr); - } - catch (const DaqException&) - { - throw; - } - catch (...) - { - DAQ_THROW_EXCEPTION(ValidateFailedException); - } - } + validator.validateNoLock(objPtr, valuePtr); + } + catch (const DaqException&) + { + throw; + } + catch (...) + { + DAQ_THROW_EXCEPTION(ValidateFailedException); } } @@ -613,7 +590,7 @@ inline ErrCode checkAndCoerceWrite(const PropertyPtr& prop, ObjectPtr().getReferencedPropertyUnresolved(); - if (refEval.assigned()) + const auto refEval = prop.asPtr().getReferencedPropertyUnresolved(); + if (!refEval.assigned()) + return false; + + for (const auto& refPropName : refEval.getPropertyReferences()) { - auto refNames = refEval.getPropertyReferences(); - for (auto refPropName : refNames) - { - if (objPtr.hasProperty(refPropName) && objPtr.getProperty(refPropName).getIsReferenced()) - return true; - } + if (objPtr.hasProperty(refPropName) && objPtr.getProperty(refPropName).getIsReferenced()) + return true; } return false; } -inline Bool checkIsReferenced(const StringPtr& referencedPropName, const PropertyInternalPtr& prop) +inline bool checkIsReferenced(const StringPtr& referencedPropName, const PropertyInternalPtr& prop) { const auto refProp = prop.getReferencedPropertyUnresolved(); if (!refProp.assigned()) return false; - for (auto propName : refProp.getPropertyReferences()) + for (const auto& propName : refProp.getPropertyReferences()) { if (propName == referencedPropName) - { return true; - } } return false; From a4523a6f1acc59ed4fc5b2238228305d373ea389 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Tue, 11 Aug 2026 08:10:25 +0200 Subject: [PATCH 29/34] Bind selection-set properties once Extract the post-bind commit tail of setPropertyValueInternal into writeBoundPropertyValue and call it directly from the selection-set path with the property it already bound for key conversion, instead of re-entering setPropertyValueInternal and binding a second time. Frozen-check and batch-queue ordering are preserved. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 100 +++++++++++------- 1 file changed, 61 insertions(+), 39 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 8d7d41be20..ee2a38f05a 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -339,6 +339,7 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::fireProperty if (!emitter.hasListeners()) return; + // Does it make sense to only protect the write events? if (!valueWrite) { emitter(objPtr, args); @@ -851,51 +853,62 @@ ErrCode GenericPropertyObjectImpl::setPropertyV PropertyPtr prop; OPENDAQ_RETURN_IF_FAILED(bindForWrite(propName, prop)); - const auto propInternal = prop.asPtr(); - // TODO: If function type, check if return value is correct type. - if (!protectedAccess) + return writeBoundPropertyValue(prop, propName, valuePtr, triggerEvent, protectedAccess, isUpdating); + }); + + OPENDAQ_RETURN_IF_FAILED(errCode, fmt::format(R"(Failed to set property value "{}")", propName)); + return errCode; +} + +template +ErrCode GenericPropertyObjectImpl::writeBoundPropertyValue(const PropertyPtr& prop, + const StringPtr& propName, + BaseObjectPtr& valuePtr, + bool triggerEvent, + bool protectedAccess, + bool isUpdating) +{ + const auto propInternal = prop.asPtr(); + // TODO: If function type, check if return value is correct type. + if (!protectedAccess) + { + if (propInternal.getReadOnlyNoLock() || propInternal.getValueTypeNoLock() == ctObject) { - if (propInternal.getReadOnlyNoLock() || propInternal.getValueTypeNoLock() == ctObject) - { - return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_ACCESSDENIED, fmt::format(R"(Property "{}" is read only)", propName)); - } + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_ACCESSDENIED, fmt::format(R"(Property "{}" is read only)", propName)); } + } - OPENDAQ_RETURN_IF_FAILED(details::checkAndCoerceWrite(prop, valuePtr, objPtr)); - - if (propInternal.getValueTypeNoLock() == ctObject) - configureClonedObj(propName, valuePtr); + OPENDAQ_RETURN_IF_FAILED(details::checkAndCoerceWrite(prop, valuePtr, objPtr)); - if (triggerEvent) - { - BaseObjectPtr newValue = valuePtr; - ErrCode err = callPropertyValueWrite(prop, newValue, PropertyEventType::Update, isUpdating); - OPENDAQ_RETURN_IF_FAILED(err); + if (propInternal.getValueTypeNoLock() == ctObject) + configureClonedObj(propName, valuePtr); - if (err == OPENDAQ_IGNORED) - return OPENDAQ_SUCCESS; + if (triggerEvent) + { + BaseObjectPtr newValue = valuePtr; + ErrCode err = callPropertyValueWrite(prop, newValue, PropertyEventType::Update, isUpdating); + OPENDAQ_RETURN_IF_FAILED(err); - if (valuePtr == newValue) - { - writeLocalValue(propName, newValue); - setOwnerToPropertyValue(newValue); - } + if (err == OPENDAQ_IGNORED) + return OPENDAQ_SUCCESS; - if (!isUpdating) - triggerCoreEventInternal(CoreEventArgsPropertyValueChanged(objPtr, propName, newValue, path)); - } - else + if (valuePtr == newValue) { - if (!writeLocalValue(propName, valuePtr)) - return OPENDAQ_IGNORED; - setOwnerToPropertyValue(valuePtr); + writeLocalValue(propName, newValue); + setOwnerToPropertyValue(newValue); } - return OPENDAQ_SUCCESS; - }); + if (!isUpdating) + triggerCoreEventInternal(CoreEventArgsPropertyValueChanged(objPtr, propName, newValue, path)); + } + else + { + if (!writeLocalValue(propName, valuePtr)) + return OPENDAQ_IGNORED; + setOwnerToPropertyValue(valuePtr); + } - OPENDAQ_RETURN_IF_FAILED(errCode, fmt::format(R"(Failed to set property value "{}")", propName)); - return errCode; + return OPENDAQ_SUCCESS; } @@ -1241,7 +1254,16 @@ ErrCode GenericPropertyObjectImpl::setPropertyS BaseObjectPtr indexOrKey; OPENDAQ_RETURN_IF_FAILED(details::selectionValueToKey(prop, valuePtr, indexOrKey)); - return setPropertyValueInternal(propertyName, indexOrKey, true, protectedAccess, updateCount > 0); + if (frozen) + return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_FROZEN); + + if (updateCount > 0) + { + batchedUpdates.emplace_back(std::make_pair(propName, UpdatingAction{true, protectedAccess, indexOrKey})); + return OPENDAQ_SUCCESS; + } + + return writeBoundPropertyValue(prop, boundName, indexOrKey, true, protectedAccess, false); }); OPENDAQ_RETURN_IF_FAILED(errCode, "Failed to set property selection value"); return errCode; @@ -1616,14 +1638,14 @@ ErrCode GenericPropertyObjectImpl::getPropertyS if (details::isChildProperty(propName)) { - const ErrCode errCode = getProperty(propName, &prop); - OPENDAQ_RETURN_IF_FAILED(errCode, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); + const ErrCode err = getProperty(propName, &prop); + OPENDAQ_RETURN_IF_FAILED(err, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); valuePtr = prop.getValue(); } else { - const ErrCode errCode = readPropertyValueInternal(propName, retrieveUpdatingValue, prop, valuePtr); - OPENDAQ_RETURN_IF_FAILED(errCode, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); + const ErrCode err = readPropertyValueInternal(propName, retrieveUpdatingValue, prop, valuePtr); + OPENDAQ_RETURN_IF_FAILED(err, OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Selection property "{}" not found)", propName)); if (valuePtr.assigned()) valuePtr = callPropertyValueRead(prop, valuePtr); } From e46b6f851b6b18bf54670a520f16b4d61ba09c03 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Tue, 11 Aug 2026 08:13:13 +0200 Subject: [PATCH 30/34] Decompose getPropertiesInternal into named steps Split the 92-line function into collectAllProperties (class + local property gathering), bindAndFilterProperties (owner binding and visibility/reference filtering), and applyCustomPropertyOrder (customOrder-first ordering). getPropertiesInternal is now a four-line orchestrator. No behavior change; findProperties already delegated here and needed no edits. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index ee2a38f05a..16f7089f29 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -417,6 +417,12 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak collectAllProperties(Bool includeCoreProperties) const; + // Binds the collected properties to this object and drops invisible/referenced ones unless requested + ErrCode bindAndFilterProperties(const std::vector& allProperties, Bool includeInvisible, Bool bind, PropertyOrderedMap& lookup); + // Consumes `lookup`: properties named in customOrder first, then the rest in default order + ListPtr applyCustomPropertyOrder(PropertyOrderedMap& lookup) const; // Gets the property value, if stored in local value dictionary (propValues) // Parses brackets, if the property is a list @@ -1869,6 +1875,18 @@ ErrCode GenericPropertyObjectImpl::getPropertie if (!includeInvisible && !bind) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPARAMETER); + const std::vector allProperties = collectAllProperties(includeCoreProperties); + + PropertyOrderedMap lookup; + OPENDAQ_RETURN_IF_FAILED(bindAndFilterProperties(allProperties, includeInvisible, bind, lookup)); + + *list = applyCustomPropertyOrder(lookup).detach(); + return OPENDAQ_SUCCESS; +} + +template +std::vector GenericPropertyObjectImpl::collectAllProperties(Bool includeCoreProperties) const +{ std::vector allProperties; if (objectClass.assigned()) { @@ -1888,8 +1906,16 @@ ErrCode GenericPropertyObjectImpl::getPropertie allProperties.push_back(prop); } - PropertyOrderedMap lookup; - for (auto& prop : allProperties) + return allProperties; +} + +template +ErrCode GenericPropertyObjectImpl::bindAndFilterProperties(const std::vector& allProperties, + Bool includeInvisible, + Bool bind, + PropertyOrderedMap& lookup) +{ + for (const auto& prop : allProperties) { if (!bind) { @@ -1930,9 +1956,14 @@ ErrCode GenericPropertyObjectImpl::getPropertie } } + return OPENDAQ_SUCCESS; +} + +template +ListPtr GenericPropertyObjectImpl::applyCustomPropertyOrder(PropertyOrderedMap& lookup) const +{ auto properties = List(); - // Add properties with explicit order first, then the rest in default order for (const auto& propName : customOrder) { const auto iter = lookup.find(propName); @@ -1948,8 +1979,7 @@ ErrCode GenericPropertyObjectImpl::getPropertie properties.unsafePushBack(prop.second); } - *list = properties.detach(); - return OPENDAQ_SUCCESS; + return properties; } template From 0f8010baf75743999573717457fc7079c165261e Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Tue, 11 Aug 2026 08:16:15 +0200 Subject: [PATCH 31/34] Deduplicate hasDuplicateReferences PropertyObjectClassImpl::hasDuplicateReferences was dead code - declared and defined but never called; removed. The builder now delegates to a shared details::hasDuplicateReferences overload in property_object_helpers.h that takes the property list to scan, replacing its member copy of the same set-intersection algorithm. The existing object-level overload (getIsReferenced-based) is a different check and stays as is. Co-Authored-By: Claude Fable 5 --- .../property_object_class_builder_impl.h | 1 - .../coreobjects/property_object_class_impl.h | 2 -- .../coreobjects/property_object_helpers.h | 27 +++++++++++++++++ .../property_object_class_builder_impl.cpp | 29 ++----------------- .../src/property_object_class_impl.cpp | 27 ----------------- 5 files changed, 29 insertions(+), 57 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_class_builder_impl.h b/core/coreobjects/include/coreobjects/property_object_class_builder_impl.h index dc2fa8fc14..6a4f98aad2 100644 --- a/core/coreobjects/include/coreobjects/property_object_class_builder_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_class_builder_impl.h @@ -54,7 +54,6 @@ class PropertyObjectClassBuilderImpl : public ImplementationOf customOrder; WeakRefPtr manager; - bool hasDuplicateReferences(const PropertyPtr& prop) const; ListPtr getProperties() const; }; diff --git a/core/coreobjects/include/coreobjects/property_object_class_impl.h b/core/coreobjects/include/coreobjects/property_object_class_impl.h index d3009220cd..8d03bf0437 100644 --- a/core/coreobjects/include/coreobjects/property_object_class_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_class_impl.h @@ -68,8 +68,6 @@ class PropertyObjectClassImpl : public ImplementationOf& properties) const; ErrCode serializeProperties(ISerializer* serializer); - - bool hasDuplicateReferences(const PropertyPtr& prop); }; diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h index 0d372128bc..b8b188d89c 100644 --- a/core/coreobjects/include/coreobjects/property_object_helpers.h +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -33,6 +33,7 @@ #include #include #include +#include BEGIN_NAMESPACE_OPENDAQ @@ -633,6 +634,32 @@ inline bool hasDuplicateReferences(const PropertyPtr& prop, const PropertyObject return false; } +// Checks whether the property's reference targets overlap with those of any property in the list +inline bool hasDuplicateReferences(const PropertyPtr& prop, const ListPtr& properties) +{ + const auto refEval = prop.asPtr().getReferencedPropertyUnresolved(); + if (!refEval.assigned()) + return false; + + std::unordered_set refNamesSet; + for (const auto& refName : refEval.getPropertyReferences()) + refNamesSet.insert(refName); + + for (const auto& ownProp : properties) + { + if (const auto refEvalOwn = ownProp.asPtr().getReferencedPropertyUnresolved(); refEvalOwn.assigned()) + { + for (const auto& refPropName : refEvalOwn.getPropertyReferences()) + { + if (refNamesSet.count(refPropName)) + return true; + } + } + } + + return false; +} + inline bool checkIsReferenced(const StringPtr& referencedPropName, const PropertyInternalPtr& prop) { const auto refProp = prop.getReferencedPropertyUnresolved(); diff --git a/core/coreobjects/src/property_object_class_builder_impl.cpp b/core/coreobjects/src/property_object_class_builder_impl.cpp index c08beb83d3..aa19c2ebbd 100644 --- a/core/coreobjects/src/property_object_class_builder_impl.cpp +++ b/core/coreobjects/src/property_object_class_builder_impl.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -72,7 +73,7 @@ ErrCode PropertyObjectClassBuilderImpl::addProperty(IProperty* property) { auto p = PropertyPtr::Borrow(property); - if (hasDuplicateReferences(p)) + if (details::hasDuplicateReferences(p, getProperties())) return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDVALUE, "Reference property references a property that is already referenced by another."); if (props.hasKey(p.getName())) @@ -151,32 +152,6 @@ ErrCode PropertyObjectClassBuilderImpl::getManager(ITypeManager** manager) return OPENDAQ_SUCCESS; } -bool PropertyObjectClassBuilderImpl::hasDuplicateReferences(const PropertyPtr& prop) const -{ - if (const auto refEval = prop.asPtr().getReferencedPropertyUnresolved(); refEval.assigned()) - { - const auto refNames = refEval.getPropertyReferences(); - std::unordered_set refNamesSet; - for (auto refName : refNames) - refNamesSet.insert(refName); - - for (auto ownProp : getProperties()) - { - if (auto refEvalOwn = ownProp.asPtr().getReferencedPropertyUnresolved(); refEvalOwn.assigned()) - { - auto refNamesOwn = refEvalOwn.getPropertyReferences(); - for (auto refPropName : refNamesOwn) - { - if (refNamesSet.count(refPropName)) - return true; - } - } - } - } - - return false; -} - ListPtr PropertyObjectClassBuilderImpl::getProperties() const { ListPtr properties = List(); diff --git a/core/coreobjects/src/property_object_class_impl.cpp b/core/coreobjects/src/property_object_class_impl.cpp index fc1a768db7..47343fdd47 100644 --- a/core/coreobjects/src/property_object_class_impl.cpp +++ b/core/coreobjects/src/property_object_class_impl.cpp @@ -266,33 +266,6 @@ ErrCode PropertyObjectClassImpl::serializeProperties(ISerializer* serializer) return OPENDAQ_SUCCESS; } -bool PropertyObjectClassImpl::hasDuplicateReferences(const PropertyPtr& prop) -{ - if (const auto refEval = prop.asPtr().getReferencedPropertyUnresolved(); refEval.assigned()) - { - const auto refNames = refEval.getPropertyReferences(); - std::unordered_set refNamesSet; - for (auto refName : refNames) - refNamesSet.insert(refName); - - const auto thisPtr = this->borrowPtr(); - for (auto ownProp : thisPtr.getProperties(true)) - { - if (auto refEvalOwn = ownProp.asPtr().getReferencedPropertyUnresolved(); refEvalOwn.assigned()) - { - auto refNamesOwn = refEvalOwn.getPropertyReferences(); - for (auto refPropName : refNamesOwn) - { - if (refNamesSet.count(refPropName)) - return true; - } - } - } - } - - return false; -} - ErrCode PropertyObjectClassImpl::serialize(ISerializer* serializer) { serializer->startTaggedObject(this); From bcd6d591f4cbbe1587ea0a4ec75101440e7da3e7 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Tue, 11 Aug 2026 08:18:09 +0200 Subject: [PATCH 32/34] Tighten legacy lock accessors Define the legacy generation in terms of its counterparts: getRecursiveConfigLock now wraps getRecursiveConfigLock2 instead of repeating the getRecursiveLockGuard/checkErrorInfo sequence, and the std-mutex accessors are single-expression. A comment marks the non-2 set as the legacy generation pending its cross-repo retirement. Co-Authored-By: Claude Fable 5 --- .../include/coreobjects/property_object_impl.h | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 16f7089f29..d6217edac3 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -1358,26 +1358,24 @@ void GenericPropertyObjectImpl::configureCloned } } +// The non-"2" lock accessors are the legacy generation, kept for derived classes still on std lock types; +// each is defined in terms of its "2" counterpart or the shared local mutex. template std::unique_ptr GenericPropertyObjectImpl::getRecursiveConfigLock() { - LockGuardPtr lockGuard; - checkErrorInfo(getRecursiveLockGuard(&lockGuard)); - return std::make_unique(lockGuard); + return std::make_unique(getRecursiveConfigLock2()); } template std::lock_guard GenericPropertyObjectImpl::getAcquisitionLock() { - std::mutex* mutexPtr = getLocalMutex(); - return std::lock_guard(*mutexPtr); + return std::lock_guard(*getLocalMutex()); } template std::unique_lock GenericPropertyObjectImpl::getUniqueLock() { - std::mutex* mutexPtr = getLocalMutex(); - return std::unique_lock(*mutexPtr); + return std::unique_lock(*getLocalMutex()); } template From 584228ab2279a435bacb96dacd513e07c5642db0 Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Tue, 11 Aug 2026 08:52:05 +0200 Subject: [PATCH 33/34] Standardize brace usage in property object headers Applied to property_object_impl.h and property_object_helpers.h: - Single-statement if/else bodies have no braces, regardless of content - plain calls, returns of openDAQ error macros (return DAQ_MAKE_ERROR_INFO(...)), DAQ_THROW_EXCEPTION, and OPENDAQ_RETURN_IF_FAILED alike (all macros involved are hygienic: expression, single throw, or do-while(0)). - Statements wrapped across rows for length gain braces. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_helpers.h | 30 ++++---- .../coreobjects/property_object_impl.h | 69 +++---------------- 2 files changed, 22 insertions(+), 77 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_helpers.h b/core/coreobjects/include/coreobjects/property_object_helpers.h index b8b188d89c..0c175e537b 100644 --- a/core/coreobjects/include/coreobjects/property_object_helpers.h +++ b/core/coreobjects/include/coreobjects/property_object_helpers.h @@ -82,9 +82,7 @@ inline int parseIndex(char const* lBracket) int index = strtol(lBracket + 1, &end, 10); if (end != last) - { DAQ_THROW_EXCEPTION(InvalidParameterException, "Could not parse the property index."); - } return index; } @@ -99,13 +97,9 @@ inline ConstCharPtr getPropNameWithoutIndex(const StringPtr& name, StringPtr& pr auto first = strchr(propNameData, '['); if (first == nullptr) - { propName = String(propNameData); - } else - { propName = String(propNameData, first - propNameData); - } return first; } @@ -133,9 +127,7 @@ inline ErrCode checkContainerType(const PropertyPtr& prop, const BaseObjectPtr& { auto inspect = value.asPtrOrNull(true); if (inspect.assigned() && !inspect.getInterfaceIds().empty()) - { return inspect.getInterfaceIds()[0] == IPropertyObject::Id; - } return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDTYPE, "Only base Property Object object-type values are allowed"); } @@ -292,8 +284,10 @@ inline ErrCode checkPropertyTypeAndConvert(const PropertyPtr& prop, BaseObjectPt { const auto enumVal = propInternal.getDefaultValueNoLock().asPtrOrNull(); if (!enumVal.assigned()) + { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDSTATE, fmt::format(R"(Default value of enumeration property {} is not assigned)", prop.getName())); + } const auto type = enumVal.getEnumerationType(); const Int intVal = value.convertTo(ctInt); @@ -345,9 +339,7 @@ inline ErrCode readDefaultPropertyValue(const PropertyPtr& property, const Strin const int index = parseIndex(bracket); ListPtr list = value; if (index >= static_cast(list.getCount())) - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_OUTOFRANGE, fmt::format(R"(The index parameter is out of bounds of the list for property "{}")", propName)); - } value = list[std::size_t(index)]; } @@ -381,13 +373,17 @@ inline ErrCode selectionValueToKey(const PropertyPtr& prop, const BaseObjectPtr& if (propType == PropertyType::IndexSelection) { if (!selectionValues.assigned()) + { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Index selection property "{}" has no selection values assigned)", propName)); + } const auto valuesList = selectionValues.asPtrOrNull(true); if (!valuesList.assigned()) + { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Index selection property "{}" values is not a list)", propName)); + } for (SizeT i = 0; i < valuesList.getCount(); ++i) { @@ -404,13 +400,17 @@ inline ErrCode selectionValueToKey(const PropertyPtr& prop, const BaseObjectPtr& else if (propType == PropertyType::SparseSelection) { if (!selectionValues.assigned()) + { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Sparse selection property "{}" has no selection values assigned)", propName)); + } const auto valuesDict = selectionValues.asPtrOrNull(true); if (!valuesDict.assigned()) + { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Sparse selection property "{}" values is not a dictionary)", propName)); + } for (const auto& [key, value] : valuesDict) { @@ -425,13 +425,9 @@ inline ErrCode selectionValueToKey(const PropertyPtr& prop, const BaseObjectPtr& return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Value not found in sparse selection values of property "{}")", propName)); } else if (propType == PropertyType::Selection) - { indexOrKey = valuePtr; - } else - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Property "{}" is not an index selection or sparse selection property)", propName)); - } return OPENDAQ_SUCCESS; } @@ -451,16 +447,20 @@ inline ErrCode selectionKeyToValue(const PropertyPtr& prop, BaseObjectPtr& value { const auto valuesList = values.asPtrOrNull(true); if (!valuesList.assigned()) + { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Index selection property "{}" values is not a list)", propName)); + } valuePtr = valuesList.getItemAt(valuePtr); } else if (propType == PropertyType::SparseSelection) { const auto valuesDict = values.asPtrOrNull(true); if (!valuesDict.assigned()) + { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPROPERTY, fmt::format(R"(Sparse selection property "{}" values is not a dictionary)", propName)); + } valuePtr = valuesDict.get(valuePtr); } else if (propType == PropertyType::Selection) @@ -594,9 +594,7 @@ inline ErrCode checkAndCoerceWrite(const PropertyPtr& prop, ObjectPtr(true).cloneWithOwner(objPtr); auto refProp = boundProp.getReferencedPropertyNoLock(); diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index d6217edac3..3c175f66f1 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -526,9 +526,7 @@ GenericPropertyObjectImpl::GenericPropertyObjec for (const auto& prop : objectClass.getProperties(true)) { if (checkIsChildObjectProperty(prop)) - { setChildPropertyObject(prop.getName(), cloneChildPropertyObject(prop)); - } } } } @@ -610,13 +608,9 @@ ErrCode GenericPropertyObjectImpl::getClassName OPENDAQ_PARAM_NOT_NULL(className); if (this->className.assigned()) - { *className = this->className.addRefAndReturn(); - } else - { *className = String("").detach(); - } return OPENDAQ_SUCCESS; } @@ -669,8 +663,7 @@ ErrCode GenericPropertyObjectImpl::callProperty if (!updatePropertyStack.registerPropertyUpdating(name, newValue)) return OPENDAQ_IGNORED; - const bool isBaseStackLevel = updatePropertyStack.isBaseStackLevel(name); - if (isBaseStackLevel) + if (updatePropertyStack.isBaseStackLevel(name)) { if (newValue.assigned() && !shouldWriteLocalValue(name, newValue)) { @@ -689,11 +682,9 @@ ErrCode GenericPropertyObjectImpl::callProperty daqClearErrorInfo(); oldValue = defaultValue; } - PropertyValueEventArgsPtr args; - if (changeType == PropertyEventType::Clear) - args = PropertyValueEventArgs(prop, defaultValue, oldValue, changeType, isUpdating); - else - args = PropertyValueEventArgs(prop, newValue, oldValue, changeType, isUpdating); + + auto chosenValue = changeType == PropertyEventType::Clear ? defaultValue : newValue; + PropertyValueEventArgsPtr args = PropertyValueEventArgs(prop, chosenValue, oldValue, changeType, isUpdating); errCode = firePropertyValueEvents(prop, args, true); @@ -775,9 +766,7 @@ BaseObjectPtr GenericPropertyObjectImpl::callPr const BaseObjectPtr& readValue) { if (!prop.assigned()) - { return readValue; - } auto args = PropertyValueEventArgs(prop, readValue, readValue, PropertyEventType::Read, False); firePropertyValueEvents(prop, args, false); @@ -849,9 +838,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyV parentObjProtected.setProtectedPropertyValue(leafName, valuePtr); } else - { parentObj.setPropertyValue(leafName, valuePtr); - } return OPENDAQ_SUCCESS; } @@ -879,9 +866,7 @@ ErrCode GenericPropertyObjectImpl::writeBoundPr if (!protectedAccess) { if (propInternal.getReadOnlyNoLock() || propInternal.getValueTypeNoLock() == ctObject) - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_ACCESSDENIED, fmt::format(R"(Property "{}" is read only)", propName)); - } } OPENDAQ_RETURN_IF_FAILED(details::checkAndCoerceWrite(prop, valuePtr, objPtr)); @@ -1065,21 +1050,15 @@ ErrCode GenericPropertyObjectImpl::readLocalVal if (info.index != -1) { if (it->second.getCoreType() != ctList) - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDPARAMETER, fmt::format(R"(Could not access the index as the value is not a list for property "{}")", name)); - } ListPtr list = it->second; if (info.index >= (int) list.getCount()) - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_OUTOFRANGE, fmt::format(R"(The index parameter is out of bounds of the list for property "{}")", name)); - } value = list[std::size_t(info.index)]; } else - { value = it->second; - } return OPENDAQ_SUCCESS; } @@ -1123,9 +1102,7 @@ ErrCode GenericPropertyObjectImpl::getBoundProp property = getUnboundPropertyOrNull(propName); if (!property.assigned()) - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", propName)); - } bool isRef; property = details::checkForRefPropAndGetBoundProp(property, objPtr, &isRef); @@ -1148,9 +1125,7 @@ ErrCode GenericPropertyObjectImpl::readProperty value = property.getDefaultValue(); } else - { res = readLocalValue(resolvedName, value); - } OPENDAQ_RETURN_IF_FAILED_EXCEPT(res, OPENDAQ_ERR_NOTFOUND); if (res == OPENDAQ_ERR_NOTFOUND) @@ -1248,9 +1223,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyS return parentObjProtected->setProtectedPropertySelectionValue(leafName, value); } else - { return parentObj->setPropertySelectionValue(leafName, value); - } } StringPtr boundName = propName; @@ -1352,9 +1325,7 @@ void GenericPropertyObjectImpl::configureCloned } } else - { this->propValues.insert(val); - } } } @@ -1515,9 +1486,7 @@ ErrCode GenericPropertyObjectImpl::clearPropert parentObjProtected.clearProtectedPropertyValue(leafName); } else - { parentObj.clearPropertyValue(leafName); - } return OPENDAQ_SUCCESS; } @@ -1530,9 +1499,7 @@ ErrCode GenericPropertyObjectImpl::clearPropert if (!protectedAccess) { if (propInternal.getReadOnlyNoLock()) - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_ACCESSDENIED, fmt::format(R"(Property "{}" is read only)", propName)); - } } { @@ -1692,9 +1659,7 @@ ErrCode GenericPropertyObjectImpl::getProperty( } if (const auto freezable = prop.template asPtrOrNull(true); freezable.assigned()) - { OPENDAQ_RETURN_IF_FAILED(freezable->freeze()); - } *property = prop.detach(); return OPENDAQ_SUCCESS; @@ -1723,8 +1688,10 @@ ErrCode GenericPropertyObjectImpl::addPropertyI return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDVALUE, fmt::format(R"(Property "{}" does not have an assigned name.)", propName)); if (details::hasDuplicateReferences(propPtr, objPtr)) + { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALIDVALUE, fmt::format(R"(Reference property "{}" references a property that is already referenced by another.)", propName)); + } propPtr.asPtr(true).setOwner(objPtr); @@ -1800,23 +1767,17 @@ ErrCode GenericPropertyObjectImpl::removeProper OPENDAQ_PARAM_NOT_NULL(propertyName); if (frozen) - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_FROZEN); - } auto lock = getRecursiveConfigLock2(); auto namePtr = StringPtr::Borrow(propertyName); if (localProperties.find(propertyName) == localProperties.cend()) - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", namePtr)); - } localProperties.erase(propertyName); if (propValues.find(propertyName) != propValues.cend()) - { propValues.erase(propertyName); - } if (auto it = corePropertyNames.find(namePtr); it != corePropertyNames.end()) corePropertyNames.erase(it); @@ -1894,9 +1855,7 @@ std::vector GenericPropertyObjectImpl::bindAndFilte auto boundProp = prop.asPtr(true).cloneWithOwner(objPtr); if (!includeInvisible && boundProp.getIsReferenced()) - { continue; - } try { if (!includeInvisible && !boundProp.getVisible()) - { continue; - } auto freezable = boundProp.template asPtrOrNull(true); if (freezable.assigned()) - { freezable.freeze(); - } lookup.insert_or_assign(boundProp.getName(), boundProp); } @@ -2004,10 +1957,12 @@ ErrCode GenericPropertyObjectImpl::getPropertyV return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_NOTFOUND, fmt::format(R"(Property "{}" does not exist)", name)); if (prop.getReferencedPropertyUnresolved().assigned()) + { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_INVALID_OPERATION, fmt::format(R"({} is not allowed for the reference properties "{}")", valueWrite ? "getOnPropertyValueWrite" : "getOnPropertyValueRead", name)); + } auto& events = valueWrite ? valueWriteEvents : valueReadEvents; auto [it, _] = events.try_emplace(name); @@ -2145,9 +2100,7 @@ template std::mutex* GenericPropertyObjectImpl::getLocalMutex() { if (this->lockingStrategy == LockingStrategy::InheritLock) - { DAQ_THROW_EXCEPTION(daq::InvalidStateException, "Can't acquire local mutex if locking strategy is set to inherit"); - } MutexImpl* mutexImpl = dynamic_cast(this->sync.getObject()); return &mutexImpl->mutex; @@ -2806,9 +2759,7 @@ void GenericPropertyObjectImpl::DeserializeLoca const auto propName = prop.getName(); if (!propObjPtr.hasProperty(propName)) - { propObjPtr.addProperty(prop); - } } } @@ -2823,9 +2774,7 @@ template ErrCode GenericPropertyObjectImpl::toString(CharPtr* str) { if (str == nullptr) - { return DAQ_MAKE_ERROR_INFO(OPENDAQ_ERR_ARGUMENT_NULL, "Parameter must not be null"); - } std::ostringstream stream; stream << "PropertyObject"; @@ -2948,9 +2897,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyF const TypeManagerPtr& typeManager) { if (!serialized.assigned()) - { return propObj->clearPropertyValue(propName); - } BaseObjectPtr propValue; From 22ecfe6c35bdb1b4e309f53dad3ecb218ac7876e Mon Sep 17 00:00:00 2001 From: Jaka Mohorko Date: Tue, 11 Aug 2026 09:02:30 +0200 Subject: [PATCH 34/34] Rename write-path helpers for clarity bindForWrite -> bindProperty: the old suffix only encoded the absence of bracket parsing, which the name could not convey; a contract comment now covers that difference from getBoundPropertyInternal. writeBoundPropertyValue -> checkAndSetPropertyValue: the function is the full commit stage of a set (access check, coercion/validation, object clone configuration, event-firing commit or silent write), which plain write undersold. Co-Authored-By: Claude Fable 5 --- .../coreobjects/property_object_impl.h | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/core/coreobjects/include/coreobjects/property_object_impl.h b/core/coreobjects/include/coreobjects/property_object_impl.h index 3c175f66f1..d9a878fdbc 100644 --- a/core/coreobjects/include/coreobjects/property_object_impl.h +++ b/core/coreobjects/include/coreobjects/property_object_impl.h @@ -339,7 +339,9 @@ class GenericPropertyObjectImpl : public ImplementationOfWeak::setPropertyV } PropertyPtr prop; - OPENDAQ_RETURN_IF_FAILED(bindForWrite(propName, prop)); + OPENDAQ_RETURN_IF_FAILED(bindProperty(propName, prop)); - return writeBoundPropertyValue(prop, propName, valuePtr, triggerEvent, protectedAccess, isUpdating); + return checkAndSetPropertyValue(prop, propName, valuePtr, triggerEvent, protectedAccess, isUpdating); }); OPENDAQ_RETURN_IF_FAILED(errCode, fmt::format(R"(Failed to set property value "{}")", propName)); @@ -854,7 +856,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyV } template -ErrCode GenericPropertyObjectImpl::writeBoundPropertyValue(const PropertyPtr& prop, +ErrCode GenericPropertyObjectImpl::checkAndSetPropertyValue(const PropertyPtr& prop, const StringPtr& propName, BaseObjectPtr& valuePtr, bool triggerEvent, @@ -1078,7 +1080,7 @@ void GenericPropertyObjectImpl::triggerCoreEven } template -ErrCode GenericPropertyObjectImpl::bindForWrite(StringPtr& propName, PropertyPtr& prop) +ErrCode GenericPropertyObjectImpl::bindProperty(StringPtr& propName, PropertyPtr& prop) { prop = getUnboundPropertyOrNull(propName); prop = details::checkForRefPropAndGetBoundProp(prop, objPtr); @@ -1228,7 +1230,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyS StringPtr boundName = propName; PropertyPtr prop; - OPENDAQ_RETURN_IF_FAILED(bindForWrite(boundName, prop)); + OPENDAQ_RETURN_IF_FAILED(bindProperty(boundName, prop)); BaseObjectPtr indexOrKey; OPENDAQ_RETURN_IF_FAILED(details::selectionValueToKey(prop, valuePtr, indexOrKey)); @@ -1242,7 +1244,7 @@ ErrCode GenericPropertyObjectImpl::setPropertyS return OPENDAQ_SUCCESS; } - return writeBoundPropertyValue(prop, boundName, indexOrKey, true, protectedAccess, false); + return checkAndSetPropertyValue(prop, boundName, indexOrKey, true, protectedAccess, false); }); OPENDAQ_RETURN_IF_FAILED(errCode, "Failed to set property selection value"); return errCode; @@ -1492,7 +1494,7 @@ ErrCode GenericPropertyObjectImpl::clearPropert } PropertyPtr prop; - OPENDAQ_RETURN_IF_FAILED(bindForWrite(propName, prop)); + OPENDAQ_RETURN_IF_FAILED(bindProperty(propName, prop)); const auto propInternal = prop.asPtr();