Implement review comments

geoservercloud/geoservercloud.py

@@ -306,6 +306,7 @@ def create_wmts_store(
                 self.rest_endpoints.wmtsstore(workspace_name, name), json=payload
             )
 
+    # TODO: add a test for this method
     def get_feature_types(
         self, workspace_name: str, datastore_name: str
     ) -> dict[str, Any]:
@@ -319,6 +320,7 @@ def get_feature_types(
         )
         return featuretypes
 
+    # TODO: add a test for this method
     def get_feature_type(
         self, workspace_name: str, datastore_name: str, feature_type_name: str
     ) -> dict[str, Any]:
@@ -337,7 +339,7 @@ def create_feature_type(
         datastore: str | None = None,
         title: str | dict = "Default title",
         abstract: str | dict = "Default abstract",
-        attributes: dict = Templates.geom_point_attribute(),
+        attributes: dict = Templates.geom_point_attribute(),  # TODO: remove default value, because if should be None
         epsg: int = 4326,
     ) -> Response:
         """
@@ -349,6 +351,7 @@ def create_feature_type(
         datastore = datastore or self.default_datastore
         if not datastore:
             raise ValueError("Datastore not provided")
+        # TODO: use FeatureType.post_payload()
         payload: dict[str, dict[str, Any]] = Templates.feature_type(
             layer=layer,
             workspace=workspace_name,

geoservercloud/models/common.py

@@ -50,20 +50,38 @@ def update(self, other: dict):  # type: ignore
 
 class I18N:
     """
