diff --git a/CHANGELOG.next.md b/CHANGELOG.next.md index 80cd540e17..0705d87f66 100644 --- a/CHANGELOG.next.md +++ b/CHANGELOG.next.md @@ -29,6 +29,8 @@ Thanks, you're awesome :-) --> #### Bugfixes +* Addressed issue where foreign reuses weren't using the user-supplied `as` value for their destination. #960 + #### Added * Introduced `--strict` flag to perform stricter schema validation when running the generator script. #937 diff --git a/scripts/schema/finalizer.py b/scripts/schema/finalizer.py index 45abb6a3a9..3d1c7202b2 100644 --- a/scripts/schema/finalizer.py +++ b/scripts/schema/finalizer.py @@ -57,17 +57,19 @@ def perform_reuse(fields): schema = fields[schema_name] for reuse_entry in reuse_entries: # print(order, "{} => {}".format(schema_name, reuse_entry['full'])) + nest_as = reuse_entry['as'] destination_schema_name = reuse_entry['full'].split('.')[0] destination_schema = fields[destination_schema_name] ensure_valid_reuse(schema, destination_schema) new_field_details = copy.deepcopy(schema['field_details']) + new_field_details['name'] = nest_as new_field_details['original_fieldset'] = schema_name new_field_details['intermediate'] = True reused_fields = copy.deepcopy(schema['fields']) set_original_fieldset(reused_fields, schema_name) destination_fields = field_group_at_path(reuse_entry['at'], fields) - destination_fields[schema_name] = { + destination_fields[nest_as] = { 'field_details': new_field_details, 'fields': reused_fields, } diff --git a/scripts/tests/unit/test_schema_finalizer.py b/scripts/tests/unit/test_schema_finalizer.py index 64f3f25458..8a193a0454 100644 --- a/scripts/tests/unit/test_schema_finalizer.py +++ b/scripts/tests/unit/test_schema_finalizer.py @@ -47,6 +47,8 @@ def schema_process(self): 'order': 2, 'expected': [ {'full': 'process.parent', 'at': 'process', 'as': 'parent'}, + {'full': 'reuse.process', 'at': 'reuse', 'as': 'process'}, + {'full': 'reuse.process.parent', 'at': 'reuse.process', 'as': 'parent'}, ] } }, @@ -143,30 +145,57 @@ def schema_server(self): } } + def schema_process_reuse(self): + return { + 'reuse': { + 'schema_details': { + 'title': 'Reuse', + 'root': False + }, + 'field_details': { + 'name': 'Reuse', + 'node_name': 'Reuse', + 'short': 'reuse example', + }, + 'fields': { + 'pid': { + 'field_details': { + 'name': 'pid', + 'node_name': 'pid', + } + } + } + } + } + # perform_reuse def test_perform_reuse_with_foreign_reuse_and_self_reuse(self): - fields = {**self.schema_user(), **self.schema_server(), **self.schema_process()} + fields = {**self.schema_user(), **self.schema_server(), **self.schema_process(), **self.schema_process_reuse()} # If the test had multiple foreign destinations for user fields, we could compare them together instead finalizer.perform_reuse(fields) process_fields = fields['process']['fields'] server_fields = fields['server']['fields'] user_fields = fields['user']['fields'] + process_reuse_fields = fields['reuse']['fields']['process']['fields'] # Expected reuse self.assertIn('parent', process_fields) self.assertIn('user', server_fields) self.assertIn('target', user_fields) self.assertIn('effective', user_fields) + self.assertIn('parent', process_reuse_fields) # Sanity check for presence of leaf fields, after performing reuse self.assertIn('name', user_fields['target']['fields']) self.assertIn('name', user_fields['effective']['fields']) self.assertIn('name', server_fields['user']['fields']) self.assertIn('pid', process_fields['parent']['fields']) + self.assertIn('pid', process_reuse_fields['parent']['fields']) # Ensure the parent field of reused fields is marked as intermediate self.assertTrue(server_fields['user']['field_details']['intermediate']) self.assertTrue(process_fields['parent']['field_details']['intermediate']) self.assertTrue(user_fields['target']['field_details']['intermediate']) self.assertTrue(user_fields['effective']['field_details']['intermediate']) + self.assertTrue(process_reuse_fields['parent']['field_details']['intermediate']) # No unexpected cross-nesting self.assertNotIn('target', user_fields['target']['fields']) self.assertNotIn('target', user_fields['effective']['fields']) @@ -176,6 +205,7 @@ def test_perform_reuse_with_foreign_reuse_and_self_reuse(self): self.assertIn('user.effective', fields['user']['schema_details']['nestings']) self.assertIn('user.target', fields['user']['schema_details']['nestings']) self.assertIn('server.user', fields['server']['schema_details']['nestings']) + self.assertIn('reuse.process.parent', fields['reuse']['schema_details']['nestings']) # Attribute 'reused_here' lists nestings inside a destination schema self.assertIn({'full': 'process.parent', 'schema_name': 'process', 'short': 'short desc'}, fields['process']['schema_details']['reused_here']) @@ -185,6 +215,8 @@ def test_perform_reuse_with_foreign_reuse_and_self_reuse(self): fields['user']['schema_details']['reused_here']) self.assertIn({'full': 'server.user', 'schema_name': 'user', 'short': 'short desc'}, fields['server']['schema_details']['reused_here']) + self.assertIn({'full': 'reuse.process.parent', 'schema_name': 'process', 'short': 'short desc'}, + fields['reuse']['schema_details']['reused_here']) # Reused fields have an indication they're reused self.assertEqual(process_fields['parent']['field_details']['original_fieldset'], 'process', "The parent field of reused fields should have 'original_fieldset' populated") @@ -193,6 +225,8 @@ def test_perform_reuse_with_foreign_reuse_and_self_reuse(self): self.assertEqual(server_fields['user']['field_details']['original_fieldset'], 'user', "The parent field of foreign reused fields should have 'original_fieldset' populated") self.assertEqual(server_fields['user']['fields']['name']['field_details']['original_fieldset'], 'user') + self.assertEqual(process_reuse_fields['parent']['field_details']['original_fieldset'], 'process', + "The parent field of reused fields should have 'original_fieldset' populated") # Original fieldset's fields must not be marked with 'original_fieldset=' self.assertNotIn('original_fieldset', user_fields['name']['field_details']) self.assertNotIn('original_fieldset', process_fields['pid']['field_details'])