diff --git a/src/aaz_dev/cli/controller/az_atomic_profile_builder.py b/src/aaz_dev/cli/controller/az_atomic_profile_builder.py index c43ab44b..ffc8b74c 100644 --- a/src/aaz_dev/cli/controller/az_atomic_profile_builder.py +++ b/src/aaz_dev/cli/controller/az_atomic_profile_builder.py @@ -26,8 +26,11 @@ def __init__(self, mod_name, by_patch=False): self._mod_name = mod_name self._aaz_spec_manager = AAZSpecsManager() self._by_patch = by_patch + self._cfg_readers = {} def __call__(self, view_profile): + # cache parsed cfgs per profile only: code generation may modify the command cfgs of a profile in place + self._cfg_readers = {} profile = CLIAtomicProfile() profile.name = view_profile.name if view_profile.command_groups: @@ -150,9 +153,15 @@ def _build_command_from_aaz(self, *names, version_name, load_cfg=True): }) if load_cfg: - # load cfg file, which will generate the command in code - cfg_reader = self._aaz_spec_manager.load_resource_cfg_reader_by_command_with_version( - aaz_cmd, version=version) + # load cfg file, which will generate the command in code. + # Commands of one resource share a cfg file (e.g. 146 application-gateway commands in a 15MB cfg), + # so parse each cfg file once instead of once per command. + resource = version.resources[0] + key = (resource.plane, resource.id, resource.version) + if key not in self._cfg_readers: + self._cfg_readers[key] = self._aaz_spec_manager.load_resource_cfg_reader_by_command_with_version( + aaz_cmd, version=version) + cfg_reader = self._cfg_readers[key] cmd_cfg = cfg_reader.find_command(*names) assert cmd_cfg is not None, f"command model miss in AAZ: '{' '.join(names)}'" diff --git a/src/aaz_dev/command/api/editor.py b/src/aaz_dev/command/api/editor.py index 4480f7a7..8be65eb6 100644 --- a/src/aaz_dev/command/api/editor.py +++ b/src/aaz_dev/command/api/editor.py @@ -305,7 +305,8 @@ def editor_workspace_examples(name, node_names, leaf_name): data = request.get_json() if 'examples' in data: leaf = manager.update_command_tree_leaf_examples(*leaf.names, examples=data['examples']) - cfg_editor = manager.load_cfg_editor_by_command(leaf) + # examples are stored in ws.json only, no need to modify the cfg + cfg_editor = manager.load_cfg_editor_by_command(leaf, readonly=True) command = cfg_editor.find_command(*leaf.names) result = command.to_primitive() @@ -338,7 +339,7 @@ def editor_workspace_command(name, node_names, leaf_name): if request.method == "GET": # get the command configuration - cfg_editor = manager.load_cfg_editor_by_command(leaf) + cfg_editor = manager.load_cfg_editor_by_command(leaf, readonly=True) command = cfg_editor.find_command(*leaf.names) result = command.to_primitive() elif request.method == "PATCH": @@ -350,10 +351,15 @@ def editor_workspace_command(name, node_names, leaf_name): leaf = manager.update_command_tree_leaf_stage(*leaf.names, stage=data['stage']) if 'examples' in data: leaf = manager.update_command_tree_leaf_examples(*leaf.names, examples=data['examples']) - cfg_editor = manager.load_cfg_editor_by_command(leaf) - if 'confirmation' in data: + # help, stage and examples are stored in ws.json only, load the cfg for modification only when needed + cfg_editor = manager.load_cfg_editor_by_command(leaf, readonly=True) + # web always sends confirmation, treat '' and None as the same + if 'confirmation' in data and \ + (data['confirmation'] or None) != (cfg_editor.find_command(*leaf.names).confirmation or None): + cfg_editor = manager.load_cfg_editor_by_command(leaf) cfg_editor.update_command_confirmation(*leaf.names, confirmation=data['confirmation']) if 'outputs' in data: + cfg_editor = manager.load_cfg_editor_by_command(leaf) cfg_editor.update_command_outputs(*leaf.names, outputs=data['outputs']) command = cfg_editor.find_command(*leaf.names) result = command.to_primitive() diff --git a/src/aaz_dev/command/controller/command_tree.py b/src/aaz_dev/command/controller/command_tree.py index b4361d39..42cc9022 100644 --- a/src/aaz_dev/command/controller/command_tree.py +++ b/src/aaz_dev/command/controller/command_tree.py @@ -592,14 +592,10 @@ def verify_updated_command_tree(self): details = {} for group_names in self._modified_command_groups: group = self.find_command_group(*group_names) - if group == self.root: + # deleted groups (e.g. renamed) are expected; save() removes their folders + if not group or group == self.root: continue - if not group: - details[' '.join(group_names)] = { - 'type': 'group', - 'help': "Group not found." - } - elif not group.help or not group.help.short: + if not group.help or not group.help.short: details[' '.join(group.names)] = { 'type': 'group', 'help': "Miss short summary." @@ -607,12 +603,10 @@ def verify_updated_command_tree(self): for cmd_names in self._modified_commands: cmd = self.find_command(*cmd_names) + # deleted commands (e.g. renamed) are expected; save() removes their files if not cmd: - details[' '.join(cmd_names)] = { - 'type': 'command', - 'help': "Command not found." - } - elif not cmd.help or not cmd.help.short: + continue + if not cmd.help or not cmd.help.short: details[' '.join(cmd.names)] = { 'type': 'command', 'help': "Miss short summary." diff --git a/src/aaz_dev/command/controller/workspace_manager.py b/src/aaz_dev/command/controller/workspace_manager.py index ca255806..866f6b17 100644 --- a/src/aaz_dev/command/controller/workspace_manager.py +++ b/src/aaz_dev/command/controller/workspace_manager.py @@ -2,6 +2,7 @@ import logging import os import shutil +import threading from datetime import datetime from command.model.configuration import CMDHelp, CMDResource, CMDCommandExample, CMDArg, CMDCommand, \ @@ -22,6 +23,22 @@ logger = logging.getLogger('aaz') +# Parsed cfg editors shared across requests, keyed by cfg path: {path: (file stats, editor)}. +# Readonly loads share snapshots. Writers load independent editors and publish them only after a successful save. +# Published editors must not be modified; subsequent writes load a new editor. +# FIFO cap on entries, switch to a memory based LRU if many big workspaces are opened in one process. +_CFG_EDITOR_CACHE = {} +_CFG_EDITOR_CACHE_MAX_SIZE = 32 +_CFG_EDITOR_CACHE_LOCK = threading.RLock() + + +def _get_cfg_editor_stats(ws_folder, cfg_editor): + stats = [] + for resource in cfg_editor.resources: + st = os.stat(WorkspaceCfgEditor.get_cfg_path(ws_folder, resource.id)) + stats.append((st.st_mtime_ns, st.st_size)) + return tuple(stats) + class WorkspaceManager: COMMAND_TREE_ROOT_NAME = "aaz" @@ -180,10 +197,14 @@ def save(self): remove_folders = [] update_files = [] + saved_cfg_editors = [] used_resources = set() for resource_id, cfg_editor in self._cfg_editors.items(): if resource_id in used_resources: continue + if not cfg_editor.deleted: + # Argument regeneration can replace class references without linking them. + cfg_editor.link() for r_id, data in cfg_editor.iter_cfg_files_data(): assert r_id not in used_resources if data is None: @@ -193,6 +214,8 @@ def save(self): update_files.append( (WorkspaceCfgEditor.get_cfg_path(self.folder, r_id), data)) used_resources.add(r_id) + if not cfg_editor.deleted: + saved_cfg_editors.append(cfg_editor) assert set(self._cfg_editors.keys()) == used_resources if not self.ws.mod_names or not self.ws.resource_provider: @@ -227,6 +250,9 @@ def save(self): with open(file_name, 'w', encoding="utf-8") as f: f.write(data) + for cfg_editor in saved_cfg_editors: + self._cache_cfg_editor(cfg_editor) + self._cfg_editors = {} self._client_cfg_editor = None @@ -365,25 +391,64 @@ def _unset_cfg_editor(self, cfg_editor): self._reusable_leaves[tuple( cmd_names)] = node.commands.pop(name) - def load_cfg_editor_by_resource(self, resource_id, version, reload=False): + def load_cfg_editor_by_resource(self, resource_id, version, reload=False, readonly=False): + """readonly: allow sharing a cached snapshot; the caller must not modify it.""" if not reload and resource_id in self._cfg_editors: # load from modified dict cfg_editor = self._cfg_editors[resource_id] return None if cfg_editor.deleted else cfg_editor assert not self.is_in_memory - try: - cfg_editor = WorkspaceCfgEditor.load_resource( - self.folder, resource_id, version) + # one global lock, so concurrent readers wait for one parse instead of parsing the same big cfg + # again. Parsing is CPU bound under the GIL, per-path locks would not make it faster. + with _CFG_EDITOR_CACHE_LOCK: + cfg_editor = self._get_cached_cfg_editor(resource_id, version) if readonly and not reload else None + if not cfg_editor: + try: + cfg_editor = WorkspaceCfgEditor.load_resource( + self.folder, resource_id, version) + except Exception as e: + logger.error( + f"load workspace resource cfg failed: {e}: {self.name} {resource_id} {version}") + return None + if readonly: + self._cache_cfg_editor(cfg_editor) + if not readonly: for resource in cfg_editor.resources: self._cfg_editors[resource.id] = cfg_editor - return cfg_editor - except Exception as e: - logger.error( - f"load workspace resource cfg failed: {e}: {self.name} {resource_id} {version}") + return cfg_editor + + def load_cfg_editor_by_command(self, cmd, reload=False, readonly=False): + return self.load_cfg_editor_by_resource( + cmd.resources[0].id, cmd.resources[0].version, reload=reload, readonly=readonly) + + def _get_cached_cfg_editor(self, resource_id, version): + entry = _CFG_EDITOR_CACHE.get(WorkspaceCfgEditor.get_cfg_path(self.folder, resource_id)) + if not entry: return None + stats, cfg_editor = entry + try: + valid = stats == _get_cfg_editor_stats(self.folder, cfg_editor) # not changed on disk + except OSError: + valid = False + if not valid: + for resource in cfg_editor.resources: + _CFG_EDITOR_CACHE.pop(WorkspaceCfgEditor.get_cfg_path(self.folder, resource.id), None) + if not valid or any(resource.version != version for resource in cfg_editor.resources): + return None + return cfg_editor - def load_cfg_editor_by_command(self, cmd, reload=False): - return self.load_cfg_editor_by_resource(cmd.resources[0].id, cmd.resources[0].version, reload=reload) + def _cache_cfg_editor(self, cfg_editor): + try: + stats = _get_cfg_editor_stats(self.folder, cfg_editor) + except OSError: + return + with _CFG_EDITOR_CACHE_LOCK: + for resource in cfg_editor.resources: + path = WorkspaceCfgEditor.get_cfg_path(self.folder, resource.id) + _CFG_EDITOR_CACHE.pop(path, None) + while len(_CFG_EDITOR_CACHE) >= _CFG_EDITOR_CACHE_MAX_SIZE: + _CFG_EDITOR_CACHE.pop(next(iter(_CFG_EDITOR_CACHE))) + _CFG_EDITOR_CACHE[path] = (stats, cfg_editor) def update_command_tree_node_help(self, *node_names, help): node = self.find_command_tree_node(*node_names) @@ -1096,8 +1161,7 @@ def generate_to_aaz(self): self.aaz_specs.update_client_cfg(editor.cfg) # update configurations - for ws_leaf in self.iter_command_tree_leaves(): - editor = self.load_cfg_editor_by_command(ws_leaf) + for editor in self._iter_leaf_cfg_editors(): cfg = editor.cfg self.aaz_specs.update_resource_cfg(cfg) @@ -1113,12 +1177,22 @@ def generate_to_aaz(self): self.aaz_specs.update_command_group_by_ws(ws_node) self.aaz_specs.save() + def _iter_leaf_cfg_editors(self): + """Yield the cfg editor of the command tree leaves, each editor only once. + Leaves of one resource share an editor, e.g. 146 application-gateway commands are in one 15MB cfg, and + processing that cfg once per leaf would load and parse the same big cfg from aaz for every leaf.""" + visited = set() + for ws_leaf in self.iter_command_tree_leaves(): + editor = self.load_cfg_editor_by_command(ws_leaf) + if id(editor) not in visited: + visited.add(id(editor)) + yield editor + def _merge_sub_resources_in_aaz(self): """Merge the commands of subresources which exported in aaz but not exist in current workspace""" updated_cfgs = [] inserted_commands = set() - for ws_leaf in self.iter_command_tree_leaves(): - editor = self.load_cfg_editor_by_command(ws_leaf) + for editor in self._iter_leaf_cfg_editors(): existing_sub_resources = {} for cmd_names, command in editor.iter_commands(): for r in command.resources: diff --git a/src/aaz_dev/command/tests/editor_tests/test_workspace.py b/src/aaz_dev/command/tests/editor_tests/test_workspace.py index 02edb177..4a406d53 100644 --- a/src/aaz_dev/command/tests/editor_tests/test_workspace.py +++ b/src/aaz_dev/command/tests/editor_tests/test_workspace.py @@ -1,9 +1,11 @@ from command.controller.workspace_manager import WorkspaceManager +from command.controller.workspace_cfg_editor import WorkspaceCfgEditor from command.controller.cfg_reader import _SchemaIdxEnum from command.tests.common import CommandTestCase, workspace_name from utils.plane import PlaneEnum import os import json +from unittest.mock import patch from utils import exceptions from swagger.utils.tools import swagger_resource_path_to_resource_id from swagger.utils.source import SourceTypeEnum @@ -43,6 +45,127 @@ def test_workspace_manager(self, ws_name): manager.save() +class WorkspaceCacheTest(CommandTestCase): + resource_id = "/tests" + version = "2021-01-01" + cmd_names = ("test", "create") + + def setUp(self): + super().setUp() + resource = { + "id": self.resource_id, + "version": self.version, + "swagger": "mgmt-plane/test/ResourceProviders/Microsoft.Test/Paths/L3Rlc3Rz/V/MjAyMS0wMS0wMQ==", + } + command = CMDCommand({ + "name": "create", + "version": self.version, + "description": "Create a test resource.", + "resources": [resource], + "operations": [{ + "operationId": "Tests_Create", + "http": { + "path": self.resource_id, + "request": { + "method": "put", + "body": {"json": {"schema": { + "type": "object", "name": "parameters", + "props": [ + {"type": "object", "name": "sku", "props": [ + {"type": "string", "name": "name"}, + ]}, + {"type": "object", "name": "first", "cls": "Shared", "props": [ + {"type": "string", "name": "value"}, + ]}, + {"type": "@Shared", "name": "second"}, + ], + }}}, + }, + "responses": [{"statusCode": [200]}], + }, + }], + }) + editor = WorkspaceCfgEditor(CMDConfiguration({ + "plane": PlaneEnum.Mgmt, + "resources": [resource], + "commandGroups": [{"name": "test", "commands": [command]}], + })) + command = editor.find_command(*self.cmd_names) + command.generate_args() + editor.reformat() + editor.link() + self.manager = WorkspaceManager.new( + self.__class__.__name__, plane=PlaneEnum.Mgmt, mod_names="test", + resource_provider="Microsoft.Test", source=SourceTypeEnum.OpenAPI, + ) + self.manager._set_cfg_editor(editor) + self.manager.save() + + def load_editor(self, readonly=False): + manager = WorkspaceManager(self.manager.name) + manager.load() + return manager, manager.load_cfg_editor_by_resource( + self.resource_id, self.version, readonly=readonly) + + def test_saved_cache_relinks_class_arguments(self): + manager, editor = self.load_editor() + original_ref, _ = editor.find_arg_by_var(*self.cmd_names, arg_var="$parameters.second") + editor.flatten_arg( + *self.cmd_names, arg_var="$parameters.sku", + sub_args_options={"$parameters.sku.name": ["sku-name"]}, + ) + regenerated_ref, _ = editor.find_arg_by_var(*self.cmd_names, arg_var="$parameters.second") + self.assertIsNot(regenerated_ref, original_ref) + serialized = editor.cfg.to_primitive() + manager.save() + + with patch.object(WorkspaceCfgEditor, 'load_resource', wraps=WorkspaceCfgEditor.load_resource) as parse: + _, cached = self.load_editor(readonly=True) + parse.assert_not_called() + cached_ref, _ = cached.find_arg_by_var(*self.cmd_names, arg_var="$parameters.second") + self.assertIsNotNone(cached_ref.implement) + self.assertEqual(cached.cfg.to_primitive(), serialized) + cached.cfg.validate() + + cold = WorkspaceCfgEditor.load_resource(manager.folder, self.resource_id, self.version) + cold.link() + cold.link() + self.assertEqual(cold.cfg.to_primitive(), serialized) + cold_ref, _ = cold.find_arg_by_var(*self.cmd_names, arg_var="$parameters.second") + self.assertEqual(cached_ref.implement.to_primitive(), cold_ref.implement.to_primitive()) + + next_manager, writable = self.load_editor() + self.assertIsNot(writable, cached) + writable.flatten_arg(*self.cmd_names, arg_var="$parameters.second") + next_manager.save() + _, saved = self.load_editor(readonly=True) + self.assertIsNone(saved.find_arg_by_var(*self.cmd_names, arg_var="$parameters.second")[0]) + self.assertIsNotNone(saved.find_arg_by_var(*self.cmd_names, arg_var="$parameters.second.value")[0]) + saved.cfg.validate() + self.assertEqual(cached.cfg.to_primitive(), serialized) + + def test_failed_save_preserves_readonly_snapshot(self): + _, cached = self.load_editor(readonly=True) + before = cached.cfg.to_primitive() + writer, writable = self.load_editor() + self.assertIsNot(writable, cached) + writable.update_command_confirmation(*self.cmd_names, confirmation="UNSAVED") + + competing = WorkspaceManager(self.manager.name) + competing.load() + competing.save() + with self.assertRaisesRegex(exceptions.InvalidAPIUsage, "Workspace Changed"): + writer.save() + + self.assertEqual(cached.cfg.to_primitive(), before) + with patch.object(WorkspaceCfgEditor, 'load_resource', wraps=WorkspaceCfgEditor.load_resource) as parse: + _, current = self.load_editor(readonly=True) + parse.assert_not_called() + self.assertEqual(current.cfg.to_primitive(), before) + cold = WorkspaceCfgEditor.load_resource(writer.folder, self.resource_id, self.version) + self.assertEqual(cold.cfg.to_primitive(), before) + + class WorkspaceEditorTest(CommandTestCase): @workspace_name("test_workspace_editor_add_resources_by_swagger") @@ -85,6 +208,63 @@ def test_workspace_editor_add_resources_by_swagger(self, ws_name): assert manager.load_cfg_editor_by_command(address_cg.commands['delete']) assert manager.load_cfg_editor_by_command(address_cg.commands['update']) + @workspace_name("test_workspace_editor_cfg_editor_cache") + def test_workspace_editor_cfg_editor_cache(self, ws_name): + mod_names = "cdn" + manager = WorkspaceManager.new(ws_name, plane=PlaneEnum.Mgmt, mod_names=mod_names, resource_provider="Microsoft.Cdn", source=SourceTypeEnum.OpenAPI) + manager.save() + version = '2021-06-01' + resource_id = swagger_resource_path_to_resource_id('/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Cdn/profiles/{profileName}/endpoints/{endpointName}') + manager.add_new_resources_by_swagger(mod_names=mod_names, version=version, resources=[{"id": resource_id}]) + manager.save() + cmd_names = ("cdn", "profile", "endpoint", "show") + cfg_path = WorkspaceCfgEditor.get_cfg_path(manager.folder, resource_id) + + def load(readonly=False): + m = WorkspaceManager(ws_name) + m.load() + return m, m.load_cfg_editor_by_resource(resource_id, version, readonly=readonly) + + # concurrent read only loads parse the cold cfg once and share it + from concurrent.futures import ThreadPoolExecutor + from command.controller import workspace_manager + workspace_manager._CFG_EDITOR_CACHE.clear() + with ThreadPoolExecutor(2) as pool: + (m, editor), (_, editor_b) = pool.map(lambda _: load(readonly=True), range(2)) + assert editor_b is editor + # read only editor is not saved + mtime = os.stat(cfg_path).st_mtime_ns + m.save() + assert os.stat(cfg_path).st_mtime_ns == mtime + + # writers load independently, but reuse their editor within the same manager + with patch.object(WorkspaceCfgEditor, 'load_resource', wraps=WorkspaceCfgEditor.load_resource) as parse: + m2, editor2 = load() + for leaf in m2.iter_command_tree_leaves(): + assert m2.load_cfg_editor_by_command(leaf) is editor2 + assert parse.call_count == 1 + assert editor2 is not editor + editor2.find_command(*cmd_names).confirmation = "unsaved" + m3, editor3 = load(readonly=True) + assert editor3 is editor and not editor3.find_command(*cmd_names).confirmation + + # saved edits are kept + m4, editor4 = load() + assert editor4 is not editor3 + editor4.update_command_confirmation(*cmd_names, confirmation="saved") + m4.save() + m5, editor5 = load(readonly=True) + assert editor5 is editor4 and editor5.find_command(*cmd_names).confirmation == "saved" + assert not editor3.find_command(*cmd_names).confirmation + + # file changed on disk is reloaded + with open(cfg_path, 'r', encoding="utf-8") as f: + data = json.load(f) + with open(cfg_path, 'w', encoding="utf-8") as f: + f.write(json.dumps(data, ensure_ascii=False) + " ") + m6, editor6 = load(readonly=True) + assert editor6 is not editor5 and editor6.find_command(*cmd_names).confirmation == "saved" + @workspace_name("test_workspace_editor_subresource") def test_workspace_editor_subresource(self, ws_name): mod_names = "cdn"