Skip to content

Commit

Permalink
Domain Traversal Refactoring (#398)
Browse files Browse the repository at this point in the history
This PR contains the following fixes:

* Changes how domain.init() traverses the file structure
* Require domain to be initialized with `__file__` (Breaking Change)
* Fixed with SQLAlchemy model generation
* Fixed inconsistencies in the usage of the `schema-name` attribute in Models
* Cleaned up test domain initialization
* Removed naive MULTI_TENANCY implementation
* Upgraded packages
  • Loading branch information
subhashb authored Mar 16, 2024
1 parent 9955576 commit aac52d6
Show file tree
Hide file tree
Showing 49 changed files with 922 additions and 1,019 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ jobs:
if: steps.cache.outputs.cache-hit != 'true'

- name: Tests
run: pytest --slow --sqlite --postgresql --elasticsearch --redis --cov=protean --cov-config .coveragerc tests
run: pytest --ignore=tests/support/ --slow --sqlite --postgresql --elasticsearch --redis --cov=protean --cov-config .coveragerc tests
env:
TEST_CMD: pre-commit run --all-files
POSTGRES_PASSWORD: postgres
Expand Down
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
repos:
- repo: https://github.com/psf/black
rev: 22.3.0
rev: 24.2.0
hooks:
- id: black
language_version: python3.8

- repo: [email protected]:PyCQA/autoflake.git
rev: v2.2.1
rev: v2.3.1
hooks:
- id: autoflake
args:
Expand All @@ -17,7 +17,7 @@ repos:
]

- repo: https://github.com/pycqa/isort
rev: 5.10.1
rev: 5.13.2
hooks:
- id: isort
args: ["--profile", "black", "--filter-files", "--quiet"]
1,217 changes: 589 additions & 628 deletions poetry.lock

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ celery = { version = "~5.2.7", extras = ["redis"], optional = true}
flask = {version = ">=1.1.1", optional = true}
sendgrid = {version = ">=6.1.3", optional = true}
message-db-py = {version = ">=0.1.2", optional = true}
tox = "^4.14.1"

[tool.poetry.extras]
elasticsearch = ["elasticsearch", "elasticsearch-dsl"]
Expand All @@ -77,9 +78,9 @@ optional = true
black = ">=23.11.0"
check-manifest = ">=0.49"
coverage = ">=7.3.2"
docutils = ">=0.18.0"
docutils = ">=0.20.1"
pre-commit = ">=2.16.0"
tox = ">=4.11.3"
tox = ">=4.14.1"
twine = ">=4.0.2"

[tool.poetry.group.test]
Expand Down Expand Up @@ -143,7 +144,8 @@ markers = [
"message_db",
"sendgrid",
"database",
"eventstore"
"eventstore",
"no_test_domain",
]

[tool.isort]
Expand Down
10 changes: 3 additions & 7 deletions src/protean/adapters/repository/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
""" Package for Concrete Implementations of Protean repositories """

import collections
import importlib
import logging
Expand All @@ -7,7 +8,6 @@

from protean.core.repository import BaseRepository, repository_factory
from protean.exceptions import ConfigurationError
from protean.globals import g
from protean.utils import fully_qualified_name

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -109,9 +109,7 @@ def _initialize(self):

def get_connection(self, provider_name="default"):
"""Fetch connection from Provider"""
if (
hasattr(g, "MULTITENANCY") and g.MULTITENANCY is True
) or self._providers is None:
if self._providers is None:
self._initialize()

try:
Expand All @@ -121,9 +119,7 @@ def get_connection(self, provider_name="default"):

def repository_for(self, aggregate_cls):
"""Retrieve a Repository registered for the Aggregate"""
if (
hasattr(g, "MULTITENANCY") and g.MULTITENANCY is True
) or self._providers is None:
if self._providers is None:
self._initialize()

