Repository: redis/redis-py
Pull request #2549 (2023-01-10 03:59:24)

Allow replica to master promotion in nodes_cache
### Pull Request check-list

_Please make sure to review and check all of these items:_

- [X] Does `$ tox` pass with this change (including linting)?
- [X] Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
- [X] Is the new or changed code fully tested?
- [ ] Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
- [ ] Is there an example added to the examples folder (if applicable)?
- [ ] Was the change added to CHANGES file?

_NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open._

### Description of change
_Please provide a description of the change here._
fixes https://github.com/redis/redis-py/issues/2433

When replica is promoted to master, ClusterNode's server_type must change accordingly, but the current implementation reuses the server_type, so no master is found in nodes_cache after initialization

This PR changes the server_type to a correct value, even if we reuse ClusterNode from nodes_cache

I've personally experienced this scenario, when i changed instance type in AWS Elasticache
  - changing instance type resulted in the addition of a new replica node and that node getting promoted to master



codecov-commenter: # [Codecov](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) Report
Base: **92.24**% // Head: **78.77**% // Decreases project coverage by **`-13.47%`** :warning:
> Coverage data is based on head [(`bf2a30c`)](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) compared to base [(`f46d7f3`)](https://codecov.io/gh/redis/redis-py/commit/f46d7f3ef7ce64f90acd4a3dcc14375ab9ae9c9d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis).
> Patch coverage: 3.70% of modified lines in pull request are covered.

> :exclamation: Current head bf2a30c differs from pull request most recent head af47499. Consider uploading reports for the commit af47499 to get more accurate results

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           master    #2549       +/-   ##
===========================================
- Coverage   92.24%   78.77%   -13.48%     
===========================================
  Files         115      115               
  Lines       29554    29581       +27     
===========================================
- Hits        27262    23301     -3961     
- Misses       2292     6280     +3988     
```


| [Impacted Files](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) | Coverage Δ | |
|---|---|---|
| [redis/cluster.py](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvY2x1c3Rlci5weQ==) | `18.77% <0.00%> (-71.58%)` | :arrow_down: |
| [tests/test\_cluster.py](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-dGVzdHMvdGVzdF9jbHVzdGVyLnB5) | `17.93% <4.00%> (-79.19%)` | :arrow_down: |
| [tests/test\_asyncio/test\_cluster.py](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-dGVzdHMvdGVzdF9hc3luY2lvL3Rlc3RfY2x1c3Rlci5weQ==) | `18.51% <0.00%> (-78.97%)` | :arrow_down: |
| [redis/asyncio/cluster.py](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvYXN5bmNpby9jbHVzdGVyLnB5) | `16.96% <0.00%> (-74.72%)` | :arrow_down: |
| [redis/asyncio/parser.py](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvYXN5bmNpby9wYXJzZXIucHk=) | `17.30% <0.00%> (-63.47%)` | :arrow_down: |
| [redis/commands/cluster.py](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvY29tbWFuZHMvY2x1c3Rlci5weQ==) | `36.95% <0.00%> (-57.07%)` | :arrow_down: |
| [redis/crc.py](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvY3JjLnB5) | `45.45% <0.00%> (-54.55%)` | :arrow_down: |
| [redis/exceptions.py](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#diff-cmVkaXMvZXhjZXB0aW9ucy5weQ==) | `86.95% <0.00%> (-11.60%)` | :arrow_down: |
| ... and [12 more](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/redis/redis-py/pull/2549?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis).


Patch:
diff --git a/CHANGES b/CHANGES
index fca8d316..02daf5ee 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,4 @@
+    * Fix string cleanse in Redis Graph
     * Make PythonParser resumable in case of error (#2510)
     * Add `timeout=None` in `SentinelConnectionManager.read_response`
     * Documentation fix: password protected socket connection (#2374)
diff --git a/redis/cluster.py b/redis/cluster.py
index f1150073..235d8f2d 100644
--- a/redis/cluster.py
+++ b/redis/cluster.py
@@ -1437,6 +1437,8 @@ class NodesManager:
             if target_node is None or target_node.redis_connection is None:
                 # create new cluster node for this cluster
                 target_node = ClusterNode(host, port, role)
+            if target_node.server_type != role:
+                target_node.server_type = role
 
         return target_node
 
diff --git a/redis/commands/helpers.py b/redis/commands/helpers.py
index 6989ab59..b65cd1a9 100644
--- a/redis/commands/helpers.py
+++ b/redis/commands/helpers.py
@@ -115,6 +115,7 @@ def quote_string(v):
     if len(v) == 0:
         return '""'
 
+    v = v.replace("\\", "\\\\")
     v = v.replace('"', '\\"')
 
     return f'"{v}"'


Test patch:
diff --git a/tests/test_cluster.py b/tests/test_cluster.py
index 27cfee1c..55b660c7 100644
--- a/tests/test_cluster.py
+++ b/tests/test_cluster.py
@@ -2262,6 +2262,57 @@ class TestNodesManager:
 
         assert len(n_manager.nodes_cache) == 6
 
+    def test_init_promote_server_type_for_node_in_cache(self):
+        """
+        When replica is promoted to master, nodes_cache must change the server type
+        accordingly
+        """
+        cluster_slots_before_promotion = [
+            [0, 16383, ["127.0.0.1", 7000], ["127.0.0.1", 7003]]
+        ]
+        cluster_slots_after_promotion = [
+            [0, 16383, ["127.0.0.1", 7003], ["127.0.0.1", 7004]]
+        ]
+
+        cluster_slots_results = [
+            cluster_slots_before_promotion,
+            cluster_slots_after_promotion,
+        ]
+
+        with patch.object(Redis, "execute_command") as execute_command_mock:
+
+            def execute_command(*_args, **_kwargs):
+                if _args[0] == "CLUSTER SLOTS":
+                    mock_cluster_slots = cluster_slots_results.pop(0)
+                    return mock_cluster_slots
+                elif _args[0] == "COMMAND":
+                    return {"get": [], "set": []}
+                elif _args[0] == "INFO":
+                    return {"cluster_enabled": True}
+                elif len(_args) > 1 and _args[1] == "cluster-require-full-coverage":
+                    return {"cluster-require-full-coverage": False}
+                else:
+                    return execute_command_mock(*_args, **_kwargs)
+
+            execute_command_mock.side_effect = execute_command
+
+            nm = NodesManager(
+                startup_nodes=[ClusterNode(host=default_host, port=default_port)],
+                from_url=False,
+                require_full_coverage=False,
+                dynamic_startup_nodes=True,
+            )
+
+            assert nm.default_node.host == "127.0.0.1"
+            assert nm.default_node.port == 7000
+            assert nm.default_node.server_type == PRIMARY
+
+            nm.initialize()
+
+            assert nm.default_node.host == "127.0.0.1"
+            assert nm.default_node.port == 7003
+            assert nm.default_node.server_type == PRIMARY
+
     def test_init_slots_cache_cluster_mode_disabled(self):
         """
         Test that creating a RedisCluster failes if one of the startup nodes
diff --git a/tests/test_graph.py b/tests/test_graph.py
index d71df486..4721b2f4 100644
--- a/tests/test_graph.py
+++ b/tests/test_graph.py
@@ -124,7 +124,7 @@ def test_path(client):
 
 @pytest.mark.redismod
 def test_param(client):
-    params = [1, 2.3, "str", True, False, None, [0, 1, 2]]
+    params = [1, 2.3, "str", True, False, None, [0, 1, 2], r"\" RETURN 1337 //"]
     query = "RETURN $param"
     for param in params:
         result = client.graph().query(query, {"param": param})
diff --git a/tests/test_helpers.py b/tests/test_helpers.py
index 35958290..57a94d2f 100644
--- a/tests/test_helpers.py
+++ b/tests/test_helpers.py
@@ -80,3 +80,9 @@ def test_quote_string():
     assert quote_string("hello world!") == '"hello world!"'
     assert quote_string("") == '""'
     assert quote_string("hello world!") == '"hello world!"'
+    assert quote_string("abc") == '"abc"'
+    assert quote_string("") == '""'
+    assert quote_string('"') == r'"\""'
+    assert quote_string(r"foo \ bar") == r'"foo \\ bar"'
+    assert quote_string(r"foo \" bar") == r'"foo \\\" bar"'
+    assert quote_string('a"a') == r'"a\"a"'