Repository: OpenAssetIO/OpenAssetIO (language: python)

Issue:
Use `optional<EntityReference>` in `defaultEntityReference`
## What

Change returns type of `defaultEntityReference` to optional

## Why

Because having no data is not an error state

## ACs

- Change the `DefaultEntityReferenceSuccessCallback` to take `std::optional<EntityReference>`.
- Update documentation to not instruct the use of `kEntityAccessError` when one isn't available, and to supply a null optional instead.


Discussion / hints:

Fix patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index cd3e8c4ae..9e63488b5 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -33,6 +33,13 @@ v1.0.0-alpha.xx
 - Removed python `exceptions` module in favour of new `errors` module.
   [#1071](https://github.com/OpenAssetIO/OpenAssetIO/issues/1071)
 
+- Changed success/return type of `defaultEntityReference` to wrap in
+  `std::optional`. Updated expected manager response such that an
+  optional without a value signals that no default is available for an
+  otherwise valid query. Changed default implementation to respond with
+  an empty optional in the success callback, rather than error.
+  [#1100](https://github.com/OpenAssetIO/OpenAssetIO/issues/1100)
+
 ### Improvements
 
 - Improved error messaging when `defaultManagerForInterface` points to a
diff --git a/src/openassetio-core/include/openassetio/hostApi/Manager.hpp b/src/openassetio-core/include/openassetio/hostApi/Manager.hpp
index f0d695ed6..992cf3d11 100644
--- a/src/openassetio-core/include/openassetio/hostApi/Manager.hpp
+++ b/src/openassetio-core/include/openassetio/hostApi/Manager.hpp
@@ -864,7 +864,8 @@ class OPENASSETIO_CORE_EXPORT Manager final {
   /**
    * Callback signature used for a successful default entity reference query.
    */
-  using DefaultEntityReferenceSuccessCallback = std::function<void(std::size_t, EntityReference)>;
+  using DefaultEntityReferenceSuccessCallback =
+      std::function<void(std::size_t, std::optional<EntityReference>)>;
 
   /**
    * Called to determine an @ref EntityReference considered to be a
@@ -888,20 +889,20 @@ class OPENASSETIO_CORE_EXPORT Manager final {
    *
    * @param successCallback Callback that will be called for each
    * successful default retrieved for each of the given sets in @p
-   * traitSets. It will be given the corresponding index of the traitSet
-   * in @p traitSets along with the default entity reference. The
-   * callback will be called on the same thread that initiated the call
-   * to `defaultEntityReference`.
+   * traitSets. It will be given the corresponding index of the trait
+   * set in @p traitSets along with the default entity reference. If the
+   * query is well-formed, but there is no available default entity
+   * reference, then the `optional` entity reference will not contain a
+   * value. The callback will be called on the same thread that
+   * initiated the call to `defaultEntityReference`.
    *
    * @param errorCallback Callback that will be called for each failure
    * to retrieve a sensible default entity reference. It will be given
    * the corresponding index for each of the given sets in @p traitSets
    * along with a populated @fqref{BatchElementError}
    * "BatchElementError" (see @fqref{BatchElementError.ErrorCode}
-   * "ErrorCodes"). The @fqref{BatchElementError.ErrorCode.kEntityAccessError}
-   * "kEntityAccessError" error will be used if no suitable default
-   * reference exists, and the @fqref{BatchElementError.ErrorCode.kInvalidTraitSet}
-   * "kInvalidTraitSet" error used if the requested trait set is
+   * "ErrorCodes"). The @fqref{BatchElementError.ErrorCode.kInvalidTraitSet}
+   * "kInvalidTraitSet" error will be used if the requested trait set is
    * not recognised by the manager. The callback will be called on the
    * same thread that initiated the call to `defaultEntityReference`.
    */
diff --git a/src/openassetio-core/include/openassetio/managerApi/ManagerInterface.hpp b/src/openassetio-core/include/openassetio/managerApi/ManagerInterface.hpp
index 1084a13f1..2b61a20b1 100644
--- a/src/openassetio-core/include/openassetio/managerApi/ManagerInterface.hpp
+++ b/src/openassetio-core/include/openassetio/managerApi/ManagerInterface.hpp
@@ -5,6 +5,7 @@
 
 #include <functional>
 #include <memory>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -800,7 +801,8 @@ class OPENASSETIO_CORE_EXPORT ManagerInterface {
    * Callback signature used for a successful default entity reference
    * query.
    */
-  using DefaultEntityReferenceSuccessCallback = std::function<void(std::size_t, EntityReference)>;
+  using DefaultEntityReferenceSuccessCallback =
+      std::function<void(std::size_t, std::optional<EntityReference>)>;
 
   /**
    * Called to determine an @ref EntityReference considered to be a
@@ -837,22 +839,22 @@ class OPENASSETIO_CORE_EXPORT ManagerInterface {
    *
    * @param successCallback Callback that will be called for each
    * successful default retrieved for each of the given sets in @p
-   * traitSets. It will be given the corresponding index of the traitSet
-   * in @p traitSets along with the default entity reference. The
-   * callback must called on the same thread that initiated the call
-   * to `defaultEntityReference`.
+   * traitSets. It should be given the corresponding index of the
+   * trait set in @p traitSets along with the default entity reference.
+   * If the query is well-formed, but there is no default entity
+   * reference, then provide an `optional` without a value. The callback
+   * must called on the same thread that initiated the call to
+   * `defaultEntityReference`.
    *
    * @param errorCallback Callback that must be called for each failure
    * to retrieve a sensible default entity reference. It should be given
    * the corresponding index for each of the given sets in @p traitSets
    * along with a populated @fqref{BatchElementError}
    * "BatchElementError" (see @fqref{BatchElementError.ErrorCode}
-   * "ErrorCodes"). The @fqref{BatchElementError.ErrorCode.kEntityAccessError}
-   * "kEntityAccessError" error must be used if no suitable default
-   * reference exists, and the @fqref{BatchElementError.ErrorCode.kInvalidTraitSet}
-   * "kInvalidTraitSet" error used if the requested trait set is
-   * unrecognised. The callback must be called on the same thread that
-   * initiated the call to `defaultEntityReference`.
+   * "ErrorCodes"). The @fqref{BatchElementError.ErrorCode.kInvalidTraitSet}
+   * "kInvalidTraitSet" error should be used if the requested trait set
+   * is unrecognised. The callback must be called on the same thread
+   * that initiated the call to `defaultEntityReference`.
    */
   virtual void defaultEntityReference(const trait::TraitSets& traitSets,
                                       access::DefaultEntityAccess defaultEntityAccess,
diff --git a/src/openassetio-core/src/managerApi/ManagerInterface.cpp b/src/openassetio-core/src/managerApi/ManagerInterface.cpp
index a920078c9..8067739a4 100644
--- a/src/openassetio-core/src/managerApi/ManagerInterface.cpp
+++ b/src/openassetio-core/src/managerApi/ManagerInterface.cpp
@@ -63,13 +63,11 @@ void ManagerInterface::defaultEntityReference(
     [[maybe_unused]] const access::DefaultEntityAccess defaultEntityAccess,
     [[maybe_unused]] const ContextConstPtr& context,
     [[maybe_unused]] const HostSessionPtr& hostSession,
-    [[maybe_unused]] const DefaultEntityReferenceSuccessCallback& successCallback,
-    const BatchElementErrorCallback& errorCallback) {
+    const DefaultEntityReferenceSuccessCallback& successCallback,
+    [[maybe_unused]] const BatchElementErrorCallback& errorCallback) {
   const auto size = traitSets.size();
   for (size_t i = 0; i < size; ++i) {
-    BatchElementError err{BatchElementError::ErrorCode::kEntityAccessError,
-                          "Manager does not implement defaultEntityReference"};
-    errorCallback(i, std::move(err));
+    successCallback(i, {});
   }
 }
 


Test patch:
diff --git a/src/openassetio-python/tests/package/hostApi/test_manager.py b/src/openassetio-python/tests/package/hostApi/test_manager.py
index ad6ac922b..4b53336c6 100644
--- a/src/openassetio-python/tests/package/hostApi/test_manager.py
+++ b/src/openassetio-python/tests/package/hostApi/test_manager.py
@@ -417,8 +417,9 @@ def test_wraps_the_corresponding_method_of_the_held_interface(
         method = mock_manager_interface.mock.defaultEntityReference
 
         def call_callbacks(*_args):
+            invoke_defaultEntityReference_success_cb(1, None)
             invoke_defaultEntityReference_success_cb(0, a_ref)
-            invoke_defaultEntityReference_error_cb(1, a_batch_element_error)
+            invoke_defaultEntityReference_error_cb(2, a_batch_element_error)
 
         method.side_effect = call_callbacks
 
@@ -438,8 +439,8 @@ def call_callbacks(*_args):
             mock.ANY,
         )
 
-        success_callback.assert_called_once_with(0, a_ref)
-        error_callback.assert_called_once_with(1, a_batch_element_error)
+        success_callback.assert_has_calls([mock.call(1, None), mock.call(0, a_ref)])
+        error_callback.assert_called_once_with(2, a_batch_element_error)
 
 
 class Test_Manager_getWithRelationship:
diff --git a/src/openassetio-python/tests/package/managerApi/test_managerinterface.py b/src/openassetio-python/tests/package/managerApi/test_managerinterface.py
index 31bfeecf1..c8aadaeab 100644
--- a/src/openassetio-python/tests/package/managerApi/test_managerinterface.py
+++ b/src/openassetio-python/tests/package/managerApi/test_managerinterface.py
@@ -159,15 +159,13 @@ def test_method_defined_in_cpp(self, method_introspector):
         )
         assert method_introspector.is_implemented_once(ManagerInterface, "defaultEntityReference")
 
-    def test_when_given_single_trait_set_then_calls_error_once(
-        self, manager_interface, a_host_session
-    ):
+    def test_gives_empty_value_for_all_trait_sets(self, manager_interface, a_host_session):
         success_callback = Mock()
         error_callback = Mock()
 
-        traitsSets = [set()]
+        traitsSets = [set(), set()]
 
-        refs = manager_interface.defaultEntityReference(
+        manager_interface.defaultEntityReference(
             traitsSets,
             DefaultEntityAccess.kRead,
             Context(),
@@ -176,38 +174,9 @@ def test_when_given_single_trait_set_then_calls_error_once(
             error_callback,
         )
 
-        success_callback.assert_not_called()
+        success_callback.assert_has_calls([call(0, None), call(1, None)])
 
-        expected_code = BatchElementError.ErrorCode.kEntityAccessError
-        expected_err = BatchElementError(
-            expected_code, "Manager does not implement defaultEntityReference"
-        )
-        error_callback.assert_has_calls([call(i, expected_err) for i in range(len(traitsSets))])
-
-    def test_when_given_multiple_trait_set_then_calls_error_once_for_each_trait_set(
-        self, manager_interface, a_host_session
-    ):
-        success_callback = Mock()
-        error_callback = Mock()
-
-        traitsSets = [set(), set(), set()]
-
-        refs = manager_interface.defaultEntityReference(
-            traitsSets,
-            DefaultEntityAccess.kRead,
-            Context(),
-            a_host_session,
-            success_callback,
-            error_callback,
-        )
-
-        success_callback.assert_not_called()
-
-        expected_code = BatchElementError.ErrorCode.kEntityAccessError
-        expected_err = BatchElementError(
-            expected_code, "Manager does not implement defaultEntityReference"
-        )
-        error_callback.assert_has_calls([call(i, expected_err) for i in range(len(traitsSets))])
+        error_callback.assert_not_called()
 
 
 class Test_ManagerInterface_getWithRelationship: