Reorganise DTO to `refs`, `summary`, `detail`, `response` packages to avoid circular dependency issues
Closed this issue · 4 comments
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.
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:
ref/alarmsummary/alarmresponse/alarm
Imports would thus look like:
from gallagher.dto.ref.alarm import AlarmRefGroup 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:
alarmcardholder
and then divide them further as:
alarm/refalarm/summaryalarm/response
Imports would thus look like:
from gallagher.dto.alarm.ref import AlarmRefWe 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.
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