From 045fe7f6c22ca32c7cbedf1bd02f3dac83d39628 Mon Sep 17 00:00:00 2001 From: Kenneth Belitzky Date: Sun, 27 Jul 2025 15:22:55 +0000 Subject: [PATCH 1/2] feat: support multiple --mappings-file flags with deep merge - Modified GenerateCommand to accept multiple --mappings-file arguments using action='append' - Added _deep_merge_dicts method for intelligent merging of mapping files - Later files override earlier ones for conflicting keys, with deep merging for nested dicts - Updated documentation to explain merging behavior and provide examples - Added comprehensive tests for deep merge functionality and multiple mappings file handling Resolves #66 --- docs/mappings.md | 19 ++++- docs/usage.md | 2 +- struct_module/commands/generate.py | 38 ++++++--- tests/test_commands.py | 119 +++++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 13 deletions(-) diff --git a/docs/mappings.md b/docs/mappings.md index 20545c4..e9c6bcd 100644 --- a/docs/mappings.md +++ b/docs/mappings.md @@ -151,7 +151,7 @@ struct generate --mappings-file ./mymap.yaml file://my-struct.yaml . ### Multiple Mappings Files -You can specify multiple mappings files that will be merged: +You can specify multiple mappings files that will be merged in order: ```sh struct generate \ @@ -160,7 +160,22 @@ struct generate \ file://my-struct.yaml . ``` -Later files override earlier ones for conflicting keys. +**Merging behavior:** + +- Files are processed in the order specified +- Later files override earlier ones for conflicting keys +- Deep merging is performed for nested dictionaries +- This enables clean separation of common vs environment-specific configuration + +**Example with environment variable:** + +```sh +struct generate \ + --mappings-file ./mappings/common.yaml \ + --mappings-file ./mappings/${ENVIRONMENT}.yaml \ + file://infrastructure.yaml \ + ./output +``` ## Practical Examples diff --git a/docs/usage.md b/docs/usage.md index 8856ebb..b716b60 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -44,7 +44,7 @@ struct generate \ - `--backup`: Specify backup directory for existing files - `--file-strategy`: Choose how to handle existing files (overwrite, skip, append, rename, backup) - `--log-file`: Write logs to specified file -- `--mappings-file`: Provide external mappings file +- `--mappings-file`: Provide external mappings file (can be used multiple times) ## Generate Schema Command diff --git a/struct_module/commands/generate.py b/struct_module/commands/generate.py index 46a0d56..2399b33 100644 --- a/struct_module/commands/generate.py +++ b/struct_module/commands/generate.py @@ -22,12 +22,24 @@ def __init__(self, parser): parser.add_argument('-f', '--file-strategy', type=str, choices=['overwrite', 'skip', 'append', 'rename', 'backup'], default='overwrite', help='Strategy for handling existing files').completer = file_strategy_completer parser.add_argument('-p', '--global-system-prompt', type=str, help='Global system prompt for OpenAI') parser.add_argument('--non-interactive', action='store_true', help='Run the command in non-interactive mode') - parser.add_argument('--mappings-file', type=str, - help='Path to a YAML file containing mappings to be used in templates') + parser.add_argument('--mappings-file', type=str, action='append', + help='Path to a YAML file containing mappings to be used in templates (can be specified multiple times)') parser.add_argument('-o', '--output', type=str, choices=['console', 'file'], default='file', help='Output mode') parser.set_defaults(func=self.execute) + def _deep_merge_dicts(self, dict1, dict2): + """ + Deep merge two dictionaries, with dict2 values overriding dict1 values. + """ + result = dict1.copy() + for key, value in dict2.items(): + if key in result and isinstance(result[key], dict) and isinstance(value, dict): + result[key] = self._deep_merge_dicts(result[key], value) + else: + result[key] = value + return result + def _run_hooks(self, hooks, hook_type="pre"): # helper for running hooks if not hooks: return True @@ -75,14 +87,20 @@ def execute(self, args): # Load mappings if provided mappings = {} if getattr(args, 'mappings_file', None): - if os.path.exists(args.mappings_file): - with open(args.mappings_file, 'r') as mf: - try: - mappings = yaml.safe_load(mf) or {} - except Exception as e: - self.logger.error(f"Failed to load mappings file: {e}") - else: - self.logger.error(f"Mappings file not found: {args.mappings_file}") + for mappings_file_path in args.mappings_file: + if os.path.exists(mappings_file_path): + self.logger.info(f"Loading mappings from: {mappings_file_path}") + with open(mappings_file_path, 'r') as mf: + try: + file_mappings = yaml.safe_load(mf) or {} + # Deep merge the mappings, with later files overriding earlier ones + mappings = self._deep_merge_dicts(mappings, file_mappings) + except Exception as e: + self.logger.error(f"Failed to load mappings file {mappings_file_path}: {e}") + return + else: + self.logger.error(f"Mappings file not found: {mappings_file_path}") + return if args.backup and not os.path.exists(args.backup): os.makedirs(args.backup) diff --git a/tests/test_commands.py b/tests/test_commands.py index b40dc20..5d88a6e 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -400,3 +400,122 @@ def mock_relpath_side_effect(file_path, base_path): structures = schema['definitions']['PluginList']['enum'] assert 'builtin' in structures assert len(structures) == 1 + +def test_deep_merge_dicts(): + """Test the _deep_merge_dicts functionality""" + parser = argparse.ArgumentParser() + command = GenerateCommand(parser) + + dict1 = { + 'a': 1, + 'b': { + 'c': 2, + 'd': 3 + }, + 'e': 4 + } + + dict2 = { + 'b': { + 'd': 30, + 'f': 40 + }, + 'g': 50 + } + + result = command._deep_merge_dicts(dict1, dict2) + + expected = { + 'a': 1, + 'b': { + 'c': 2, + 'd': 30, # overridden by dict2 + 'f': 40 # added from dict2 + }, + 'e': 4, + 'g': 50 # added from dict2 + } + + assert result == expected + +def test_multiple_mappings_files(): + """Test loading and merging multiple mappings files""" + parser = argparse.ArgumentParser() + command = GenerateCommand(parser) + + # Mock two mappings files + mappings1_content = { + 'mappings': { + 'teams': { + 'devops': 'devops-team' + }, + 'environments': { + 'dev': { + 'database_url': 'postgres://dev-db:5432' + } + } + } + } + + mappings2_content = { + 'mappings': { + 'teams': { + 'frontend': 'frontend-team' + }, + 'environments': { + 'dev': { + 'debug': True + }, + 'prod': { + 'database_url': 'postgres://prod-db:5432' + } + } + } + } + + with patch('os.path.exists', return_value=True), \ + patch('builtins.open', new_callable=MagicMock) as mock_open, \ + patch('yaml.safe_load', side_effect=[mappings1_content, mappings2_content]), \ + patch.object(command, '_create_structure') as mock_create_structure: + + # Create args with multiple mappings files + args = argparse.Namespace( + structure_definition='test.yaml', + base_path='/tmp/test', + mappings_file=['mappings1.yaml', 'mappings2.yaml'], + backup=None, + output='file' + ) + + # Mock config loading + with patch.object(command, '_load_yaml_config', return_value={'files': []}): + command.execute(args) + + # Verify both files were attempted to be opened + assert mock_open.call_count == 2 + + # Verify _create_structure was called with merged mappings + mock_create_structure.assert_called_once() + call_args = mock_create_structure.call_args[0] + merged_mappings = call_args[1] # Second argument is mappings + + # Verify the mappings were merged correctly + expected_mappings = { + 'mappings': { + 'teams': { + 'devops': 'devops-team', + 'frontend': 'frontend-team' + }, + 'environments': { + 'dev': { + 'database_url': 'postgres://dev-db:5432', + 'debug': True + }, + 'prod': { + 'database_url': 'postgres://prod-db:5432' + } + } + } + } + + assert merged_mappings == expected_mappings From 44369a3b88a3737c0d12b64da1cadf7bb1b58852 Mon Sep 17 00:00:00 2001 From: Kenneth Belitzky Date: Sun, 27 Jul 2025 15:30:23 +0000 Subject: [PATCH 2/2] fix: add structures_path parameter to test_multiple_mappings_files --- tests/test_commands.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index 5d88a6e..370b11f 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -484,7 +484,8 @@ def test_multiple_mappings_files(): base_path='/tmp/test', mappings_file=['mappings1.yaml', 'mappings2.yaml'], backup=None, - output='file' + output='file', + structures_path=None ) # Mock config loading