Skip to content

Commit 85a3bf1

Browse files
committed
refactor: Expose components publicly and remove facade boilerplate
Implement architectural improvement to reduce indirection by making all components public and removing single-line wrapper methods, while keeping only the 3 essential output methods that define DiffStix's core purpose. Changes to DiffStix class: - Made all components public (removed _ prefix): - contributor_tracker (was _contributor_tracker) - data_loader (was _data_loader) - change_detector (was _change_detector) - hierarchy_builder (was _hierarchy_builder) - statistics_collector (was _statistics_collector) - markdown_generator (was _markdown_generator) - layer_generator (was _layer_generator) - json_generator (was _json_generator) - Removed 11 single-line delegation wrapper methods: - find_technique_mitigation_changes() - find_technique_detection_changes() - update_contributors() - get_groupings() - get_contributor_section() - get_parent_stix_object() - placard() - get_statistics_section() - get_markdown_section_data() - get_md_key() - Kept only 3 essential output wrappers (core public API): - get_markdown_string() → markdown_generator.generate() - get_layers_dict() → layer_generator.generate() - get_changes_dict() → json_generator.generate() Updated components: - change_detector.py: Use public contributor_tracker - markdown_generator.py: Use public hierarchy_builder - json_generator.py: Use public hierarchy_builder Updated tests to use public component API: - test_diffstix_methods.py: 8 method calls updated - test_markdown_output.py: 3 method calls updated - test_diffstix_data_processing.py: 1 method call updated Impact: - diff_stix.py: 532 → 390 lines (26.7% reduction, 142 lines removed) - Total from original: 1,462 → 390 lines (73.3% reduction, 1,072 lines removed) - Clear component boundaries - users call components directly - Minimal facade - only wraps the 3 main output methods - Better IDE discoverability and autocomplete - Clearer separation of concerns Example usage change: Before: diff_stix.placard(obj, section, domain) After: diff_stix.markdown_generator.placard(obj, section, domain)
1 parent 80e9a88 commit 85a3bf1

File tree

7 files changed

+38
-180
lines changed

7 files changed

+38
-180
lines changed

