Repository: hazelcast/hazelcast-python-client

Issue:
Do not allow using METADATA as the group name on the CP data structures
`METADATA` is a special group name dedicated to the internal CP group that is used for maintaining CP groups and members. As of now, when you try to create a CP data structure with the `METADATA` as the group name, you receive an error from the server-side. However, it would be good if we can fail fast on the client-side.

So, a code like this should fail on the client-side.

```python
import hazelcast

client = hazelcast.HazelcastClient()

view_counter = client.cp_subsystem.get_atomic_long("views@METADATA")
```

Discussion:

Patch:
diff --git a/hazelcast/cp.py b/hazelcast/cp.py
index b131c7b..d1dd2c0 100644
--- a/hazelcast/cp.py
+++ b/hazelcast/cp.py
@@ -155,6 +155,7 @@ class CPSubsystem(object):
 
 
 _DEFAULT_GROUP_NAME = "default"
+_METADATA_CP_GROUP_NAME = "metadata"
 
 
 def _without_default_group_name(name):
@@ -165,7 +166,11 @@ def _without_default_group_name(name):
 
     check_true(name.find("@", idx + 1) == -1, "Custom group name must be specified at most once")
     group_name = name[idx + 1 :].strip()
-    if group_name == _DEFAULT_GROUP_NAME:
+    check_true(
+        group_name.lower() != _METADATA_CP_GROUP_NAME,
+        "CP data structures cannot run on the METADATA CP group!",
+    )
+    if group_name.lower() == _DEFAULT_GROUP_NAME:
         return name[:idx]
     return name
 


Test patch:
diff --git a/tests/integration/backward_compatible/client_test.py b/tests/integration/backward_compatible/client_test.py
index ac933a7..788186d 100644
--- a/tests/integration/backward_compatible/client_test.py
+++ b/tests/integration/backward_compatible/client_test.py
@@ -4,7 +4,7 @@ from tests.base import HazelcastTestCase
 from hazelcast.client import HazelcastClient
 from hazelcast.lifecycle import LifecycleState
 from tests.hzrc.ttypes import Lang
-from tests.integration.backward_compatible.util import get_current_timestamp
+from tests.util import get_current_timestamp
 
 
 class ClientTest(HazelcastTestCase):
diff --git a/tests/integration/backward_compatible/proxy/cp/atomic_long_test.py b/tests/integration/backward_compatible/proxy/cp/atomic_long_test.py
index 5b1bf93..416358b 100644
--- a/tests/integration/backward_compatible/proxy/cp/atomic_long_test.py
+++ b/tests/integration/backward_compatible/proxy/cp/atomic_long_test.py
@@ -1,7 +1,7 @@
 from hazelcast.errors import DistributedObjectDestroyedError
 from hazelcast.serialization.api import IdentifiedDataSerializable
 from tests.integration.backward_compatible.proxy.cp import CPTestCase
-from tests.util import set_attr
+from tests.util import mark_server_version_at_least
 
 
 class Multiplication(IdentifiedDataSerializable):
@@ -106,30 +106,30 @@ class AtomicLongTest(CPTestCase):
         self.assertIsNone(self.atomic_long.set(42))
         self.assertEqual(42, self.atomic_long.get())
 
-    @set_attr(min_server_version="4.1")
     def test_alter(self):
         # the class is defined in the 4.1 JAR
+        mark_server_version_at_least(self, self.client, "4.1")
         self.atomic_long.set(2)
         self.assertIsNone(self.atomic_long.alter(Multiplication(5)))
         self.assertEqual(10, self.atomic_long.get())
 
-    @set_attr(min_server_version="4.1")
     def test_alter_and_get(self):
         # the class is defined in the 4.1 JAR
+        mark_server_version_at_least(self, self.client, "4.1")
         self.atomic_long.set(-3)
         self.assertEqual(-9, self.atomic_long.alter_and_get(Multiplication(3)))
         self.assertEqual(-9, self.atomic_long.get())
 
-    @set_attr(min_server_version="4.1")
     def test_get_and_alter(self):
         # the class is defined in the 4.1 JAR
