From 9a71adc6175860ad1d6e335fac3801f749a0b018 Mon Sep 17 00:00:00 2001
From: Wen Hui <wen.hui.ware@gmail.com>
Date: Thu, 7 Nov 2024 20:05:16 -0500
Subject: [PATCH] Revert "Decline unsubscribe related command in non-subscribed
 mode" (#1265)

This PR goal is to revert the changes on PR
https://github.com/valkey-io/valkey/pull/759

Recently, we got some reports that in Valkey 8.0 the PR
https://github.com/valkey-io/valkey/pull/759 (Decline unsubscribe
related command in non-subscribed mode) causes break change.
(https://github.com/valkey-io/valkey/issues/1228)

Although from my thought, call commands "unsubscribeCommand",
"sunsubscribeCommand", "punsubscribeCommand" in request-response mode
make no sense. This is why I created PR
https://github.com/valkey-io/valkey/pull/759

But breaking change is always no good, @valkey-io/core-team How do you
think we revert this PR code changes?

Signed-off-by: hwware <wen.hui.ware@gmail.com>
---
 src/server.c               |  6 ------
 tests/unit/info.tcl        |  3 +--
 tests/unit/pubsub.tcl      | 25 ++++++++++++++++++++-----
 tests/unit/pubsubshard.tcl |  5 +++--
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/src/server.c b/src/server.c
index 5882ec8f5..e341cc82e 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4093,12 +4093,6 @@ int processCommand(client *c) {
         return C_OK;
     }
 
-    /* Not allow several UNSUBSCRIBE commands executed under non-pubsub mode */
-    if (!c->flag.pubsub && (c->cmd->proc == unsubscribeCommand || c->cmd->proc == sunsubscribeCommand ||
-                            c->cmd->proc == punsubscribeCommand)) {
-        rejectCommandFormat(c, "-NOSUB '%s' command executed not in subscribed mode", c->cmd->fullname);
-        return C_OK;
-    }
     /* Only allow commands with flag "t", such as INFO, REPLICAOF and so on,
      * when replica-serve-stale-data is no and we are a replica with a broken
      * link with primary. */
diff --git a/tests/unit/info.tcl b/tests/unit/info.tcl
index 61d1acd1f..278a1d8e3 100644
--- a/tests/unit/info.tcl
+++ b/tests/unit/info.tcl
@@ -424,8 +424,7 @@ start_server {tags {"info" "external:skip"}} {
             set info [r info clients]
             assert_equal [getInfoProperty $info pubsub_clients] {1}
             # non-pubsub clients should not be involved
-            catch {unsubscribe $rd2 {non-exist-chan}} e
-            assert_match {*NOSUB*} $e
+            assert_equal {0} [unsubscribe $rd2 {non-exist-chan}]
             set info [r info clients]
             assert_equal [getInfoProperty $info pubsub_clients] {1}
             # close all clients
diff --git a/tests/unit/pubsub.tcl b/tests/unit/pubsub.tcl
index 68dc79a4a..24b78b6e5 100644
--- a/tests/unit/pubsub.tcl
+++ b/tests/unit/pubsub.tcl
@@ -109,12 +109,9 @@ start_server {tags {"pubsub network"}} {
         $rd1 close
     }
 
-    test "UNSUBSCRIBE and PUNSUBSCRIBE from non-subscribed channels" {
+    test "UNSUBSCRIBE from non-subscribed channels" {
         set rd1 [valkey_deferring_client]
-        foreach command {unsubscribe punsubscribe} {
-            catch {$command $rd1 {foo bar quux}} e
-            assert_match {*NOSUB*} $e
-        }
+        assert_equal {0 0 0} [unsubscribe $rd1 {foo bar quux}]
         # clean up clients
         $rd1 close
     }
@@ -204,6 +201,14 @@ start_server {tags {"pubsub network"}} {
         $rd close
     } {0} {resp3}
 
+    test "PUNSUBSCRIBE from non-subscribed channels" {
+        set rd1 [valkey_deferring_client]
+        assert_equal {0 0 0} [punsubscribe $rd1 {foo.* bar.* quux.*}]
+
+        # clean up clients
+        $rd1 close
+    }
+
     test "NUMSUB returns numbers, not strings (#1561)" {
         r pubsub numsub abc def
     } {abc 0 def 0}
@@ -241,6 +246,16 @@ start_server {tags {"pubsub network"}} {
         $rd1 close
     }
 
+    test "PUNSUBSCRIBE and UNSUBSCRIBE should always reply" {
+        # Make sure we are not subscribed to any channel at all.
+        r punsubscribe
+        r unsubscribe
+        # Now check if the commands still reply correctly.
+        set reply1 [r punsubscribe]
+        set reply2 [r unsubscribe]
+        concat $reply1 $reply2
+    } {punsubscribe {} 0 unsubscribe {} 0}
+
     ### Keyspace events notification tests
 
     test "Keyspace notifications: we receive keyspace notifications" {
diff --git a/tests/unit/pubsubshard.tcl b/tests/unit/pubsubshard.tcl
index d62a41570..e0e1e2972 100644
--- a/tests/unit/pubsubshard.tcl
+++ b/tests/unit/pubsubshard.tcl
@@ -74,8 +74,9 @@ start_server {tags {"pubsubshard external:skip"}} {
 
     test "SUNSUBSCRIBE from non-subscribed channels" {
         set rd1 [valkey_deferring_client]
-        catch {sunsubscribe $rd1 {foo}} e
-        assert_match {*NOSUB*} $e
+        assert_equal {0} [sunsubscribe $rd1 {foo}]
+        assert_equal {0} [sunsubscribe $rd1 {bar}]
+        assert_equal {0} [sunsubscribe $rd1 {quux}]
 
         # clean up clients
         $rd1 close