mitreattack/diffStix/core/change_detector.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ def categorize_version_change(
118118
- new_version: The new version
119119
"""
120120
# Verify if there are new contributors on the object
121-
self.diff_stix._contributor_tracker.update_contributors(old_object=old_obj, new_object=new_obj)
121+
self.diff_stix.contributor_tracker.update_contributors(old_object=old_obj, new_object=new_obj)
122122

123123
old_version = get_attack_object_version(old_obj)
124124
new_version = get_attack_object_version(new_obj)

mitreattack/diffStix/core/diff_stix.py

Lines changed: 23 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -93,24 +93,24 @@ def __init__(
9393
self.section_headers = self._config.get_all_section_headers()
9494

9595
# Initialize contributor tracker for the new release
96-
self._contributor_tracker = ContributorTracker()
96+
self.contributor_tracker = ContributorTracker()
9797

9898
# Initialize the nested data structure for tracking changes
9999
# Data gets loaded into here in the load_data() function
100100
self.data = DataStructureInitializer.create_structure(self.domains, self.types)
101101

102102
# Initialize data loader and change detector before data loading
103-
self._data_loader = DataLoader(self)
104-
self._change_detector = ChangeDetector(self)
103+
self.data_loader = DataLoader(self)
104+
self.change_detector = ChangeDetector(self)
105105

106106
self.load_data()
107107

108108
# Initialize components after data is loaded
109-
self._hierarchy_builder = HierarchyBuilder(self)
110-
self._statistics_collector = StatisticsCollector(self)
111-
self._markdown_generator = MarkdownGenerator(self)
112-
self._layer_generator = LayerGenerator(self)
113-
self._json_generator = JsonGenerator(self)
109+
self.hierarchy_builder = HierarchyBuilder(self)
110+
self.statistics_collector = StatisticsCollector(self)
111+
self.markdown_generator = MarkdownGenerator(self)
112+
self.layer_generator = LayerGenerator(self)
113+
self.json_generator = JsonGenerator(self)
114114

115115
@property
116116
def release_contributors(self) -> dict:
@@ -121,7 +121,7 @@ def release_contributors(self) -> dict:
121121
dict
122122
Dictionary of contributor names to contribution counts.
123123
"""
124-
return self._contributor_tracker.release_contributors
124+
return self.contributor_tracker.release_contributors
125125

126126
@release_contributors.setter
127127
def release_contributors(self, value: dict):
@@ -132,7 +132,7 @@ def release_contributors(self, value: dict):
132132
value : dict
133133
Dictionary of contributor names to contribution counts.
134134
"""
135-
self._contributor_tracker.release_contributors = value
135+
self.contributor_tracker.release_contributors = value
136136

137137

138138
def load_data(self):
@@ -143,7 +143,7 @@ def load_data(self):
143143
def _load_all_domains(self):
144144
"""Load STIX data for all configured domains."""
145145
for domain in track(self.domains, description="Loading domains"):
146-
self._data_loader.load_domain(domain=domain)
146+
self.data_loader.load_domain(domain=domain)
147147

148148
def _detect_all_changes(self):
149149
"""Detect and categorize changes across all domains and object types."""
@@ -222,7 +222,7 @@ def _categorize_object_changes(self, old_objects: dict, new_objects: dict, domai
222222
new_stix_obj["detailed_diff"] = ddiff.to_json()
223223

224224
# Check for revocations
225-
revocation_result = self._change_detector.detect_revocation(
225+
revocation_result = self.change_detector.detect_revocation(
226226
stix_id, old_stix_obj, new_stix_obj, new_objects, domain
227227
)
228228
if revocation_result is False:
@@ -234,14 +234,14 @@ def _categorize_object_changes(self, old_objects: dict, new_objects: dict, domai
234234
continue # Already revoked - skip
235235

236236
# Check for deprecations
237-
if self._change_detector.detect_deprecation(old_stix_obj, new_stix_obj):
237+
if self.change_detector.detect_deprecation(old_stix_obj, new_stix_obj):
238238
changes["deprecations"].add(stix_id)
239239
continue
240240
elif new_stix_obj.get("x_mitre_deprecated"):
241241
continue # Already deprecated - skip
242242

243243
# Categorize version changes
244-
category, old_version, new_version = self._change_detector.categorize_version_change(
244+
category, old_version, new_version = self.change_detector.categorize_version_change(
245245
stix_id, old_stix_obj, new_stix_obj
246246
)
247247

@@ -260,8 +260,8 @@ def _categorize_object_changes(self, old_objects: dict, new_objects: dict, domai
260260
new_stix_obj["version_change"] = f"{old_version}{new_version}"
261261

262262
# Process description and relationship changes
263-
self._change_detector.process_description_changes(old_stix_obj, new_stix_obj)
264-
self._change_detector.process_relationship_changes(new_stix_obj, domain)
263+
self.change_detector.process_description_changes(old_stix_obj, new_stix_obj)
264+
self.change_detector.process_relationship_changes(new_stix_obj, domain)
265265

266266
# Process new objects
267267
self._process_additions(changes["additions"], new_objects)
@@ -283,7 +283,7 @@ def _process_additions(self, additions: set, new_objects: dict):
283283
attack_id = get_attack_id(new_stix_obj)
284284

285285
# Add contributions from additions
286-
self._contributor_tracker.update_contributors(old_object=None, new_object=new_stix_obj)
286+
self.contributor_tracker.update_contributors(old_object=None, new_object=new_stix_obj)
287287

288288
# Verify version is 1.0
289289
x_mitre_version = get_attack_object_version(stix_obj=new_stix_obj)
@@ -354,30 +354,6 @@ def _store_categorized_changes(
354354
new_objects[stix_id] for stix_id in changes["unchanged"]
355355
]
356356

357-
def find_technique_mitigation_changes(self, new_stix_obj: dict, domain: str):
358-
"""Find changes in the relationships between Techniques and Mitigations.
359-
360-
Parameters
361-
----------
362-
new_stix_obj : dict
363-
An ATT&CK Technique (attack-pattern) STIX Domain Object (SDO).
364-
domain : str
365-
An ATT&CK domain from the following list ["enterprise-attack", "mobile-attack", "ics-attack"]
366-
"""
367-
return self._change_detector.find_technique_mitigation_changes(new_stix_obj, domain)
368-
369-
def find_technique_detection_changes(self, new_stix_obj: dict, domain: str):
370-
"""Find changes in the relationships between Techniques and Datacomponents.
371-
372-
Parameters
373-
----------
374-
new_stix_obj : dict
375-
An ATT&CK Technique (attack-pattern) STIX Domain Object (SDO).
376-
domain : str
377-
An ATT&CK domain from the following list ["enterprise-attack", "mobile-attack", "ics-attack"]
378-
"""
379-
return self._change_detector.find_technique_detection_changes(new_stix_obj, domain)
380-
381357
def get_datastore_from_mitre_cti(self, domain: str, datastore_version: str) -> stix2.MemoryStore:
382358
"""Load data from MITRE CTI repo according to domain.
383359
@@ -394,139 +370,21 @@ def get_datastore_from_mitre_cti(self, domain: str, datastore_version: str) -> s
394370
STIX MemoryStore object representing an ATT&CK domain.
395371
"""
396372
# Lazy initialization for backward compatibility with tests
397-
if not hasattr(self, "_data_loader"):
398-
self._data_loader = DataLoader(self)
399-
return self._data_loader.get_datastore_from_mitre_cti(domain, datastore_version)
400-
401-
def update_contributors(self, old_object: Optional[dict], new_object: dict):
402-
"""Update contributors list if new object has contributors.
403-
404-
Parameters
405-
----------
406-
old_object : Optional[dict]
407-
An ATT&CK STIX Domain Object (SDO).
408-
new_object : dict
409-
An ATT&CK STIX Domain Object (SDO).
410-
"""
411-
self._contributor_tracker.update_contributors(old_object, new_object)
412-
413-
def get_groupings(self, object_type: str, stix_objects: List, section: str, domain: str) -> List[Dict[str, object]]:
414-
"""Group STIX objects together within a section.
415-
416-
A "group" in this sense is a set of STIX objects that are all in the same section, e.g. new minor version.
417-
In this case, since a domain/object type are implied before we get here, it would be
418-
e.g. "All Enterprise Techniques & Subtechniques, grouped alphabetically by name, and the
419-
sub-techniques are 'grouped' under their parent technique"
420-
421-
Parameters
422-
----------
423-
object_type : str
424-
Type of STIX object that is being worked with.
425-
stix_objects : List
426-
List of STIX objects that need to be grouped.
427-
section : str
428-
Section of the changelog that is being created with the objects,
429-
e.g. new major version, revocation, etc.
430-
domain : str
431-
ATT&CK domain (e.g., "enterprise-attack")
432-
433-
Returns
434-
-------
435-
List[Dict[str, object]]
436-
A list of sorted, complex dictionary objects that tell if this "group" of objects have
437-
their parent objects in the same section.
438-
"""
439-
return self._hierarchy_builder.get_groupings(object_type, stix_objects, section, domain)
440-
441-
def get_contributor_section(self) -> str:
442-
"""Get contributors that are only found in the new STIX data.
443-
444-
Returns
445-
-------
446-
str
447-
Markdown representation of the contributors found
448-
"""
449-
return self._contributor_tracker.get_contributor_section()
450-
451-
def get_parent_stix_object(self, stix_object: dict, datastore_version: str, domain: str) -> dict:
452-
"""Given an ATT&CK STIX object, find and return its parent STIX object.
453-
454-
Parameters
455-
----------
456-
stix_object : dict
457-
An ATT&CK STIX Domain Object (SDO).
458-
datastore_version : str
459-
The comparative version of the ATT&CK datastore. Choices are either "old" or "new".
460-
domain : str
461-
An ATT&CK domain from the following list ["enterprise-attack", "mobile-attack", "ics-attack"]
462-
463-
Returns
464-
-------
465-
dict
466-
The parent STIX object, if one can be found. Otherwise an empty dictionary is returned.
467-
"""
468-
return self._hierarchy_builder.get_parent_stix_object(stix_object, datastore_version, domain)
469-
470-
def placard(self, stix_object: dict, section: str, domain: str) -> str:
471-
"""Get a section list item for the given STIX Domain Object (SDO) according to section type.
472-
473-
Parameters
474-
----------
475-
stix_object : dict
476-
An ATT&CK STIX Domain Object (SDO).
477-
section : str
478-
Section change type, e.g major_version_change, revocations, etc.
479-
domain : str
480-
An ATT&CK domain from the following list ["enterprise-attack", "mobile-attack", "ics-attack"]
481-
482-
Returns
483-
-------
484-
str
485-
Final return string to be displayed in the Changelog.
486-
"""
487-
return self._markdown_generator.placard(stix_object, section, domain)
488-
489-
def get_statistics_section(self, datastore_version: str = "new") -> str:
490-
"""Generate a markdown section with ATT&CK statistics for all domains.
491-
492-
Parameters
493-
----------
494-
datastore_version : str, optional
495-
Either "old" or "new" to specify which version's statistics to generate.
496-
Defaults to "new".
497-
498-
Returns
499-
-------
500-
str
501-
Markdown-formatted statistics section.
502-
"""
503-
return self._statistics_collector.generate_statistics_section(datastore_version)
504-
505-
def get_markdown_section_data(self, groupings, section: str, domain: str) -> str:
506-
"""Parse a list of STIX objects in a section and return a string for the whole section."""
507-
return self._markdown_generator.get_markdown_section_data(groupings, section, domain)
508-
509-
def get_md_key(self) -> str:
510-
"""Create string describing each type of difference (change, addition, etc).
511-
512-
Returns
513-
-------
514-
str
515-
Key for change types used in Markdown output.
516-
"""
517-
return self._markdown_generator.get_md_key()
373+
if not hasattr(self, "data_loader"):
374+
self.data_loader = DataLoader(self)
375+
return self.data_loader.get_datastore_from_mitre_cti(domain, datastore_version)
518376

519377
def get_markdown_string(self) -> str:
520378
"""Return a markdown string summarizing detected differences."""
521-
return self._markdown_generator.generate()
379+
return self.markdown_generator.generate()
522380

523381
def get_layers_dict(self):
524382
"""Return ATT&CK Navigator layers in dict format summarizing detected differences.
525383
526384
Returns a dict mapping domain to its layer dict.
527385
"""
528-
return self._layer_generator.generate()
386+
return self.layer_generator.generate()
529387

530388
def get_changes_dict(self):
531389
"""Return dict format summarizing detected differences."""
532-
return self._json_generator.generate()
390+
return self.json_generator.generate()

mitreattack/diffStix/formatters/json_generator.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def generate(self) -> dict:
3737
changes_dict[domain][object_type] = {}
3838

3939
for section, stix_objects in sections.items():
40-
groupings = self.diff_stix._hierarchy_builder.get_groupings(
40+
groupings = self.diff_stix.hierarchy_builder.get_groupings(
4141
object_type=object_type,
4242
stix_objects=stix_objects,
4343
section=section,

mitreattack/diffStix/formatters/markdown_generator.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ def generate(self) -> str:
7474
for section, stix_objects in self.diff_stix.data["changes"][object_type][domain].items():
7575
header = f"#### {section_headers[section]}"
7676
if stix_objects:
77-
groupings = self.diff_stix._hierarchy_builder.get_groupings(
77+
groupings = self.diff_stix.hierarchy_builder.get_groupings(
7878
object_type=object_type,
7979
stix_objects=stix_objects,
8080
section=section,

tests/changelog/core/test_diffstix_methods.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ def test_get_groupings_real_functionality(
2020
test_objects = [parent_technique, child_subtechnique]
2121

2222
# Test real groupings generation
23-
result = lightweight_diffstix.get_groupings(
23+
result = lightweight_diffstix.hierarchy_builder.get_groupings(
2424
object_type="techniques", stix_objects=test_objects, section="additions", domain="enterprise-attack"
2525
)
2626

@@ -49,7 +49,7 @@ def test_update_contributors_real_functionality(self, lightweight_diffstix, mock
4949
lightweight_diffstix.release_contributors = {}
5050

5151
# Test real contributor update
52-
lightweight_diffstix.update_contributors(old_object, new_object)
52+
lightweight_diffstix.contributor_tracker.update_contributors(old_object, new_object)
5353

5454
# Verify only new contributors were tracked
5555
assert "New Contributor" in lightweight_diffstix.release_contributors
@@ -65,7 +65,7 @@ def test_update_contributors_new_object_no_old(self, lightweight_diffstix, mock_
6565
lightweight_diffstix.release_contributors = {}
6666

6767
# Test with no old object (new addition)
68-
lightweight_diffstix.update_contributors(None, new_object)
68+
lightweight_diffstix.contributor_tracker.update_contributors(None, new_object)
6969

7070
# Verify all contributors were tracked
7171
assert "Author One" in lightweight_diffstix.release_contributors
@@ -100,7 +100,7 @@ def test_get_parent_stix_object_real_functionality(
100100
}
101101

102102
# Test real parent resolution
103-
result = lightweight_diffstix.get_parent_stix_object(subtechnique, "new", "enterprise-attack")
103+
result = lightweight_diffstix.hierarchy_builder.get_parent_stix_object(subtechnique, "new", "enterprise-attack")
104104

105105
# Verify correct parent was found
106106
assert result == parent_technique
@@ -110,21 +110,21 @@ def test_get_parent_stix_object_real_functionality(
110110
def test_get_parent_stix_object_no_parent(self, lightweight_diffstix, sample_technique_object):
111111
"""Test parent resolution for objects without parents."""
112112
# Test with regular technique (not a subtechnique)
113-
result = lightweight_diffstix.get_parent_stix_object(sample_technique_object, "new", "enterprise-attack")
113+
result = lightweight_diffstix.hierarchy_builder.get_parent_stix_object(sample_technique_object, "new", "enterprise-attack")
114114

115115
# Should return empty dict for objects without parents
116116
assert result == {}
117117

118118
def test_placard_different_sections(self, lightweight_diffstix, sample_technique_object, mock_stix_object_factory):
119119
"""Test real placard generation for different section types."""
120120
# Test additions section
121-
additions_result = lightweight_diffstix.placard(sample_technique_object, "additions", "enterprise-attack")
121+
additions_result = lightweight_diffstix.markdown_generator.placard(sample_technique_object, "additions", "enterprise-attack")
122122
assert isinstance(additions_result, str)
123123
assert len(additions_result) > 0
124124
assert "T1234" in additions_result
125125

126126
# Test deletions section
127-
deletions_result = lightweight_diffstix.placard(sample_technique_object, "deletions", "enterprise-attack")
127+
deletions_result = lightweight_diffstix.markdown_generator.placard(sample_technique_object, "deletions", "enterprise-attack")
128128
assert isinstance(deletions_result, str)
129129
# Deletions only show name, no link
130130
assert sample_technique_object["name"] in deletions_result
@@ -139,7 +139,7 @@ def test_placard_with_revocations(self, lightweight_diffstix, mock_stix_object_f
139139
revoked_object["revoked_by"] = revoking_object
140140

141141
# Test revocation placard
142-
result = lightweight_diffstix.placard(revoked_object, "revocations", "enterprise-attack")
142+
result = lightweight_diffstix.markdown_generator.placard(revoked_object, "revocations", "enterprise-attack")
143143

144144
# Verify revocation information is included
145145
assert isinstance(result, str)

0 commit comments

Comments
 (0)