fix an issue residing in the default setup for system resources where the properties were not honored due to only accepting the default value for the type property

Author: tipatterson-dev

docs/source/architecture/construction.md

@@ -237,7 +237,7 @@ with open('my_app_config.json') as f:
 node = Node.from_storage_dict(cfg['nodes'][0])
 for sys_dict in cfg['systems']:
     sys = System.from_storage_dict(sys_dict, node)
-    node.add_new_system(sys)
+    node.add_system(sys)
 ```
 
 ## What about new datastreams/controlstreams without going through System?

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "oshconnect"
-version = "0.5.1a5"
+version = "0.5.1a7"
 description = "Library for interfacing with OSH, helping guide visualization efforts, and providing a place to store configurations. Implements OGC CS API Part 3 (Pub/Sub) MQTT topic conventions including :data topics and resource event topics."
 readme = "README.md"
 authors = [

src/oshconnect/oshconnectapi.py

@@ -303,9 +303,7 @@ def add_system_to_node(self, system: System, target_node: Node, insert_resource:
         :return:
         """
         if target_node in self._nodes:
-            target_node.add_new_system(system)
-            if insert_resource:
-                system.insert_self()
+            target_node.add_system(system, insert_resource=insert_resource)
             self._systems.append(system)
             return
 

src/oshconnect/resource_datamodels.py

@@ -139,11 +139,19 @@ class SystemResource(BaseModel):
     def to_smljson_dict(self) -> dict:
         """Render this system as an `application/sml+json` dict (SensorML JSON encoding).
 
-        Sets ``feature_type = "PhysicalSystem"`` to match the SML discriminator
-        before dumping. Output keys are camelCase per the CS API wire format.
+        The ``type`` discriminator (``PhysicalSystem``,
+        ``PhysicalComponent``, ``SimpleProcess``, ``AggregateProcess``,
+        etc.) is preserved from ``self.feature_type`` when set —
+        important for cross-node sync, where the source's SML kind
+        determines how OSH surfaces ``featureType`` (e.g. ``Sensor``
+        vs. ``System``). Defaults to ``"PhysicalSystem"`` only when
+        ``feature_type`` is unset, so callers building a bare
+        ``SystemResource`` still get a valid SML body. Does not
+        mutate ``self``.
         """
-        self.feature_type = "PhysicalSystem"
-        return self.model_dump(by_alias=True, exclude_none=True, mode='json')
+        dumped = self.model_dump(by_alias=True, exclude_none=True, mode='json')
+        dumped.setdefault("type", "PhysicalSystem")
+        return dumped
 
     def to_smljson(self) -> str:
         """JSON-string variant of `to_smljson_dict`."""
@@ -152,12 +160,14 @@ def to_smljson(self) -> str:
     def to_geojson_dict(self) -> dict:
         """Render this system as an `application/geo+json` dict.
 
-        Sets ``feature_type = "Feature"`` to match the GeoJSON discriminator
-        before dumping. Useful when posting to endpoints that expect the
-        GeoJSON Feature shape.
+        The ``type`` field is always set to ``"Feature"`` per the
+        GeoJSON spec, regardless of ``self.feature_type`` — that's the
+        whole point of this rendering variant. Does not mutate
+        ``self``.
         """
-        self.feature_type = "Feature"
-        return self.model_dump(by_alias=True, exclude_none=True, mode='json')
+        dumped = self.model_dump(by_alias=True, exclude_none=True, mode='json')
+        dumped["type"] = "Feature"
+        return dumped
 
     def to_geojson(self) -> str:
         """JSON-string variant of `to_geojson_dict`."""

src/oshconnect/streamableresource.py

@@ -311,31 +311,32 @@ def discover_systems(self) -> list[System] | None:
         else:
             return None
 
-    def add_new_system(self, system: System):
-        """Attach a system to this node without inserting it server-side.
-
-        Use `add_system(system, insert_resource=True)` if you also want to
-        POST it to the server.
-        """
-        system.set_parent_node(self)
-        self._systems.append(system)
-
     def get_api_helper(self) -> APIHelper:
         """Return the `APIHelper` this node uses for HTTP calls."""
         return self._api_helper
 
     # System Management
 
-    def add_system(self, system: System, insert_resource: bool = False):
-        """
-        Add a system to the target node.
-        :param system: System object
-        :param insert_resource: Whether to insert the system into the target node's server, default is False
-        :return:
+    def add_system(self, system: System, insert_resource: bool = False) -> System:
+        """Attach a system to this node.
+
+        When ``insert_resource=True``, the system is first POSTed to the
+        server via ``system.insert_self()`` (which populates its
+        server-assigned resource id), then attached locally — so the
+        system enters this node's collection already carrying its real
+        id. With ``insert_resource=False`` the system is attached
+        in-memory only; useful when reconstructing state from a
+        datastore or staging a system before a deferred POST.
+
+        :param system: ``System`` object to attach.
+        :param insert_resource: Whether to POST the system to the
+            server before attaching it locally.
+        :return: The same ``System`` (now parented to this node and
+            tracked in ``self.systems()``).
         """
         if insert_resource:
             system.insert_self()
-        self.add_new_system(system)
+        system.set_parent_node(self)
         self._systems.append(system)
         return system
 
@@ -1103,10 +1104,35 @@ def from_system_resource(system_resource: SystemResource, parent_node: Node) ->
 
     def to_system_resource(self) -> SystemResource:
         """Render this `System` as a `SystemResource` pydantic model
-        suitable for POSTing to the server. Wrapper-specific: assembles
-        attached datastreams into the resource's ``outputs`` list.
+        suitable for POSTing to the server.
+
+        When this wrapper already carries an ``_underlying_resource``
+        (e.g. populated by ``from_csapi_dict``, ``set_system_resource``,
+        or a prior ``retrieve_resource`` call), all of its fields are
+        preserved into a deep copy — so cross-node sync, partial
+        updates, and re-POSTs round-trip everything the source carried,
+        not just ``uniqueId`` / ``label`` / a hardcoded
+        ``PhysicalSystem`` type. Currently-attached datastreams are
+        always reflected into ``outputs`` so newly-added children come
+        along.
+
+        When no underlying resource is present (i.e. during this
+        wrapper's own ``__init__``), a thin shell is built from
+        wrapper attrs and the SML type defaults to ``PhysicalSystem``.
         """
-        resource = SystemResource(uid=self.urn, label=self.name, feature_type='PhysicalSystem')
+        underlying = getattr(self, '_underlying_resource', None)
+        if underlying is not None:
+            resource = underlying.model_copy(deep=True)
+            # Pick up any wrapper-side updates the user made directly
+            # on the System (the wrapper doesn't proxy these into the
+            # resource on assignment).
+            if self.urn and not resource.uid:
+                resource.uid = self.urn
+            if self.name and not resource.label:
+                resource.label = self.name
+        else:
+            resource = SystemResource(uid=self.urn, label=self.name,
+                                      feature_type='PhysicalSystem')
         if self.datastreams:
             resource.outputs = [ds.get_underlying_resource() for ds in self.datastreams]
         return resource
@@ -1300,16 +1326,26 @@ def insert_self(self):
         """POST this system to the server (Content-Type
         ``application/sml+json``) and capture the new resource ID from
         the ``Location`` response header.
+
+        Server-assigned fields (``id``, ``links``) are stripped from
+        the body before POST so a re-POSTed (e.g. cross-node-synced)
+        system doesn't leak the source server's identifier or links to
+        the destination — the destination assigns its own.
         """
+        body_resource = self.to_system_resource().model_copy(deep=True)
+        body_resource.system_id = None
+        body_resource.links = None
         res = self._parent_node.get_api_helper().create_resource(
             APIResourceTypes.SYSTEM,
-            self.to_system_resource().model_dump_json(by_alias=True, exclude_none=True),
+            body_resource.model_dump_json(by_alias=True, exclude_none=True),
             req_headers={'Content-Type': 'application/sml+json'})
 
         if res.ok:
             location = res.headers['Location']
             sys_id = location.split('/')[-1]
             self._resource_id = sys_id
+            if self._underlying_resource is not None:
+                self._underlying_resource.system_id = sys_id
             print(f'Created system: {self._resource_id}')
 
     def retrieve_resource(self):

tests/test_csapi_serialization.py

@@ -149,6 +149,151 @@ def test_system_full_chain_geojson_dict_to_resource_to_wrapper(node):
     assert sys.name == "GeoSys2"
 
 
+# ---------------------------------------------------------------------------
+# SML type preservation and non-mutation
+# ---------------------------------------------------------------------------
+
+def test_to_smljson_preserves_non_default_feature_type():
+    """A source whose SML type is ``PhysicalComponent`` (which OSH
+    surfaces as ``featureType: Sensor``) must round-trip through
+    ``to_smljson_dict`` without being collapsed back to
+    ``PhysicalSystem``. Regression guard for cross-node sync."""
+    src = SystemResource(uid="urn:test:s1", label="S1",
+                         feature_type="PhysicalComponent")
+    dumped = src.to_smljson_dict()
+    assert dumped["type"] == "PhysicalComponent"
+
+
+def test_to_smljson_defaults_to_physical_system_when_unset():
+    """When ``feature_type`` is unset, the SML body still gets a
+    sensible default so callers building a bare SystemResource
+    continue to produce a valid SML body."""
+    src = SystemResource(uid="urn:test:s1", label="S1")
+    dumped = src.to_smljson_dict()
+    assert dumped["type"] == "PhysicalSystem"
+
+
+def test_to_smljson_does_not_mutate_feature_type():
+    """Pre-fix, ``to_smljson_dict`` set ``self.feature_type`` as a
+    side effect, which clobbered the source's SML kind. After the
+    fix, the model is untouched."""
+    src = SystemResource(uid="urn:test:s1", label="S1",
+                         feature_type="PhysicalComponent")
+    src.to_smljson_dict()
+    assert src.feature_type == "PhysicalComponent"
+
+
+def test_to_geojson_always_emits_feature_without_mutating():
+    """GeoJSON form requires ``type: Feature`` per spec, regardless
+    of ``feature_type`` on the model. The model itself stays
+    unmutated."""
+    src = SystemResource(uid="urn:test:s1", label="S1",
+                         feature_type="PhysicalComponent")
+    dumped = src.to_geojson_dict()
+    assert dumped["type"] == "Feature"
+    assert src.feature_type == "PhysicalComponent"
+
+
+# ---------------------------------------------------------------------------
+# System.to_system_resource preserves _underlying_resource
+# ---------------------------------------------------------------------------
+
+def test_to_system_resource_preserves_full_underlying(node):
+    """When the wrapper carries a full ``_underlying_resource`` (e.g.,
+    populated by discovery / ``from_csapi_dict``), the resource
+    rendered for POST keeps every field — not just uid/label/type."""
+    raw = {
+        "type": "PhysicalComponent",
+        "id": "src-server-id-abc",
+        "uniqueId": "urn:test:source:1",
+        "label": "Source Sensor",
+        "description": "Original description",
+        "definition": "http://www.opengis.net/def/system",
+        "keywords": ["thermal", "imaging"],
+    }
+    res = SystemResource.from_smljson_dict(raw)
+    sys = System.from_resource(res, node)
+
+    rendered = sys.to_system_resource()
+
+    # Type preserved (was hardcoded to PhysicalSystem pre-fix).
+    assert rendered.feature_type == "PhysicalComponent"
+    # Other fields preserved (were silently dropped pre-fix).
+    assert rendered.description == "Original description"
+    assert rendered.definition == "http://www.opengis.net/def/system"
+    assert rendered.keywords == ["thermal", "imaging"]
+
+
+def test_to_system_resource_thin_shell_for_freshly_constructed(node):
+    """A System constructed from scratch (no parsed resource) still
+    produces a sensible thin shell with default ``PhysicalSystem``
+    type — backward-compat with code that doesn't go through
+    discovery."""
+    sys = System(name="Fresh", label="Fresh", urn="urn:test:fresh:1",
+                 parent_node=node)
+    rendered = sys.to_system_resource()
+    assert rendered.feature_type == "PhysicalSystem"
+    assert rendered.uid == "urn:test:fresh:1"
+
+
+# ---------------------------------------------------------------------------
+# insert_self strips server-assigned fields from the POST body
+# ---------------------------------------------------------------------------
+
+class _MockResponse:
+    status_code = 201
+    ok = True
+    text = ""
+    headers = {"Location": "http://localhost:8282/sensorhub/api/systems/dest-id-xyz"}
+
+
+def _capture_post(into: dict):
+    def _f(url, params=None, headers=None, auth=None, data=None, json=None, **kwargs):
+        into["url"] = str(url)
+        into["data"] = data
+        into["json"] = json
+        return _MockResponse()
+    return _f
+
+
+def test_insert_self_strips_id_and_links_from_body(node, monkeypatch):
+    """When re-POSTing a discovered system to a destination node, the
+    source's server-assigned ``id`` and ``links`` must not leak into
+    the body — the destination assigns its own. Regression guard for
+    cross-node sync."""
+    raw = {
+        "type": "PhysicalComponent",
+        "id": "source-side-id",
+        "uniqueId": "urn:test:source:1",
+        "label": "Source Sensor",
+        "links": [{"href": "http://source.example/extra", "rel": "alternate"}],
+    }
+    res = SystemResource.from_smljson_dict(raw)
+    sys = System.from_resource(res, node)
+
+    captured: dict = {}
+    monkeypatch.setattr(
+        "oshconnect.csapi4py.request_wrappers.requests.post",
+        _capture_post(captured),
+    )
+
+    sys.insert_self()
+
+    body = json.loads(captured["data"])
+    # Source-assigned identifiers must NOT be present in the POST body.
+    assert "id" not in body, (
+        "POST body must not carry source's server-assigned id"
+    )
+    assert "links" not in body, (
+        "POST body must not carry source's server-assigned links"
+    )
+    # But the SML kind from the source IS preserved.
+    assert body["type"] == "PhysicalComponent"
+    assert body["uniqueId"] == "urn:test:source:1"
+    # Wrapper picked up the destination's id from the Location header.
+    assert sys._resource_id == "dest-id-xyz"
+
+
 # ===========================================================================
 # Datastream: resource representation, schema document, observations
 # ===========================================================================

tests/test_datastore.py

@@ -255,7 +255,7 @@ def test_save_all_and_load_all(self):
         sm = SessionManager()
         node = make_node(sm)
         system = make_system(node)
-        node.add_new_system(system)
+        node.add_system(system)
 
         store.save_all([node])
         nodes = store.load_all(session_manager=sm)