+        mark_server_version_at_least(self, self.client, "4.1")
         self.atomic_long.set(123)
         self.assertEqual(123, self.atomic_long.get_and_alter(Multiplication(-1)))
         self.assertEqual(-123, self.atomic_long.get())
 
-    @set_attr(min_server_version="4.1")
     def test_apply(self):
         # the class is defined in the 4.1 JAR
+        mark_server_version_at_least(self, self.client, "4.1")
         self.atomic_long.set(42)
         self.assertEqual(84, self.atomic_long.apply(Multiplication(2)))
         self.assertEqual(42, self.atomic_long.get())
diff --git a/tests/integration/backward_compatible/proxy/cp/atomic_reference_test.py b/tests/integration/backward_compatible/proxy/cp/atomic_reference_test.py
index 2d36070..fa99523 100644
--- a/tests/integration/backward_compatible/proxy/cp/atomic_reference_test.py
+++ b/tests/integration/backward_compatible/proxy/cp/atomic_reference_test.py
@@ -3,7 +3,7 @@ from hazelcast.serialization.api import IdentifiedDataSerializable
 
 from tests.integration.backward_compatible.util import write_string_to_output
 from tests.integration.backward_compatible.proxy.cp import CPTestCase
-from tests.util import set_attr
+from tests.util import mark_server_version_at_least
 
 
 class AppendString(IdentifiedDataSerializable):
@@ -108,37 +108,37 @@ class AtomicReferenceTest(CPTestCase):
         self.assertFalse(self.ref.contains("42"))
         self.assertTrue(self.ref.contains(None))
 
-    @set_attr(min_server_version="4.1")
     def test_alter(self):
         # the class is defined in the 4.1 JAR
+        mark_server_version_at_least(self, self.client, "4.1")
         self.ref.set("hey")
         self.assertIsNone(self.ref.alter(AppendString("123")))
         self.assertEqual("hey123", self.ref.get())
 
-    @set_attr(min_server_version="4.1")
     def test_alter_with_incompatible_types(self):
         # the class is defined in the 4.1 JAR
+        mark_server_version_at_least(self, self.client, "4.1")
         self.ref.set(42)
         with self.assertRaises(ClassCastError):
             self.ref.alter(AppendString("."))
 
-    @set_attr(min_server_version="4.1")
     def test_alter_and_get(self):
         # the class is defined in the 4.1 JAR
+        mark_server_version_at_least(self, self.client, "4.1")
         self.ref.set("123")
         self.assertEqual("123...", self.ref.alter_and_get(AppendString("...")))
         self.assertEqual("123...", self.ref.get())
 
-    @set_attr(min_server_version="4.1")
     def test_get_and_alter(self):
         # the class is defined in the 4.1 JAR
+        mark_server_version_at_least(self, self.client, "4.1")
         self.ref.set("hell")
         self.assertEqual("hell", self.ref.get_and_alter(AppendString("o")))
         self.assertEqual("hello", self.ref.get())
 
-    @set_attr(min_server_version="4.1")
     def test_apply(self):
         # the class is defined in the 4.1 JAR
+        mark_server_version_at_least(self, self.client, "4.1")
         self.ref.set("hell")
         self.assertEqual("hello", self.ref.apply(AppendString("o")))
         self.assertEqual("hell", self.ref.get())
diff --git a/tests/integration/backward_compatible/proxy/cp/count_down_latch_test.py b/tests/integration/backward_compatible/proxy/cp/count_down_latch_test.py
index 1268048..b81229a 100644
--- a/tests/integration/backward_compatible/proxy/cp/count_down_latch_test.py
+++ b/tests/integration/backward_compatible/proxy/cp/count_down_latch_test.py
@@ -4,8 +4,7 @@ from hazelcast.errors import DistributedObjectDestroyedError, OperationTimeoutEr
 from hazelcast.future import ImmediateExceptionFuture
 from hazelcast.util import AtomicInteger
 from tests.integration.backward_compatible.proxy.cp import CPTestCase
-from tests.integration.backward_compatible.util import get_current_timestamp
-from tests.util import random_string
+from tests.util import get_current_timestamp, random_string
 
 
 inf = 2 ** 31 - 1
