From 7333b1fc0d860403812ece1a6fedee04e79713af Mon Sep 17 00:00:00 2001
From: Amna <amna.snene@savoirfairelinux.com>
Date: Tue, 27 Aug 2024 10:58:34 -0400
Subject: [PATCH] upnp: make released mappings available for future use

When a client (controller) releases a mapping, the expected behaviour is
that the client deletes it from its local mapping list (the controller
list) and the context makes it available for future use (by the same
controller or any other controller). Currently, when the client releases
a mapping, the context requests a mapping removal from the IGD and
removes it from its local list (the context list).

GitLab: #39
Change-Id: Ic7d54551372f2af9752c1469dc3442db2503a0fb
---
 include/upnp/upnp_context.h |  1 -
 src/upnp/upnp_context.cpp   | 13 +++++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/upnp/upnp_context.h b/include/upnp/upnp_context.h
index d161053..e29e05b 100644
--- a/include/upnp/upnp_context.h
+++ b/include/upnp/upnp_context.h
@@ -105,7 +105,6 @@ public:
     Mapping::sharedPtr_t reserveMapping(Mapping& requestedMap);
 
     // Release a used mapping (make it available for future use).
-    // TODO: The current implementation doesn't seem to do the "make it available for future use" part... fix this.
     void releaseMapping(const Mapping& map);
 
     // Register a controller
diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp
index cf9b6ed..cffab99 100644
--- a/src/upnp/upnp_context.cpp
+++ b/src/upnp/upnp_context.cpp
@@ -399,9 +399,14 @@ UPnPContext::releaseMapping(const Mapping& map)
             return;
         }
 
-        // Remove it.
-        requestRemoveMapping(mapPtr);
-        unregisterMapping(mapPtr, true);
+        // Reset the mapping options: disable auto-update and remove the notify callback.
+        // This is important because the mapping will be available again and can be reused
+        // by another (or the same) controller which may have different preferences.
+        // The notify callback is also removed to avoid calling it when the mapping is not used anymore.
+        mapPtr->setNotifyCallback(nullptr);
+        mapPtr->enableAutoUpdate(false);
+        mapPtr->setAvailable(true);
+        if (logger_) logger_->debug("Mapping {} released", mapPtr->toString());
         enforceAvailableMappingsLimits();
     });
 }
@@ -620,7 +625,7 @@ UPnPContext::enforceAvailableMappingsLimits()
     // If there is no valid IGD, do nothing.
     if (!isReady())
         return;
-        
+
     for (auto type : {PortType::TCP, PortType::UDP}) {
         int pendingCount = 0;
         int inProgressCount = 0;
-- 
GitLab