-    Class to handle internationalization of strings
-    items like title, abstract, etc. that can be internationalized
-    become a dictionary with the key being the language code
-    and their key in the payload changes to internationalizedTitle, internationalizedAbstract, etc.
+    Geoserver handles internationalization with 2 possible (mutually exclusive) keys in the rest payload:
+    either:
+    - [key: string]
+    or
+    - [internationalKey: dictionary]
+
+    example:
+    a) as key: string we get {"title": "Test Title"}
+    b) as key: dict we get {"internationalTitle": {"en": "Test Title", "es": "Título de Prueba"}}
+
+    This class gives a layer of abstraction to handle both cases.
+
+    Usage:
+    Call the class by adding both possible keys in a tuple and the value.
+
+    Parameters:
+    keys: tuple[str, str] example : ("title", "internationalTitle")
+    value: str | dict example: "Test Title" | {"en": "Test Title", "es": "Título de Prueba"}
+
+    Example:
+
+    my_i18n = I18N(("title", "internationalTitle"), "Test Title")
     """
 
     def __init__(self, keys: tuple[str, Any], value: str | dict) -> None:
         self._str_key = keys[0]
         self._i18n_key = keys[1]
         self._value = value
         if isinstance(value, str):
-            self._payload = (self.str_key, self._value)
+            self._content = {self.str_key: self._value}
         elif isinstance(value, dict):
-            self._payload = (self._i18n_key, self._value)  # type: ignore
+            self._content = {self._i18n_key: self._value}
         else:
             raise ValueError("Invalid value type")
 
@@ -79,9 +97,8 @@ def i18n_key(self):
     def value(self):
         return self._value
 
-    @property
-    def payload_tuple(self):
-        return self._payload
+    def asdict(self):
+        return self._content
 
     def __repr__(self):
-        return json.dumps({self._payload[0]: self._payload[1]}, indent=4)
+        return json.dumps(self._content, indent=4)

geoservercloud/models/featuretype.py

@@ -72,12 +72,12 @@ def post_payload(self):
             "featureType": {
                 "name": self.name,
                 "nativeName": self.native_name,
-                self._title.payload_tuple[0]: self._title.payload_tuple[1],
-                self._abstract.payload_tuple[0]: self._abstract.payload_tuple[1],
                 "srs": self.srs,
                 "keywords": self.keywords,
             }
         }
+        payload["featureType"].update(self.title.asdict())
+        payload["featureType"].update(self.abstract.asdict())
         if self.metadataLink != {}:
             payload["featureType"]["metadataLinks"] = self.metadataLink
         if self.attributes:

geoservercloud/models/workspaces.py

@@ -19,7 +19,6 @@ def workspaces(self):
 
     @classmethod
     def from_response(cls, response: Response):
-        # Parse the JSON response
         json_data = response.json()
 
         workspaces = []

geoservercloud/templates.py

@@ -83,71 +83,72 @@ def workspace_wms(workspace: str) -> dict[str, dict[str, Any]]:
             }
         }
 
-    @staticmethod
-    def postgis_data_store(
-        datastore: str,
-        pg_host: str,
-        pg_port: int,
-        pg_db: str,
-        pg_user: str,
-        pg_password: str,
-        namespace: str,
-        pg_schema: str = "public",
-    ) -> dict[str, dict[str, Any]]:
-        return {
-            "dataStore": {
-                "name": datastore,
-                "connectionParameters": {
-                    "entry": [
-                        {"@key": "dbtype", "$": "postgis"},
-                        {"@key": "host", "$": pg_host},
-                        {"@key": "port", "$": pg_port},
-                        {"@key": "database", "$": pg_db},
-                        {"@key": "user", "$": pg_user},
-                        {"@key": "passwd", "$": pg_password},
-                        {"@key": "schema", "$": pg_schema},
-                        {
-                            "@key": "namespace",
-                            "$": namespace,
-                        },
-                        {"@key": "Expose primary keys", "$": "true"},
-                    ]
-                },
-            }
-        }
+    # TODO: verify that we don't need these 2 templates anymore
+    # @staticmethod
+    # def postgis_data_store(
+    #     datastore: str,
+    #     pg_host: str,
+    #     pg_port: int,
+    #     pg_db: str,
+    #     pg_user: str,
+    #     pg_password: str,
+    #     namespace: str,
+    #     pg_schema: str = "public",
+    # ) -> dict[str, dict[str, Any]]:
+    #     return {
+    #         "dataStore": {
+    #             "name": datastore,
+    #             "connectionParameters": {
+    #                 "entry": [
+    #                     {"@key": "dbtype", "$": "postgis"},
+    #                     {"@key": "host", "$": pg_host},
+    #                     {"@key": "port", "$": pg_port},
+    #                     {"@key": "database", "$": pg_db},
+    #                     {"@key": "user", "$": pg_user},
+    #                     {"@key": "passwd", "$": pg_password},
+    #                     {"@key": "schema", "$": pg_schema},
+    #                     {
+    #                         "@key": "namespace",
+    #                         "$": namespace,
+    #                     },
+    #                     {"@key": "Expose primary keys", "$": "true"},
+    #                 ]
+    #             },
+    #         }
+    #     }
 
-    @staticmethod
-    def postgis_jndi_data_store(
-        datastore: str,
-        jndi_reference: str,
-        namespace: str,
-        pg_schema: str = "public",
-        description: str | None = None,
-    ) -> dict[str, dict[str, Any]]:
-        return {
-            "dataStore": {
-                "name": datastore,
-                "description": description,
-                "connectionParameters": {
-                    "entry": [
-                        {"@key": "dbtype", "$": "postgis"},
-                        {
-                            "@key": "jndiReferenceName",
-                            "$": jndi_reference,
-                        },
-                        {
-                            "@key": "schema",
-                            "$": pg_schema,
-                        },
-                        {
-                            "@key": "namespace",
-                            "$": namespace,
-                        },
-                        {"@key": "Expose primary keys", "$": "true"},
-                    ]
-                },
-            }
-        }
+    # @staticmethod
+    # def postgis_jndi_data_store(
+    #     datastore: str,
+    #     jndi_reference: str,
+    #     namespace: str,
+    #     pg_schema: str = "public",
+    #     description: str | None = None,
+    # ) -> dict[str, dict[str, Any]]:
+    #     return {
+    #         "dataStore": {
+    #             "name": datastore,
+    #             "description": description,
+    #             "connectionParameters": {
+    #                 "entry": [
+    #                     {"@key": "dbtype", "$": "postgis"},
+    #                     {
+    #                         "@key": "jndiReferenceName",
+    #                         "$": jndi_reference,
+    #                     },
+    #                     {
+    #                         "@key": "schema",
+    #                         "$": pg_schema,
+    #                     },
+    #                     {
+    #                         "@key": "namespace",
+    #                         "$": namespace,
+    #                     },
+    #                     {"@key": "Expose primary keys", "$": "true"},
+    #                 ]
+    #             },
+    #         }
+    #     }
 
     @staticmethod
     def wmts_store(
@@ -173,6 +174,8 @@ def geom_point_attribute() -> dict[str, Any]:
             }
         }
 
+    # TODO: remove this template after finishing to merge
+    # BBOX stuff in the FeatureType class
     @staticmethod
     def feature_type(
         layer: str,

tests/models/test_common.py

@@ -2,13 +2,12 @@
 
 import pytest
 
-from geoservercloud.models import (  # Adjust import based on your module location
+from geoservercloud.models import (
     I18N,
     KeyDollarListDict,
 )
 
 
-# Test initialization with input_list (deserialization)
 def test_keydollarlistdict_initialization_with_input_list():
     input_list = [{"@key": "host", "$": "localhost"}, {"@key": "port", "$": "5432"}]
 
@@ -18,27 +17,12 @@ def test_keydollarlistdict_initialization_with_input_list():
     assert kdl_dict["port"] == "5432"
 
 
-# Test initialization without input_list
 def test_keydollarlistdict_initialization_without_input_list():
     kdl_dict = KeyDollarListDict()
 
-    assert len(kdl_dict) == 0  # Should be an empty dictionary
+    assert kdl_dict == {}
 
 
-# Test deserialization of input_list
-def test_keydollarlistdict_deserialization():
-    input_list = [
-        {"@key": "username", "$": "admin"},
-        {"@key": "password", "$": "secret"},
-    ]
-
-    kdl_dict = KeyDollarListDict(input_list)
-
-    assert kdl_dict["username"] == "admin"
-    assert kdl_dict["password"] == "secret"
-
-
-# Test serialization method
 def test_keydollarlistdict_serialization():
     kdl_dict = KeyDollarListDict()
     kdl_dict["host"] = "localhost"
@@ -52,7 +36,6 @@ def test_keydollarlistdict_serialization():
     assert kdl_dict.serialize() == expected_output
 
 
-# Test __repr__ method
 def test_keydollarlistdict_repr():
     kdl_dict = KeyDollarListDict([{"@key": "db", "$": "postgres"}])
 
@@ -61,7 +44,6 @@ def test_keydollarlistdict_repr():
     assert repr(kdl_dict) == expected_repr
 
 
-# Test __str__ method
 def test_keydollarlistdict_str():
     kdl_dict = KeyDollarListDict([{"@key": "db", "$": "postgres"}])
 
@@ -70,7 +52,6 @@ def test_keydollarlistdict_str():
     assert str(kdl_dict) == expected_str
 
 
-# Test initialization with a string value
 def test_i18n_initialization_string():
     keys = ("title", "internationalizedTitle")
     value = "Test Title"
@@ -80,10 +61,9 @@ def test_i18n_initialization_string():
     assert i18n_instance.str_key == "title"
     assert i18n_instance.i18n_key == "internationalizedTitle"
     assert i18n_instance.value == value
-    assert i18n_instance._payload == ("title", value)
+    assert i18n_instance.asdict() == {"title": value}
 
 
-# Test initialization with a dictionary value (internationalized)
 def test_i18n_initialization_dict():
     keys = ("abstract", "internationalizedAbstract")
     value = {"en": "Test Abstract", "fr": "Résumé Test"}
@@ -93,53 +73,17 @@ def test_i18n_initialization_dict():
     assert i18n_instance.str_key == "abstract"
     assert i18n_instance.i18n_key == "internationalizedAbstract"
     assert i18n_instance.value == value
-    assert i18n_instance._payload == ("internationalizedAbstract", value)
+    assert i18n_instance.asdict() == {"internationalizedAbstract": value}
 
 
-# Test invalid value type (neither string nor dictionary)
 def test_i18n_invalid_value_type():
     keys = ("title", "internationalizedTitle")
-    value = 123  # Invalid value type
+    value = 123
 
     with pytest.raises(ValueError, match="Invalid value type"):
         I18N(keys, value)
 
 
-# Test str_key property
-def test_i18n_str_key_property():
-    keys = ("title", "internationalizedTitle")
-    value = "Test Title"
-    i18n_instance = I18N(keys, value)
-
-    assert i18n_instance.str_key == "title"
-
-
-# Test i18n_key property
-def test_i18n_i18n_key_property():
-    keys = ("abstract", "internationalizedAbstract")
-    value = {"en": "Test Abstract", "fr": "Résumé Test"}
-    i18n_instance = I18N(keys, value)
-
-    assert i18n_instance.i18n_key == "internationalizedAbstract"
-
-
-# Test value property
-def test_i18n_value_property():
-    keys = ("title", "internationalizedTitle")
-    value = "Test Title"
-    i18n_instance = I18N(keys, value)
-
-    assert i18n_instance.value == "Test Title"
-
-
-def test_i18n_payload_tuple_property():
-    keys = ("title", "internationalizedTitle")
-    value = "Test Title"
-    i18n_instance = I18N(keys, value)
-
-    assert i18n_instance.payload_tuple == ("title", "Test Title")
-
-
 def test_i18n_repr():
     keys = ("title", "internationalizedTitle")
     value = "Test Title"