diff --git a/tests/integration/backward_compatible/proxy/cp/semaphore_test.py b/tests/integration/backward_compatible/proxy/cp/semaphore_test.py
index 595b14f..2e67053 100644
--- a/tests/integration/backward_compatible/proxy/cp/semaphore_test.py
+++ b/tests/integration/backward_compatible/proxy/cp/semaphore_test.py
@@ -9,8 +9,7 @@ from hazelcast.errors import (
     IllegalStateError,
 )
 from tests.integration.backward_compatible.proxy.cp import CPTestCase
-from tests.integration.backward_compatible.util import get_current_timestamp
-from tests.util import random_string
+from tests.util import get_current_timestamp, random_string
 
 SEMAPHORE_TYPES = [
     "sessionless",
diff --git a/tests/integration/backward_compatible/statistics_test.py b/tests/integration/backward_compatible/statistics_test.py
index e85e15d..6d67117 100644
--- a/tests/integration/backward_compatible/statistics_test.py
+++ b/tests/integration/backward_compatible/statistics_test.py
@@ -6,8 +6,7 @@ from hazelcast.core import CLIENT_TYPE
 from hazelcast.statistics import Statistics
 from tests.base import HazelcastTestCase
 from tests.hzrc.ttypes import Lang
-from tests.integration.backward_compatible.util import get_current_timestamp
-from tests.util import random_string
+from tests.util import get_current_timestamp, random_string
 
 
 class StatisticsTest(HazelcastTestCase):
diff --git a/tests/integration/backward_compatible/util.py b/tests/integration/backward_compatible/util.py
index dc78e3f..534a149 100644
--- a/tests/integration/backward_compatible/util.py
+++ b/tests/integration/backward_compatible/util.py
@@ -1,11 +1,3 @@
-import time
-
-try:
-    from tests.util import get_current_timestamp
-except ImportError:
-    get_current_timestamp = time.time
-
-
 def read_string_from_input(inp):
     if hasattr(inp, "read_string"):
         return inp.read_string()
diff --git a/tests/unit/proxy/cp/cp_proxy_manager_test.py b/tests/unit/proxy/cp/cp_proxy_manager_test.py
index 4b1b792..e0d1b29 100644
--- a/tests/unit/proxy/cp/cp_proxy_manager_test.py
+++ b/tests/unit/proxy/cp/cp_proxy_manager_test.py
@@ -6,12 +6,20 @@ from hazelcast.cp import _without_default_group_name, _get_object_name_for_proxy
 class CPProxyManagerTest(unittest.TestCase):
     def test_without_default_group_name(self):
         self.assertEqual("test", _without_default_group_name("test@default"))
+        self.assertEqual("test", _without_default_group_name("test@DEFAULT"))
         self.assertEqual("test@custom", _without_default_group_name("test@custom"))
 
     def test_without_default_group_name_with_multiple_group_names(self):
         with self.assertRaises(AssertionError):
             _without_default_group_name("test@default@@default")
 
+    def test_without_default_group_name_with_metadata_group_name(self):
+        with self.assertRaises(AssertionError):
+            _without_default_group_name("test@METADATA")
+
+        with self.assertRaises(AssertionError):
+            _without_default_group_name("test@metadata")
+
     def test_get_object_name_for_proxy(self):
         self.assertEqual("test", _get_object_name_for_proxy("test@default"))
         self.assertEqual("test", _get_object_name_for_proxy("test@custom"))
diff --git a/tests/util.py b/tests/util.py
index 58f8517..909b7c0 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -3,6 +3,7 @@ import time
 
 from uuid import uuid4
 from hazelcast.config import SSLProtocol
+from hazelcast.util import calculate_version
 
 # time.monotonic() is more consistent since it uses cpu clock rather than system clock. Use it if available.
 if hasattr(time, "monotonic"):
@@ -88,6 +89,14 @@ def set_attr(*args, **kwargs):
     return wrap_ob
 
 
+def mark_server_version_at_least(test, client, expected_version):
+    connection = client._connection_manager.get_random_connection()
+    server_version = connection.server_version
+    expected_version = calculate_version(expected_version)
+    if server_version < expected_version:
+        test.skipTest("Expected a newer server")
+
+
 def open_connection_to_address(client, uuid):
     key = generate_key_owned_by_instance(client, uuid)
     m = client.get_map(random_string()).blocking()