-
-
Notifications
You must be signed in to change notification settings - Fork 7
solution of issue #9 #124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
solution of issue #9 #124
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
|
||
""" | ||
adopted from pyro - https://github.com/irmen/Pyro5 - see following license | ||
|
@@ -59,40 +60,15 @@ | |
|
||
|
||
class BaseSerializer(object): | ||
""" | ||
Base class for (de)serializer implementations. All serializers must inherit this class | ||
and overload dumps() and loads() to be usable by the ZMQ message brokers. Any serializer | ||
that returns bytes when serialized and a python object on deserialization will be accepted. | ||
Serialization and deserialization errors will be passed as invalid message type | ||
(see ZMQ messaging contract) from server side and a exception will be raised on the client. | ||
""" | ||
|
||
def __init__(self) -> None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we undo this code deletion? This is supposed to be the Abstract Base Class for a Serializer. (May be we promote it The type definitions picked up by the code editor for |
||
super().__init__() | ||
self.type = None | ||
|
||
def loads(self, data) -> typing.Any: | ||
"method called by ZMQ message brokers to deserialize data" | ||
raise NotImplementedError("implement loads()/deserialization in subclass") | ||
|
||
def dumps(self, data) -> bytes: | ||
"method called by ZMQ message brokers to serialize data" | ||
raise NotImplementedError("implement dumps()/serialization in subclass") | ||
|
||
def convert_to_bytes(self, data) -> bytes: | ||
@staticmethod | ||
def convert_to_bytes(data): | ||
if isinstance(data, bytes): | ||
return data | ||
if isinstance(data, bytearray): | ||
if isinstance(data, (bytearray, memoryview)): | ||
return bytes(data) | ||
if isinstance(data, memoryview): | ||
return data.tobytes() | ||
raise TypeError( | ||
"serializer convert_to_bytes accepts only bytes, bytearray or memoryview, not type {}".format(type(data)) | ||
) | ||
|
||
@property | ||
def content_type(self) -> str: | ||
raise NotImplementedError("serializer must implement a content type") | ||
if isinstance(data, str): | ||
return data.encode("utf-8") | ||
raise TypeError(f"Cannot convert type {type(data)} to bytes") | ||
|
||
|
||
dict_keys = type(dict().keys()) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,128 @@ | |
from sqlalchemy import Integer, String, JSON, LargeBinary | ||
from sqlalchemy.orm import Mapped, mapped_column, DeclarativeBase, MappedAsDataclass | ||
from sqlite3 import DatabaseError | ||
from pymongo import MongoClient, errors as mongo_errors | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add |
||
from ..param import Parameterized | ||
from ..core.property import Property | ||
class MongoThingDB: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code formatting could be improved here, I would prefer if this class goes downwards towards the end of the file and there are spaces between imports and code definitions. |
||
""" | ||
MongoDB-backed database engine for Thing properties and info. | ||
This class provides persistence for Thing properties using MongoDB. | ||
Properties are stored in the 'properties' collection, with fields: | ||
- id: Thing instance identifier | ||
- name: property name | ||
- serialized_value: serialized property value | ||
Methods mirror the interface of ThingDB for compatibility. | ||
""" | ||
def __init__(self, instance: Parameterized, config_file: typing.Union[str, None] = None) -> None: | ||
""" | ||
Initialize MongoThingDB for a Thing instance. | ||
Connects to MongoDB and sets up collections. | ||
""" | ||
self.thing_instance = instance | ||
self.id = instance.id | ||
self.config = self.load_conf(config_file) | ||
self.client = MongoClient(self.config.get("mongo_uri", "mongodb://localhost:27017")) | ||
self.db = self.client[self.config.get("database", "hololinked")] | ||
self.properties = self.db["properties"] | ||
self.things = self.db["things"] | ||
|
||
@classmethod | ||
def load_conf(cls, config_file: str) -> typing.Dict[str, typing.Any]: | ||
""" | ||
Load configuration from JSON file if provided. | ||
""" | ||
if not config_file: | ||
return {} | ||
elif config_file.endswith(".json"): | ||
with open(config_file, "r") as file: | ||
return JSONSerializer.load(file) | ||
else: | ||
raise ValueError(f"config files of extension - ['json'] expected, given file name {config_file}") | ||
|
||
def fetch_own_info(self): | ||
""" | ||
Fetch Thing instance metadata from the 'things' collection. | ||
""" | ||
doc = self.things.find_one({"id": self.id}) | ||
return doc | ||
|
||
def get_property(self, property: typing.Union[str, Property], deserialized: bool = True) -> typing.Any: | ||
""" | ||
Get a property value from MongoDB for this Thing. | ||
If deserialized=True, returns the Python value. | ||
""" | ||
name = property if isinstance(property, str) else property.name | ||
doc = self.properties.find_one({"id": self.id, "name": name}) | ||
if not doc: | ||
raise mongo_errors.PyMongoError(f"property {name} not found in database") | ||
if not deserialized: | ||
return doc | ||
import base64, pickle | ||
return pickle.loads(base64.b64decode(doc["serialized_value"])) | ||
|
||
def set_property(self, property: typing.Union[str, Property], value: typing.Any) -> None: | ||
""" | ||
Set a property value in MongoDB for this Thing. | ||
Value is serialized before storage. | ||
""" | ||
name = property if isinstance(property, str) else property.name | ||
import base64, pickle | ||
serialized_value = base64.b64encode(pickle.dumps(value)).decode("utf-8") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The serializer can be defined as serializer = Serializers.for_object(self.id, self.thing_instance.__class__.__name__, name)
serialized_value = serializer.dumps(value) You can see this code in In this way, we dont have to always use pickle, because pickle usage must be inherently discouraged as its not a safe serializer due to possibility of arbitrary code execution. This may not true for the specific use case here, but must be followed as a general practice. The The base64 import can go to the top of file with all other imports. |
||
self.properties.update_one( | ||
{"id": self.id, "name": name}, | ||
{"$set": {"serialized_value": serialized_value}}, | ||
upsert=True | ||
) | ||
|
||
def get_properties(self, properties: typing.Dict[typing.Union[str, Property], typing.Any], deserialized: bool = True) -> typing.Dict[str, typing.Any]: | ||
""" | ||
Get multiple property values from MongoDB for this Thing. | ||
Returns a dict of property names to values. | ||
""" | ||
names = [obj if isinstance(obj, str) else obj.name for obj in properties.keys()] | ||
cursor = self.properties.find({"id": self.id, "name": {"$in": names}}) | ||
result = {} | ||
import base64, pickle | ||
for doc in cursor: | ||
result[doc["name"]] = doc["serialized_value"] if not deserialized else pickle.loads(base64.b64decode(doc["serialized_value"])) | ||
return result | ||
|
||
def set_properties(self, properties: typing.Dict[typing.Union[str, Property], typing.Any]) -> None: | ||
""" | ||
Set multiple property values in MongoDB for this Thing. | ||
""" | ||
for obj, value in properties.items(): | ||
name = obj if isinstance(obj, str) else obj.name | ||
import base64, pickle | ||
serialized_value = base64.b64encode(pickle.dumps(value)).decode("utf-8") | ||
self.properties.update_one( | ||
{"id": self.id, "name": name}, | ||
{"$set": {"serialized_value": serialized_value}}, | ||
upsert=True | ||
) | ||
|
||
def get_all_properties(self, deserialized: bool = True) -> typing.Dict[str, typing.Any]: | ||
cursor = self.properties.find({"id": self.id}) | ||
result = {} | ||
import base64, pickle | ||
for doc in cursor: | ||
result[doc["name"]] = doc["serialized_value"] if not deserialized else pickle.loads(base64.b64decode(doc["serialized_value"])) | ||
return result | ||
|
||
def create_missing_properties(self, properties: typing.Dict[str, Property], get_missing_property_names: bool = False) -> typing.Any: | ||
missing_props = [] | ||
existing_props = self.get_all_properties() | ||
import base64, pickle | ||
for name, new_prop in properties.items(): | ||
if name not in existing_props: | ||
serialized_value = base64.b64encode(pickle.dumps(getattr(self.thing_instance, new_prop.name))).decode("utf-8") | ||
self.properties.insert_one({"id": self.id, "name": new_prop.name, "serialized_value": serialized_value}) | ||
missing_props.append(name) | ||
if get_missing_property_names: | ||
return missing_props | ||
from dataclasses import dataclass | ||
|
||
from ..param import Parameterized | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,112 @@ | |
|
||
class TestProperty(TestCase): | ||
@classmethod | ||
def setUpClass(cls): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests are working as expected. Thank you for this. Currently I realised I cannot integrate it in the CI/CD because of a lacking mongo instance. I have to think a little bit about how to set this up in a right way in github actions with long terms goals in mind. So these tests can be placed in a file In the current state, there is also a bug that the Once you push all the requested changes, we can run the pipeline (which will most likely pass) abd merge it. Thanks for the work again. |
||
# Clear MongoDB 'properties' collection before tests | ||
try: | ||
from pymongo import MongoClient | ||
client = MongoClient("mongodb://localhost:27017") | ||
db = client["hololinked"] | ||
db["properties"].delete_many({}) | ||
except Exception as e: | ||
print(f"Warning: Could not clear MongoDB test data: {e}") | ||
def test_mongo_string_property(self): | ||
from hololinked.core.property import Property | ||
from hololinked.core import Thing | ||
|
||
class MongoTestThing(Thing): | ||
str_prop = Property(default="hello", db_persist=True) | ||
|
||
instance = MongoTestThing(id="mongo_str", use_mongo_db=True) | ||
instance.str_prop = "world" | ||
value_from_db = instance.db_engine.get_property("str_prop") | ||
self.assertEqual(value_from_db, "world") | ||
|
||
def test_mongo_float_property(self): | ||
from hololinked.core.property import Property | ||
from hololinked.core import Thing | ||
|
||
class MongoTestThing(Thing): | ||
float_prop = Property(default=1.23, db_persist=True) | ||
|
||
instance = MongoTestThing(id="mongo_float", use_mongo_db=True) | ||
instance.float_prop = 4.56 | ||
value_from_db = instance.db_engine.get_property("float_prop") | ||
self.assertAlmostEqual(value_from_db, 4.56) | ||
|
||
def test_mongo_bool_property(self): | ||
from hololinked.core.property import Property | ||
from hololinked.core import Thing | ||
|
||
class MongoTestThing(Thing): | ||
bool_prop = Property(default=False, db_persist=True) | ||
|
||
instance = MongoTestThing(id="mongo_bool", use_mongo_db=True) | ||
instance.bool_prop = True | ||
value_from_db = instance.db_engine.get_property("bool_prop") | ||
self.assertTrue(value_from_db) | ||
|
||
def test_mongo_dict_property(self): | ||
from hololinked.core.property import Property | ||
from hololinked.core import Thing | ||
|
||
class MongoTestThing(Thing): | ||
dict_prop = Property(default={"a": 1}, db_persist=True) | ||
|
||
instance = MongoTestThing(id="mongo_dict", use_mongo_db=True) | ||
instance.dict_prop = {"b": 2, "c": 3} | ||
value_from_db = instance.db_engine.get_property("dict_prop") | ||
self.assertEqual(value_from_db, {"b": 2, "c": 3}) | ||
|
||
def test_mongo_list_property(self): | ||
from hololinked.core.property import Property | ||
from hololinked.core import Thing | ||
|
||
class MongoTestThing(Thing): | ||
list_prop = Property(default=[1, 2], db_persist=True) | ||
|
||
instance = MongoTestThing(id="mongo_list", use_mongo_db=True) | ||
instance.list_prop = [3, 4, 5] | ||
value_from_db = instance.db_engine.get_property("list_prop") | ||
self.assertEqual(value_from_db, [3, 4, 5]) | ||
|
||
def test_mongo_none_property(self): | ||
from hololinked.core.property import Property | ||
from hololinked.core import Thing | ||
|
||
class MongoTestThing(Thing): | ||
none_prop = Property(default=None, db_persist=True, allow_None=True) | ||
|
||
instance = MongoTestThing(id="mongo_none", use_mongo_db=True) | ||
instance.none_prop = None | ||
value_from_db = instance.db_engine.get_property("none_prop") | ||
self.assertIsNone(value_from_db) | ||
def test_mongo_property_persistence(self): | ||
"""Test property persistence using MongoDB backend""" | ||
from hololinked.core.property import Property | ||
from hololinked.core import Thing | ||
from pymongo import MongoClient | ||
|
||
# Use a unique Thing ID and property name for each run | ||
thing_id = "mongo_test_persistence_unique" | ||
prop_name = "test_prop_unique" | ||
|
||
# Aggressively clear any old data for this key | ||
client = MongoClient("mongodb://localhost:27017") | ||
db = client["hololinked"] | ||
db["properties"].delete_many({"id": thing_id, "name": prop_name}) | ||
|
||
class MongoTestThing(Thing): | ||
test_prop_unique = Property(default=123, db_persist=True) | ||
|
||
# Create instance with MongoDB backend | ||
instance = MongoTestThing(id=thing_id, use_mongo_db=True) | ||
# Set property value | ||
instance.test_prop_unique = 456 | ||
# Read back from db_engine (should be persisted) | ||
value_from_db = instance.db_engine.get_property(prop_name) | ||
self.assertEqual(value_from_db, 456) | ||
@classmethod | ||
def setUpClass(self): | ||
super().setUpClass() | ||
print(f"test property with {self.__name__}") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One could leave this as it was, because
kwargs
already takes care of passinguse_default_db
into theprepare_object_storage
function. I dont think there is a need of doing some preprocessing here.