Merge pull request #3 from bjoernsteinhagen/bjorn/refactor-core-and-domain

main.py

@@ -5,17 +5,19 @@ from speckle_automate import (
     execute_automate_function,
 )
 
-from src.processors.material import RevitMaterialProcessor
-from src.processors.compliance import RevitComplianceChecker
-from src.processors.model import RevitModelProcessor
-from src.validators.revit import RevitSourceValidator
-from src.aggregators.carbon_totals import MassAggregator
-from src.logging.compliance_logger import ComplianceLogger
+from src.applications.revit.revit_material import RevitMaterial
+from src.applications.revit.revit_compliance import RevitCompliance
+from src.applications.revit.revit_model import RevitModel
+from src.applications.revit.revit_source_validator import RevitSourceValidator
+from src.carbon.aggregator import MassAggregator
+from src.applications.revit.revit_logger import RevitLogger
+from src.core.base import Model, Logger
+
 
 # TODO: Function inputs
 class FunctionInputs(AutomateBase):
-    """User-defined function inputs.
-    """
+    """User-defined function inputs."""
+
     whisper_message: SecretStr = Field(title="This is a secret message")
     forbidden_speckle_type: str = Field(
         title="Forbidden speckle type",
@@ -47,14 +49,14 @@ def automate_function(
             return
 
         # Create processor chain and get logger for results
-        processor, logger = create_processor_chain()
+        processor = configure_components()
 
         # Process model
         model_root = automate_context.receive_version()  # TODO: Line 35 and 36!?
         processor.process_elements(model_root)
 
         # Logger information - successes
-        logger_successes = logger.get_successful_summary()
+        logger_successes, logger_warnings = processor.get_processing_results()
         if logger_successes:
             automate_context.attach_success_to_objects(
                 category="Successfully Processed",
@@ -63,7 +65,6 @@ def automate_function(
             )
 
         # Logger information - warnings
-        logger_warnings = logger.get_warnings_summary()
         if logger_warnings:
             for missing_property, elements in logger_warnings.items():
                 automate_context.attach_warning_to_objects(
@@ -86,25 +87,33 @@ def automate_function(
         raise  # Re-raise for proper error tracking
 
 
-def create_processor_chain() -> tuple[RevitModelProcessor, ComplianceLogger]:
-    """Creates and configures the required components."""
+# TODO instead of hard-coding revit, demo a factory method to inject implementations based on
+#  function input
+def configure_components() -> Model:
+    """Configures and wires up processor components with dependencies.
+
+    Creates core system components (logger, aggregator) and configures processors
+    with required dependencies injected.
+
+    Returns:
+        tuple:
+            - Model: Main processor configured with dependencies
+    """
 
     # Core components
-    logger = ComplianceLogger()  # For tracking issues
+    logger = RevitLogger()  # For tracking issues
     mass_aggregator = MassAggregator()  # For collecting computed masses
+    # TODO: results_aggregator = ResultAggregator and get rid of mass_aggregator
 
     # Create processors
-    material_processor = RevitMaterialProcessor(mass_aggregator)  # Material calcs
-    compliance_checker = RevitComplianceChecker(logger)  # Validation
+    material_processor = RevitMaterial(mass_aggregator)  # Material handler to "inject"
+    compliance_checker = RevitCompliance(logger)  # Compliance checker to "inject"
 
-    # Create and return the main processor with logger
-    return (
-        RevitModelProcessor(
+    # Create and return the main processor with dependencies "injected"
+    return RevitModel(
         material_processor=material_processor,
         compliance_checker=compliance_checker,
         logger=logger,
-        ),
-        logger,
     )
 
 

src/applications/revit/revit_compliance.py

@@ -1,8 +1,7 @@
-from typing import Any, List, Optional
-from dataclasses import dataclass
+from typing import Any, List
 
-from src.interfaces import ComplianceChecker, Logger
-from src.utils.constants import (
+from src.core.base import Compliance, Logger
+from src.applications.revit.utils.constants import (
     ID,
     SPECKLE_TYPE,
     LINE,
@@ -16,7 +15,7 @@ from src.utils.constants import (
 )
 
 
-class RevitComplianceChecker(ComplianceChecker):
+class RevitCompliance(Compliance):
     """Implementation of the ComplianceChecker in the context of Revit.
     Checks if elements contain required properties for carbon calculations.
     """
@@ -26,7 +25,7 @@ class RevitComplianceChecker(ComplianceChecker):
 
     def check_compliance(
         self, element: Any, required_properties: List[str]
-    ) -> ComplianceChecker.ValidationResult:
+    ) -> Compliance.ValidationResult:
         """
         Validates element and returns validation result with material data if valid.
 
@@ -48,7 +47,7 @@ class RevitComplianceChecker(ComplianceChecker):
 
         return validation
 
-    def _validate_element(self, element: Any) -> ComplianceChecker.ValidationResult:
+    def _validate_element(self, element: Any) -> Compliance.ValidationResult:
         """Internal validation logic for a single element.
 
         Args:

src/applications/revit/revit_logger.py

@@ -2,13 +2,14 @@ import structlog
 from typing import Dict, DefaultDict
 from collections import defaultdict
 
-from src.interfaces.logger import Logger  # Import the interface
+from src.core.base.logger import Logger  # Import the interface
 
 # NOTE: Only provide docstring if not covered by base class
 
-class ComplianceLogger(Logger):
-    """Implements Logger interface
-    """
+
+class RevitLogger(Logger):
+    """Implements Logger interface"""
+
     def __init__(self):
         self.missing_properties: DefaultDict[str, set] = defaultdict(set)
         self.successful_elements: set = set()
@@ -18,7 +19,7 @@ class ComplianceLogger(Logger):
         self._structlog.error(message, **kwargs)
 
     def log_warning(self, message: str, **kwargs) -> None:
-        """ Log a warning message.
+        """Log a warning message.
         Categorises and caches missing properties if 'missing_property' and 'object_id' specified in the kwargs.
 
         Args:
@@ -34,7 +35,7 @@ class ComplianceLogger(Logger):
         if object_id and missing_property:
             self.missing_properties[missing_property].add(object_id)
 
-    def log_success(self, object_id: str) -> None:
+    def log_success(self, object_id: str, **kwargs) -> None:
         self._structlog.info(f"Successfully processed element", object_id=object_id)
         self.successful_elements.add(object_id)
 

src/applications/revit/revit_material.py

@@ -1,15 +1,21 @@
 from typing import Dict, Any
-from src.interfaces.material_processor import MaterialProcessor
-from src.models.material import (
+from src.core.base import Material
+from src.core.types.material import (
     MaterialData,
 )
-from src.utils.constants import VOLUME, VALUE, DENSITY, STRUCTURAL_ASSET
+from src.applications.revit.utils.constants import (
+    VOLUME,
+    VALUE,
+    DENSITY,
+    STRUCTURAL_ASSET,
+)
 
 # NOTE: Only provide docstring if not covered by base class
 
-class RevitMaterialProcessor(MaterialProcessor):
-    """Implementation of the MaterialProcessor for the Revit context.
-    """
+
+class RevitMaterial(Material):
+    """Implementation of the MaterialProcessor for the Revit context."""
+
     def __init__(self, mass_aggregator: "MassAggregator"):
         self._mass_aggregator = mass_aggregator
 

src/applications/revit/revit_model.py

@@ -1,9 +1,9 @@
 from typing import Any
-from src.interfaces.model_processor import ModelProcessor
-from src.interfaces.material_processor import MaterialProcessor
-from src.interfaces.compliance_checker import ComplianceChecker
-from src.interfaces.logger import Logger
-from src.utils.constants import (
+from src.core.base import Model
+from src.core.base import Material
+from src.core.base import Compliance
+from src.core.base.logger import Logger
+from src.applications.revit.utils.constants import (
     ELEMENTS,
     NAME,
     ID,
@@ -12,13 +12,13 @@ from src.utils.constants import (
 # NOTE: Only provide docstring if not covered by base class
 
 
-class RevitModelProcessor(ModelProcessor):
+class RevitModel(Model):
     """Implementation of the ModelProcessor in the Revit context."""
 
     def __init__(
         self,
-        material_processor: MaterialProcessor,
-        compliance_checker: ComplianceChecker,
+        material_processor: Material,
+        compliance_checker: Compliance,
         logger: Logger,
     ):
         self._material_processor = material_processor
@@ -28,6 +28,18 @@ class RevitModelProcessor(ModelProcessor):
     def process_elements(self, model: Any) -> None:
         """Model traversal.
         Model → Levels → Type Groups → Elements.
+
+        Performance Notes:
+            - Threading? Could be cool, but:
+                - Profile code first. Do we need it?
+                - Not really I/O bound. Our traversal is just walking through an in-memory object structure
+                - TBH I'm scared of the hierarchy
+                - Thread safety? Shared loggers and aggregators.
+            - Flattening nested iterations?
+                - Code less readable (e.g. chain.from_iterable(..))
+                - Obscures hierarchical nature of data
+                - Performance benefit(s) minimal?
+                - Harder to debug
         """
         levels = self._get_elements(model, "model")
 
@@ -76,6 +88,13 @@ class RevitModelProcessor(ModelProcessor):
                 f"Failed to process element {getattr(model_object, ID)}", error=str(e)
             )
 
+    # TODO: This is gross.
+    def get_processing_results(self) -> tuple[list, dict]:
+        return (
+            self._logger.get_successful_summary(),
+            self._logger.get_warnings_summary(),
+        )
+
     @staticmethod
     def _get_elements(node: Any, context: str) -> list:
         """Get elements from a node, with consistent error handling.

src/applications/revit/revit_source_validator.py

@@ -1,4 +1,4 @@
-from src.interfaces.validator import SourceApplicationValidator
+from src.core.base.source_validator import SourceApplicationValidator
 
 
 class RevitSourceValidator(SourceApplicationValidator):

src/applications/revit/utils/constants.py

@@ -1,4 +1,4 @@
-# TODO: Implementation specific constants.
+# TODO: Check that the constants only get used in the applications/revit/ level
 
 REQUIRED_PROPERTIES = ["volume", "density", "structuralAsset"]
 

src/carbon/aggregator.py

@@ -4,20 +4,22 @@ from collections import defaultdict
 
 # TODO: Replace with carbon aggregator when ready
 class MassAggregator:
-    """Cumulative sum of the computed masses. Grouped by level and type.
-    """
+    """Cumulative sum of the computed masses. Grouped by level and type."""
+
     def __init__(self):
         self.totals = defaultdict(lambda: defaultdict(lambda: defaultdict(float)))
 
     # TODO: Check! Changes for Revit model groups probably broke this
-    def add_mass(self, mass: float, level: str, collection_type: str, material: str) -> None:
+    def add_mass(
+        self, mass: float, level: str, collection_type: str, material: str
+    ) -> None:
         """Adds computed mass of a single object to the totals.
         Ignores computed masses < 1e-6.
 
         Args:
             mass (float): computed mass
             level (str): string of the associated level of the object
-            type (str): object collection (e.g. "Columns", "Structural Foundations")
+            collection_type (str): object collection (e.g. "Columns", "Structural Foundations")
             material (str): name of the structural asset
         """
         if mass <= 1e-6:

src/core/base/__init__.py

@@ -0,0 +1,13 @@
+from .logger import Logger
+from .model import Model
+from .source_validator import SourceApplicationValidator
+from .material import Material
+from .compliance import Compliance
+
+__all__ = [
+    "Logger",
+    "Model",
+    "SourceApplicationValidator",
+    "Material",
+    "Compliance",
+]

src/core/base/compliance.py

@@ -4,7 +4,7 @@ from dataclasses import dataclass
 from typing import Optional
 
 
-class ComplianceChecker(ABC):
+class Compliance(ABC):
     """Interface for compliance checks.
     Compliance are intended to be called for every object where an attribute is assumed.
     """

src/core/base/material.py

@@ -2,9 +2,9 @@ from abc import ABC, abstractmethod
 from typing import Dict, Any
 
 
-class MaterialProcessor(ABC):
-    """Interface for processing a material.
-    """
+class Material(ABC):
+    """Interface for processing a material."""
+
     @abstractmethod
     def process_material(
         self, material_data: Dict[str, Any], level: str, type_name: str

src/core/base/model.py

@@ -2,9 +2,9 @@ from abc import ABC, abstractmethod
 from typing import Any
 
 
-class ModelProcessor(ABC):
-    """Interface for model processing.
-    """
+class Model(ABC):
+    """Interface for model processing."""
+
     @abstractmethod
     def process_elements(self, model: Any) -> None:
         """Process all elements in the model.
@@ -25,3 +25,9 @@ class ModelProcessor(ABC):
             model_object (Any): speckle object
         """
         pass
+
+    # TODO: This is gross
+    @abstractmethod
+    def get_processing_results(self) -> tuple[list, dict]:
+        """Expose logging results."""
+        pass

src/interfaces/__init__.py

@@ -1,13 +0,0 @@
-from .logger import Logger
-from .model_processor import ModelProcessor
-from .validator import SourceApplicationValidator
-from .material_processor import MaterialProcessor
-from .compliance_checker import ComplianceChecker
-
-__all__ = [
-    "Logger",
-    "ModelProcessor",
-    "SourceApplicationValidator",
-    "MaterialProcessor",
-    "ComplianceChecker",
-]

src/processors/__init__.py

@@ -1,5 +0,0 @@
-from .material import RevitMaterialProcessor
-from .compliance import RevitComplianceChecker
-from .model import RevitModelProcessor
-
-__all__ = ["RevitMaterialProcessor", "RevitComplianceChecker", "RevitModelProcessor"]

src/validators/__init__.py

@@ -1,3 +0,0 @@
-from .revit import RevitSourceValidator
-
-__all__ = ["RevitSourceValidator"]

tests/manual_test.py

@@ -1,11 +1,11 @@
 # pytest: skip-file
 
-from src.processors.material import RevitMaterialProcessor
-from src.processors.compliance import RevitComplianceChecker
-from src.processors.model import RevitModelProcessor
-from src.validators.revit import RevitSourceValidator
-from src.aggregators.carbon_totals import MassAggregator
-from src.logging.compliance_logger import ComplianceLogger
+from src.applications.revit.revit_material import RevitMaterial
+from src.applications.revit.revit_compliance import RevitCompliance
+from src.applications.revit.revit_model import RevitModel
+from src.applications.revit.revit_source_validator import RevitSourceValidator
+from src.carbon.aggregator import MassAggregator
+from src.applications.revit.revit_logger import RevitLogger
 
 # Import required libraries
 from specklepy.api.client import SpeckleClient
@@ -28,26 +28,26 @@ branch = client.branch.get(stream_id=STREAM_ID, name=BRANCH_NAME)
 model_data = operations.receive(branch.commits.items[0].referencedObject, transport)
 
 
-def create_processor_chain() -> tuple[RevitModelProcessor, ComplianceLogger]:
+def create_processor_chain() -> tuple[RevitModel, RevitLogger]:
     """
     Creates and configures the processing chain with all necessary dependencies.
 
     Returns:
-        tuple[RevitModelProcessor, ComplianceLogger]:
-            - Configured processor ready to handle Revit models
+        tuple[RevitModel, RevitLogger]:
+            - Configured processor ready to handle Revit types
             - Logger instance for accessing compliance results
     """
     # Create core components
-    logger = ComplianceLogger()
+    logger = RevitLogger()
     mass_aggregator = MassAggregator()
 
     # Create processors
-    material_processor = RevitMaterialProcessor(mass_aggregator)
-    compliance_checker = RevitComplianceChecker(logger)
+    material_processor = RevitMaterial(mass_aggregator)
+    compliance_checker = RevitCompliance(logger)
 
     # Create and return the main processor with logger
     return (
-        RevitModelProcessor(
+        RevitModel(
             material_processor=material_processor,
             compliance_checker=compliance_checker,
             logger=logger,