[dns-sd] Untangle dependencies between Resolver and ResolverProxy (#3…

…0563)

* [dns-sd] Untangle dependencies between Resolver and ResolverProxy

The relationship between Resolver and Resolver Proxy is
unclear as both classes depend on each other and Resolver
Proxy is implemented in a source file of concrete Resolver
implementation. This makes it impossible to build Matter
library with several Resolver implementations and select
one to be used at runtime.

The main reason for introducing Resolver Proxy was the need
for running multiple parallel DNS-SD queries. Address this
by adding Discovery Delegate parameter to the Resolver
interface methods for starting and stopping the discovery.

Then implement Resolver Proxy as a convenience class on top
of the Resolver interface. Additionally, simplify the
Commissionable Node Controller class by injecting a test
Resolver into Resolver Proxy.

* Fix shell commands

* Rename DiscoveryDelegate to DiscoveryContext

* Add comments

src/app/CASESessionManager.h

@@ -28,8 +28,6 @@
 #include <transport/SessionDelegate.h>
 #include <transport/SessionUpdateDelegate.h>
 
-#include <lib/dnssd/ResolverProxy.h>
-
 namespace chip {
 
 class OperationalSessionSetupPoolDelegate;

src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp

@@ -117,15 +117,13 @@ CHIP_ERROR DiscoveryCommands::SetupDiscoveryCommands()
         ReturnErrorOnFailure(mDNSResolver.Init(chip::DeviceLayer::UDPEndPointManager()));
         mReady = true;
     }
-    mDNSResolver.SetOperationalDelegate(this);
     mDNSResolver.SetCommissioningDelegate(this);
     return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR DiscoveryCommands::TearDownDiscoveryCommands()
 {
     mDNSResolver.StopDiscovery();
-    mDNSResolver.SetOperationalDelegate(nullptr);
     mDNSResolver.SetCommissioningDelegate(nullptr);
     return CHIP_NO_ERROR;
 }

src/app/tests/suites/commands/discovery/DiscoveryCommands.h

@@ -24,7 +24,7 @@
 
 #include <app-common/zap-generated/tests/simulated-cluster-objects.h>
 