provider_name = aggregate_cls.meta_.provider
Expand Down
9 changes: 5 additions & 4 deletions src/protean/adapters/repository/elasticsearch.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Module containing repository implementation for Elasticsearch"""

import logging

from typing import Any
Expand Down Expand Up @@ -47,9 +48,9 @@ def from_entity(cls, entity) -> "ElasticsearchModel":
item_dict = {}
for attribute_obj in attributes(cls.meta_.entity_cls).values():
if isinstance(attribute_obj, Reference):
item_dict[
attribute_obj.relation.attribute_name
] = attribute_obj.relation.value
item_dict[attribute_obj.relation.attribute_name] = (
attribute_obj.relation.value
)
else:
item_dict[attribute_obj.attribute_name] = getattr(
entity, attribute_obj.attribute_name
Expand Down Expand Up @@ -396,7 +397,7 @@ def decorate_model_class(self, entity_cls, model_cls):

# Construct Inner Index class with options
options = {}
options["name"] = model_cls.meta_.schema or schema_name
options["name"] = model_cls.meta_.schema_name or schema_name
if "SETTINGS" in self.conn_info and self.conn_info["SETTINGS"]:
options["settings"] = self.conn_info["SETTINGS"]
index_cls = type("Index", (object,), options)
Expand Down
40 changes: 35 additions & 5 deletions src/protean/adapters/repository/sqlalchemy.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
"""Module with repository implementation for SQLAlchemy"""

import copy
import logging
import uuid

Expand Down Expand Up @@ -202,7 +204,11 @@ def field_mapping_for(field_obj: Field):


def derive_schema_name(model_cls):
if hasattr(model_cls.meta_, "schema_name"):
# Retain schema name if already present, otherwise derive from entity class
if (
hasattr(model_cls.meta_, "schema_name")
and model_cls.meta_.schema_name is not None
):
return model_cls.meta_.schema_name
else:
return model_cls.meta_.entity_cls.meta_.schema_name
Expand All @@ -222,9 +228,9 @@ def from_entity(cls, entity):
item_dict = {}
for attribute_obj in attributes(cls.meta_.entity_cls).values():
if isinstance(attribute_obj, Reference):
item_dict[
attribute_obj.relation.attribute_name
] = attribute_obj.relation.value
item_dict[attribute_obj.relation.attribute_name] = (
attribute_obj.relation.value
)
else:
item_dict[attribute_obj.attribute_name] = getattr(
entity, attribute_obj.attribute_name
Expand Down Expand Up @@ -637,16 +643,34 @@ def decorate_model_class(self, entity_cls, model_cls):
if issubclass(model_cls, SqlalchemyModel):
return model_cls
else:
# Strip out `Column` attributes from the model class
# Create a deep copy to make this work
# https://stackoverflow.com/a/62528033/1858466
columns = copy.deepcopy(
{
key: value
for key, value in vars(model_cls).items()
if isinstance(value, Column)
}
)

custom_attrs = {
key: value
for (key, value) in vars(model_cls).items()
if key not in ["Meta", "__module__", "__doc__", "__weakref__"]
and not isinstance(value, Column)
}

# Add the earlier copied columns to the custom attributes
custom_attrs = {**custom_attrs, **columns}

from protean.core.model import ModelMeta

meta_ = ModelMeta()
meta_ = ModelMeta(model_cls.meta_)
meta_.entity_cls = entity_cls
meta_.schema_name = (
schema_name if meta_.schema_name is None else meta_.schema_name
)

custom_attrs.update({"meta_": meta_, "metadata": self._metadata})
# FIXME Ensure the custom model attributes are constructed properly
Expand All @@ -669,8 +693,14 @@ def construct_model_class(self, entity_cls):
else:
from protean.core.model import ModelMeta

# Construct a new Meta object with existing values
meta_ = ModelMeta()
meta_.entity_cls = entity_cls
# If schema_name is not provided, sqlalchemy can throw
# sqlalchemy.exc.InvalidRequestError: Class does not
# have a __table__ or __tablename__ specified and
# does not inherit from an existing table-mapped class
meta_.schema_name = entity_cls.meta_.schema_name

attrs = {
"meta_": meta_,
Expand Down
24 changes: 14 additions & 10 deletions src/protean/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
Also see (1) from http://click.pocoo.org/5/setuptools/#setuptools-integration
"""

import ast
import os
import re
Expand Down Expand Up @@ -193,20 +194,24 @@ def derive_domain(domain_path):
def test(category):
import subprocess

commands = ["pytest", "--cache-clear", "--ignore=tests/support/"]

