anomaly / gallagher

The missing toolkit for extending Gallagher Command Centre, featuring a CLI, SQL interface, TUI and a Python idiomatic SDK
https://anomaly.github.io/gallagher/
MIT License
11 stars 2 forks source link

Reorganise DTO to `refs`, `summary`, `detail`, `response` packages to avoid circular dependency issues #21

Closed devraj closed 8 months ago

devraj commented 9 months ago

Consider this cdaa0b2 commit where I have had to disable imports of Refs from the alarm package in event, this is because the nature of the data is that they cross reference each other, that is to say that Alarms are Events, and Event Summary can be that of an alarm.

This will become a bigger problem as the library is built out, we will have several cases of cross references.

It's thus suggested that we refactor the code base and move all references to dto/refs.py package to avoid circular dependencies.

devraj commented 8 months ago

On initially trying to implement this I created a dto/refs.py and started porting the Ref classes to that package. It feels odd from a design point of view, if we follow that pattern, for consistency we should have summary and response as packages.

The two patterns that seem logical are:

Group them by DTO type There would be three over arching groups ref, summary, response and they contain packages for each command centre function e.g:

Imports would thus look like:

from gallagher.dto.ref.alarm import AlarmRef

Group them by CC function In this pattern the overarching groups would be the command centre functions (as we have them now by then divide them further) e.g:

and then divide them further as:

Imports would thus look like:

from gallagher.dto.alarm.ref import AlarmRef
devraj commented 8 months ago

We also have various cases where a Summary inherits from a Ref e.g:

class AccessGroupRef(
    AppBaseModel,
    HrefMixin
):
    """ Access Groups is what a user is assigned to to provide access to doors
    """
    name: str

class AccessGroupSummary(
    AccessGroupRef
):
    """ AccessGroup Summary is what the API returns on searches

    This builds on the Ref class to add the summary fields and is
    extended by the Detail class to add the fully remainder of
    the fields
    """
    description: Optional[str]
    parent: Optional[AccessGroupRef]
    division: IdentityMixin
    cardholders: Optional[HrefMixin]
    server_display_name: Optional[str]

while it is efficient to do this, it might not be wise when trying to avoid circular dependencies.

Also logically a Summary is not an extension of a Ref and by inheriting you would imply that it is the case.

devraj commented 8 months ago

Consider importing all the classes to the top level of the package, so the imports look more like the following

from gallagher.dto.ref import AlarmRef
devraj commented 8 months ago

Closed this issue by merging