From 614eefc393344019fc9136b3f803ae481b254ea3 Mon Sep 17 00:00:00 2001 From: izzy lyseggen Date: Thu, 18 Mar 2021 18:45:24 +0000 Subject: [PATCH] fix(serialisation): EMBARASSING BUG so i'm a bit of a dumbo here. i didn't realise that doing `.update()` on attr would update the parent attr as well and extend to all objects every ahhhhhh you have to do `self.thing = self.thing + blah blah` to just update the instance attr. the more ya know! #roastme @cristi8 --- speckle/objects/base.py | 4 +-- speckle/objects/fakemesh.py | 8 +++--- speckle/objects/geometry.py | 28 ++++++++++--------- .../serialization/base_object_serializer.py | 3 +- tests/test_serialization.py | 12 ++++---- 5 files changed, 30 insertions(+), 25 deletions(-) diff --git a/speckle/objects/base.py b/speckle/objects/base.py index b95d6c3..98ee528 100644 --- a/speckle/objects/base.py +++ b/speckle/objects/base.py @@ -1,7 +1,7 @@ from inspect import getattr_static from pydantic import BaseModel, validator from pydantic.main import Extra -from typing import ClassVar, Dict, List, Optional, Any, Type +from typing import ClassVar, Dict, List, Optional, Any, Set, Type from speckle.transports.memory import MemoryTransport from speckle.logging.exceptions import SpeckleException from speckle.objects.units import get_units_from_string @@ -61,7 +61,7 @@ class Base(_RegisteringBase): _units: str = "m" _chunkable: Dict[str, int] = {} # dict of chunkable props and their max chunk size _chunk_size_default: int = 1000 - _detachable: List[str] = [] # list of defined detachable props + _detachable: Set[str] = set() # list of defined detachable props def __repr__(self) -> str: return ( diff --git a/speckle/objects/fakemesh.py b/speckle/objects/fakemesh.py index a273ba9..9c414a0 100644 --- a/speckle/objects/fakemesh.py +++ b/speckle/objects/fakemesh.py @@ -11,7 +11,7 @@ CHUNKABLE_PROPS = { "test_bases": 10, } -DETACHABLE = ["detach_this", "origin", "detached_list"] +DETACHABLE = {"detach_this", "origin", "detached_list"} class FakeMesh(Base): @@ -25,9 +25,9 @@ class FakeMesh(Base): _origin: Point = None def __init__(self, **kwargs) -> None: - super().__init__(**kwargs) - self._chunkable.update(CHUNKABLE_PROPS) - self._detachable.extend(DETACHABLE) + super(FakeMesh, self).__init__(**kwargs) + self._chunkable = dict(self._chunkable, **CHUNKABLE_PROPS) + self._detachable = self._detachable.union(DETACHABLE) @property def origin(self): diff --git a/speckle/objects/geometry.py b/speckle/objects/geometry.py index 83ae439..0101932 100644 --- a/speckle/objects/geometry.py +++ b/speckle/objects/geometry.py @@ -102,7 +102,7 @@ class Polyline(Base, speckle_type=GEOMETRY + "Polyline"): def __init__(self, **data: Any) -> None: super().__init__(**data) - self._chunkable.update({"value": 20000}) + self._chunkable = dict(self._chunkable, value=20000) @classmethod def from_points(cls, points: List[Point]): @@ -150,7 +150,9 @@ class Curve(Base, speckle_type=GEOMETRY + "Curve"): def __init__(self, **data: Any) -> None: super().__init__(**data) - self._chunkable.update({"points": 20000, "weights": 20000, "knots": 20000}) + self._chunkable = dict( + self._chunkable, points=20000, weights=20000, knots=20000 + ) def as_points(self) -> List[Point]: """Converts the `value` attribute to a list of Points""" @@ -198,13 +200,12 @@ class Mesh(Base, speckle_type=GEOMETRY + "Mesh"): def __init__(self, **data) -> None: super().__init__(**data) - self._chunkable.update( - { - "vertices": 2000, - "faces": 2000, - "colors": 2000, - "textureCoordinates": 2000, - } + self._chunkable = dict( + self._chunkable, + vertices=2000, + faces=2000, + colors=2000, + textureCoordinates=2000, ) @@ -329,9 +330,10 @@ class Brep(Base, speckle_type=GEOMETRY + "Brep"): def __init__(self, **data: Any) -> None: super().__init__(**data) - self._detachable.append("displayValue") - self._chunkable.update( - { + self._detachable.update({"displayValue"}) + self._chunkable = dict( + self._chunkable, + **{ "Surfaces": 200, "Curve3D": 200, "Curve2D": 200, @@ -340,7 +342,7 @@ class Brep(Base, speckle_type=GEOMETRY + "Brep"): "Loops": 5000, "Trims": 5000, "Faces": 5000, - } + }, ) def __setattr__(self, name: str, value: Any) -> None: diff --git a/speckle/serialization/base_object_serializer.py b/speckle/serialization/base_object_serializer.py index db5b17d..b0f739a 100644 --- a/speckle/serialization/base_object_serializer.py +++ b/speckle/serialization/base_object_serializer.py @@ -127,7 +127,8 @@ class BaseObjectSerializer: hash = hash_obj(object_builder) object_builder["id"] = hash - object_builder["__closure"] = self.closure_table[hash] = closure + if closure: + object_builder["__closure"] = self.closure_table[hash] = closure # write detached or root objects to transports if detached and self.write_transports: diff --git a/tests/test_serialization.py b/tests/test_serialization.py index 307647c..f5657a9 100644 --- a/tests/test_serialization.py +++ b/tests/test_serialization.py @@ -1,4 +1,5 @@ import json +from attr import has import pytest from speckle.api import operations from speckle.transports.server import ServerTransport @@ -32,7 +33,7 @@ class TestSerialization: assert serialized_dict["@detach"]["speckle_type"] == "reference" assert serialized_dict["origin"]["speckle_type"] == "reference" assert serialized_dict["@detached_list"][-1]["speckle_type"] == "reference" - assert mesh.get_id(True) == deserialized.get_id() + assert mesh.get_id() == deserialized.get_id() def test_chunking(self, mesh): transport = MemoryTransport() @@ -48,7 +49,7 @@ class TestSerialization: assert serialized_dict["vertices"][0]["speckle_type"] == "reference" assert serialized_dict["@(100)colours"][0]["speckle_type"] == "reference" assert serialized_dict["@()default_chunk"][0]["speckle_type"] == "reference" - assert mesh.get_id(True) == deserialized.get_id() + assert mesh.get_id() == deserialized.get_id() def test_send_and_receive(self, client, sample_stream, mesh): transport = ServerTransport(client=client, stream_id=sample_stream.id) @@ -62,15 +63,16 @@ class TestSerialization: assert received.vertices == mesh.vertices assert isinstance(received.origin, Point) assert received.origin.value == mesh.origin.value - assert mesh.get_id(True) == received.get_id() + # not comparing hashes as order is not guaranteed back from server mesh.id = hash # populate with decomposed id for use in proceeding tests def test_receive_local(self, client, mesh): - received = operations.receive(mesh.id) # defaults to SQLiteTransport + hash = operations.send(mesh) # defaults to SQLiteTransport + received = operations.receive(hash) assert isinstance(received, Base) - assert mesh.get_id(True) == received.get_id() + assert mesh.get_id() == received.get_id() def test_unknown_type(self): unknown = '{"speckle_type": "mysterious.type"}'