if category:
if category == "BASIC":
print(f"Running core tests...")
subprocess.call(commands)
if category == "EVENTSTORE":
for store in ["MEMORY", "MESSAGE_DB"]:
print(f"Running tests for EVENTSTORE: {store}...")
subprocess.call(["pytest", "-m", "eventstore", f"--store={store}"])
subprocess.call(commands + ["-m", "eventstore", f"--store={store}"])
elif category == "DATABASE":
for db in ["POSTGRESQL", "SQLITE"]:
print(f"Running tests for DATABASE: {db}...")
subprocess.call(["pytest", "-m", "database", f"--db={db}"])
subprocess.call(commands + ["-m", "database", f"--db={db}"])
elif category == "WITH_COVERAGE":
subprocess.call(
[
"pytest",
"--cache-clear",
commands
+ [
"--slow",
"--sqlite",
"--postgresql",
Expand All @@ -222,9 +227,8 @@ def test(category):
else:
# Run full suite
subprocess.call(
[
"pytest",
"--cache-clear",
commands
+ [
"--slow",
"--sqlite",
"--postgresql",
Expand All @@ -239,11 +243,11 @@ def test(category):
for db in ["POSTGRESQL", "SQLITE"]:
print(f"Running tests for DB: {db}...")

subprocess.call(["pytest", "-m", "database", f"--db={db}"])
subprocess.call(commands + ["-m", "database", f"--db={db}"])

for store in ["MESSAGE_DB"]:
print(f"Running tests for EVENTSTORE: {store}...")
subprocess.call(["pytest", "-m", "eventstore", f"--store={store}"])
subprocess.call(commands + ["-m", "eventstore", f"--store={store}"])


@main.command()
Expand Down
10 changes: 6 additions & 4 deletions src/protean/core/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ class ModelMeta:
def __init__(self, meta=None):
if meta:
self.entity_cls = getattr(meta, "entity_cls", None)
self.schema = getattr(meta, "schema", None)
self.schema_name = getattr(meta, "schema_name", None)
self.database = getattr(meta, "database", None)
else:
self.entity_cls = None
self.schema = None
self.schema_name = None
self.database = None


Expand Down Expand Up @@ -57,8 +57,10 @@ def model_factory(element_cls, **kwargs):
if not (hasattr(element_cls.meta_, "entity_cls") and element_cls.meta_.entity_cls):
element_cls.meta_.entity_cls = kwargs.pop("entity_cls", None)

if not (hasattr(element_cls.meta_, "schema") and element_cls.meta_.schema):
element_cls.meta_.schema = kwargs.pop("schema", None)
if not (
hasattr(element_cls.meta_, "schema_name") and element_cls.meta_.schema_name
):
element_cls.meta_.schema_name = kwargs.pop("schema_name", None)

if not (hasattr(element_cls.meta_, "database") and element_cls.meta_.database):
element_cls.meta_.database = kwargs.pop("database", None)
Expand Down
16 changes: 8 additions & 8 deletions src/protean/domain/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""This module implements the central domain object, along with decorators
to register Domain Elements.
"""

import logging
import sys

Expand Down Expand Up @@ -126,6 +127,7 @@ class Domain(_PackageBoundObject):

def __init__(
self,
file_path: str,
domain_name: str = __name__,
root_path: str = None,
instance_relative_config: bool = False,
Expand All @@ -137,6 +139,7 @@ def __init__(
root_path=root_path,
)

self.file_path = file_path
self.domain_name = domain_name

# FIXME Additional domain attributes: (Think if this is needed)
Expand Down Expand Up @@ -170,6 +173,9 @@ def __init__(
# FIXME Should all protean elements be subclassed from a base element?
self._pending_class_resolutions: dict[str, Any] = defaultdict(list)

# Initialize domain with default adapters
self._initialize()

def init(self, traverse=True): # noqa: C901
"""Parse the domain folder, and attach elements dynamically to the domain.
Expand Down Expand Up @@ -199,11 +205,7 @@ def init(self, traverse=True): # noqa: C901
import os
import pathlib

# Fetch the domain file and derive the system path
domain_path = inspect.stack()[1][
1
] # Find the file in which the domain is defined
dir_name = pathlib.PurePath(pathlib.Path(domain_path).resolve()).parent
dir_name = pathlib.PurePath(pathlib.Path(self.file_path).resolve()).parent
path = pathlib.Path(dir_name) # Resolve the domain file's directory
system_folder_path = (
path.parent
Expand Down Expand Up @@ -233,7 +235,7 @@ def init(self, traverse=True): # noqa: C901

try:
if (
full_file_path != domain_path
full_file_path != self.file_path
): # Don't load the domain file itself again
spec = importlib.util.spec_from_file_location(
file_module_name, full_file_path
Expand All @@ -244,8 +246,6 @@ def init(self, traverse=True): # noqa: C901
logger.debug(f"Loaded {file_module_name}")
except ModuleNotFoundError as exc:
logger.error(f"Error while loading a module: {exc}")
except ModuleNotFoundError as exc:
logger.error(f"Error while autoloading modules: {exc}")

# Initialize adapters after loading domain
self._initialize()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import os
from protean.domain import Domain

# Domain Composition Root
{{ package_name }} = Domain("{{ project_name }}")
{{ package_name }} = Domain(__file__, "{{ project_name }}")

# Config class names
configs = {
Expand Down
Loading

0 comments on commit aac52d6

Please sign in to comment.