-class DiscoveryCommands : public chip::Dnssd::CommissioningResolveDelegate, public chip::Dnssd::OperationalResolveDelegate
+class DiscoveryCommands : public chip::Dnssd::CommissioningResolveDelegate
 {
 public:
     DiscoveryCommands(){};
@@ -64,10 +64,6 @@ class DiscoveryCommands : public chip::Dnssd::CommissioningResolveDelegate, publ
     /////////// CommissioningDelegate Interface /////////
     void OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
 
-    /////////// OperationalDelegate Interface /////////
-    void OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override{};
-    void OnOperationalNodeResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override{};
-
 private:
     bool mReady = false;
     chip::Dnssd::ResolverProxy mDNSResolver;

src/controller/AbstractDnssdDiscoveryController.h

@@ -39,8 +39,7 @@ namespace Controller {
 class DLL_EXPORT AbstractDnssdDiscoveryController : public Dnssd::CommissioningResolveDelegate
 {
 public:
-    AbstractDnssdDiscoveryController() {}
-    ~AbstractDnssdDiscoveryController() override { mDNSResolver.Shutdown(); }
+    explicit AbstractDnssdDiscoveryController(Dnssd::Resolver * resolver = nullptr) : mDNSResolver(resolver) {}
 
     void OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
     CHIP_ERROR StopDiscovery() { return mDNSResolver.StopDiscovery(); };

src/controller/CHIPCommissionableNodeController.cpp

@@ -32,30 +32,12 @@ CHIP_ERROR CommissionableNodeController::DiscoverCommissioners(Dnssd::DiscoveryF
 {
     ReturnErrorOnFailure(SetUpNodeDiscovery());
 
-    if (mResolver == nullptr)
-    {
 #if CONFIG_DEVICE_LAYER
-        mDNSResolver.Shutdown(); // reset if already inited
-        ReturnErrorOnFailure(mDNSResolver.Init(DeviceLayer::UDPEndPointManager()));
+    mDNSResolver.Shutdown(); // reset if already inited
+    ReturnErrorOnFailure(mDNSResolver.Init(DeviceLayer::UDPEndPointManager()));
 #endif
-        mDNSResolver.SetCommissioningDelegate(this);
-        return mDNSResolver.DiscoverCommissioners(discoveryFilter);
-    }
-
-#if CONFIG_DEVICE_LAYER
-    ReturnErrorOnFailure(mResolver->Init(DeviceLayer::UDPEndPointManager()));
-#endif
-    return mResolver->DiscoverCommissioners(discoveryFilter);
-}
-
-CHIP_ERROR CommissionableNodeController::StopDiscovery()
-{
-    if (mResolver == nullptr)
-    {
-        return AbstractDnssdDiscoveryController::StopDiscovery();
-    }
-
-    return mResolver->StopDiscovery();
+    mDNSResolver.SetCommissioningDelegate(this);
+    return mDNSResolver.DiscoverCommissioners(discoveryFilter);
 }
 
 CommissionableNodeController::~CommissionableNodeController()

src/controller/CHIPCommissionableNodeController.h

@@ -36,14 +36,12 @@ namespace Controller {
 class DLL_EXPORT CommissionableNodeController : public AbstractDnssdDiscoveryController
 {
 public:
-    CommissionableNodeController(chip::Dnssd::Resolver * resolver = nullptr) : mResolver(resolver) {}
+    CommissionableNodeController(chip::Dnssd::Resolver * resolver = nullptr) : AbstractDnssdDiscoveryController(resolver) {}
     ~CommissionableNodeController() override;
 
     void RegisterDeviceDiscoveryDelegate(DeviceDiscoveryDelegate * delegate) { mDeviceDiscoveryDelegate = delegate; }
     CHIP_ERROR DiscoverCommissioners(Dnssd::DiscoveryFilter discoveryFilter = Dnssd::DiscoveryFilter());
 
-    CHIP_ERROR StopDiscovery();
-
     /**
      * @return
      *   Pointer to DiscoveredNodeData at index idx in the list of commissioners discovered
@@ -57,7 +55,6 @@ class DLL_EXPORT CommissionableNodeController : public AbstractDnssdDiscoveryCon
     DiscoveredNodeList GetDiscoveredNodes() override { return DiscoveredNodeList(mDiscoveredCommissioners); }
 
 private:
-    Dnssd::Resolver * mResolver = nullptr;
     Dnssd::DiscoveredNodeData mDiscoveredCommissioners[CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES];
 };
 

src/controller/tests/TestCommissionableNodeController.cpp

@@ -36,15 +36,14 @@ class MockResolver : public Resolver
     bool IsInitialized() override { return true; }
     void Shutdown() override {}
     void SetOperationalDelegate(OperationalResolveDelegate * delegate) override {}
-    void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override {}
     CHIP_ERROR ResolveNodeId(const PeerId & peerId) override { return ResolveNodeIdStatus; }
     void NodeIdResolutionNoLongerNeeded(const PeerId & peerId) override {}
-    CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override { return DiscoverCommissionersStatus; }
-    CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override
+    CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter, DiscoveryContext &) override { return DiscoverCommissionersStatus; }
+    CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter, DiscoveryContext &) override
     {
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }
-    CHIP_ERROR StopDiscovery() override { return CHIP_ERROR_NOT_IMPLEMENTED; }
+    CHIP_ERROR StopDiscovery(DiscoveryContext &) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
     CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override
     {
         return CHIP_ERROR_NOT_IMPLEMENTED;

src/lib/dnssd/BUILD.gn

@@ -34,6 +34,8 @@ static_library("dnssd") {
     "IPAddressSorter.cpp",
     "IPAddressSorter.h",
     "Resolver.h",
+    "ResolverProxy.cpp",
+    "ResolverProxy.h",
     "ServiceNaming.cpp",
     "ServiceNaming.h",
     "TxtFields.cpp",