From 8013f71123bb34e70dad019424b7b506a5c2ebe2 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Sun, 11 Jan 2026 14:52:54 +0000 Subject: [PATCH 1/5] fix: display sensitivity prompt only for item definition --- .../unreleased/fixed-20260111-145147.yaml | 6 ++ src/fabric_cli/commands/fs/fab_fs_get.py | 4 +- .../commands/fs/get/fab_fs_get_item.py | 7 +- src/fabric_cli/core/fab_constant.py | 10 --- src/fabric_cli/utils/fab_cmd_get_utils.py | 36 +++++++++ tests/test_utils/test_fab_cmd_get_utils.py | 73 +++++++++++++++++++ 6 files changed, 120 insertions(+), 16 deletions(-) create mode 100644 .changes/unreleased/fixed-20260111-145147.yaml create mode 100644 tests/test_utils/test_fab_cmd_get_utils.py diff --git a/.changes/unreleased/fixed-20260111-145147.yaml b/.changes/unreleased/fixed-20260111-145147.yaml new file mode 100644 index 00000000..0a010ea2 --- /dev/null +++ b/.changes/unreleased/fixed-20260111-145147.yaml @@ -0,0 +1,6 @@ +kind: fixed +body: display sensitivity label prompt only when item definition is requested +time: 2026-01-11T14:51:47.974991712Z +custom: + Author: Aviat Cohen + AuthorLink: https://github.com/Aviat Cohen diff --git a/src/fabric_cli/commands/fs/fab_fs_get.py b/src/fabric_cli/commands/fs/fab_fs_get.py index 6712036f..bd33ac56 100644 --- a/src/fabric_cli/commands/fs/fab_fs_get.py +++ b/src/fabric_cli/commands/fs/fab_fs_get.py @@ -35,6 +35,7 @@ VirtualWorkspaceItem, Workspace, ) +from fabric_cli.utils import fab_cmd_get_utils as get_utils from fabric_cli.utils import fab_item_util, fab_ui, fab_util @@ -98,7 +99,8 @@ def _get_virtual_item(virtual_item: VirtualItem, args: Namespace) -> None: def _validate_sensitivity_label_warning(args: Namespace, item: Item) -> bool: # refactor to make the condition for get item with definition in one place - if args.query and args.query in fab_constant.ITEM_METADATA_PROPERTIES: + if args.query and not get_utils.should_retrieve_definition(args.query): + # Query is metadata-only, skip sensitivity warning return True try: diff --git a/src/fabric_cli/commands/fs/get/fab_fs_get_item.py b/src/fabric_cli/commands/fs/get/fab_fs_get_item.py index a5283f18..856fd099 100644 --- a/src/fabric_cli/commands/fs/get/fab_fs_get_item.py +++ b/src/fabric_cli/commands/fs/get/fab_fs_get_item.py @@ -7,7 +7,6 @@ from fabric_cli.client import fab_api_item as item_api from fabric_cli.client import fab_api_jobs as jobs_api -from fabric_cli.core import fab_constant from fabric_cli.core.fab_types import ItemType from fabric_cli.core.hiearchy.fab_hiearchy import Item from fabric_cli.utils import fab_cmd_get_utils as utils_get @@ -20,10 +19,8 @@ def exec( verbose: bool = True, decode: Optional[bool] = True, ) -> dict: - # If no payload query, no need to obtain definition - obtain_definition = True - if args.query and args.query in fab_constant.ITEM_METADATA_PROPERTIES: - obtain_definition = False + # Determine if we need to obtain definition based on query + obtain_definition = utils_get.should_retrieve_definition(args.query) item_def = item_utils.get_item_with_definition( item, args, decode, obtain_definition diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index d979b241..51f5b658 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -316,16 +316,6 @@ INTERACTIVE_HELP_COMMANDS = ["help", "h", "-h", "--help"] INTERACTIVE_VERSION_COMMANDS = ["version", "v", "-v", "--version"] -# Platform metadata -ITEM_METADATA_PROPERTIES = { - "id", - "type", - "displayName", - "description", - "workspaceId", - "folderId", -} - # Item set constants ITEM_QUERY_DEFINITION = "definition" ITEM_QUERY_PROPERTIES = "properties" diff --git a/src/fabric_cli/utils/fab_cmd_get_utils.py b/src/fabric_cli/utils/fab_cmd_get_utils.py index 989b36bc..58b8728f 100644 --- a/src/fabric_cli/utils/fab_cmd_get_utils.py +++ b/src/fabric_cli/utils/fab_cmd_get_utils.py @@ -13,6 +13,42 @@ from fabric_cli.utils import fab_ui as utils_ui +def should_retrieve_definition(query: str) -> bool: + """ + Determine if a query should retrieve item definition data. + + Definition data includes sensitive information that may have sensitivity labels, + so this function helps determine when definition retrieval is needed. + + Examples: + - "definition" -> True (definition data needed) + - "definition.parts" -> True (nested definition data needed) + - "." -> True (full item - includes definition) + - "properties.connectionString" -> False (just metadata, no definition needed) + - "displayName" -> False (just metadata, no definition needed) + + Args: + query: The query string to check + + Returns: + bool: True if query requires definition retrieval, False if metadata-only + """ + if not query: + # No query means full item retrieval (includes definition) + return True + + # Full item query - retrieves everything including definition + if query == ".": + return True + + # Definition queries - direct or nested + if query == "definition" or query.startswith("definition."): + return True + + # All other queries are metadata-only + return False + + def query_and_export( data: Any, args: Namespace, file_name: str, verbose: bool = True ) -> None: diff --git a/tests/test_utils/test_fab_cmd_get_utils.py b/tests/test_utils/test_fab_cmd_get_utils.py new file mode 100644 index 00000000..66d3ec47 --- /dev/null +++ b/tests/test_utils/test_fab_cmd_get_utils.py @@ -0,0 +1,73 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +from fabric_cli.utils.fab_cmd_get_utils import should_retrieve_definition + + +def test_should_retrieve_definition_scenarios(): + """Test the should_retrieve_definition function with various query scenarios.""" + + # Definition queries should return True (need definition retrieval) + definition_queries = [ + "definition", + "definition.parts", + "definition.content.sql", + "definition.nested.deeply.property", + ] + + for query in definition_queries: + result = should_retrieve_definition(query) + assert result is True, f"Query '{query}' should return True (definition retrieval needed)" + + # Full item query should return True (needs definition retrieval) + assert should_retrieve_definition(".") is True, "Full item query '.' should return True (definition retrieval needed)" + + # Empty/None query should return True (full item retrieval includes definition) + assert should_retrieve_definition("") is True, "Empty query should return True (full item includes definition)" + assert should_retrieve_definition(None) is True, "None query should return True (full item includes definition)" + + # Metadata-only queries should return False (no definition retrieval needed) + metadata_queries = [ + "properties", + "properties.connectionString", + "id", + "type", + "displayName", + "description", + "workspaceId", + "folderId", + "properties.nested.value", + "displayName.localized", + ] + + for query in metadata_queries: + result = should_retrieve_definition(query) + assert result is False, f"Query '{query}' should return False (metadata only, no definition needed)" + + # Other non-definition queries should return False + other_queries = [ + "someField", + "config.settings", + "data.values", + "random", + "definitionrelated", # similar but not definition + "definitions", # plural, not definition + "user.definition", # definition not at root + ] + + for query in other_queries: + result = should_retrieve_definition(query) + assert result is False, f"Query '{query}' should return False (no definition retrieval needed)" + + # Edge cases with dots and special formatting + edge_cases = [ + ("definition.", True), # definition with trailing dot (still needs definition) + (".definition", False), # definition not at start (metadata query) + ("definition..", True), # definition with double dot (still needs definition) + ("..definition", False), # definition not at start (metadata query) + ("..", False), # just dots (metadata query) + ] + + for query, expected in edge_cases: + result = should_retrieve_definition(query) + assert result == expected, f"Query '{query}' should return {expected}" \ No newline at end of file From 65d77dbbfcd239edcb424817b5176a1dd6b73f2f Mon Sep 17 00:00:00 2001 From: aviatco <32952699+aviatco@users.noreply.github.com> Date: Mon, 12 Jan 2026 16:24:57 +0200 Subject: [PATCH 2/5] Update .changes/unreleased/fixed-20260111-145147.yaml --- .changes/unreleased/fixed-20260111-145147.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/fixed-20260111-145147.yaml b/.changes/unreleased/fixed-20260111-145147.yaml index 0a010ea2..d476c4f9 100644 --- a/.changes/unreleased/fixed-20260111-145147.yaml +++ b/.changes/unreleased/fixed-20260111-145147.yaml @@ -2,5 +2,5 @@ kind: fixed body: display sensitivity label prompt only when item definition is requested time: 2026-01-11T14:51:47.974991712Z custom: - Author: Aviat Cohen + Author: aviatco AuthorLink: https://github.com/Aviat Cohen From e55c0b1e86d538ceb91e62240831dbd7cd161ef7 Mon Sep 17 00:00:00 2001 From: aviatco <32952699+aviatco@users.noreply.github.com> Date: Mon, 12 Jan 2026 16:25:25 +0200 Subject: [PATCH 3/5] Update .changes/unreleased/fixed-20260111-145147.yaml --- .changes/unreleased/fixed-20260111-145147.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/fixed-20260111-145147.yaml b/.changes/unreleased/fixed-20260111-145147.yaml index d476c4f9..87fd88ae 100644 --- a/.changes/unreleased/fixed-20260111-145147.yaml +++ b/.changes/unreleased/fixed-20260111-145147.yaml @@ -3,4 +3,4 @@ body: display sensitivity label prompt only when item definition is requested time: 2026-01-11T14:51:47.974991712Z custom: Author: aviatco - AuthorLink: https://github.com/Aviat Cohen + AuthorLink: https://github.com/aviatco From 25bfd8664d8c505182af17b1f4004ce4450a9e0c Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Mon, 12 Jan 2026 14:51:04 +0000 Subject: [PATCH 4/5] revert definition change and used ITEM_METADATA_PROPERTIES --- src/fabric_cli/commands/fs/fab_fs_get.py | 3 +- .../commands/fs/get/fab_fs_get_item.py | 6 +- src/fabric_cli/core/fab_constant.py | 11 +++ src/fabric_cli/utils/fab_cmd_get_utils.py | 41 ++++------- tests/test_commands/test_get.py | 20 +++++ tests/test_utils/test_fab_cmd_get_utils.py | 73 ------------------- 6 files changed, 50 insertions(+), 104 deletions(-) delete mode 100644 tests/test_utils/test_fab_cmd_get_utils.py diff --git a/src/fabric_cli/commands/fs/fab_fs_get.py b/src/fabric_cli/commands/fs/fab_fs_get.py index bd33ac56..6487458e 100644 --- a/src/fabric_cli/commands/fs/fab_fs_get.py +++ b/src/fabric_cli/commands/fs/fab_fs_get.py @@ -99,8 +99,7 @@ def _get_virtual_item(virtual_item: VirtualItem, args: Namespace) -> None: def _validate_sensitivity_label_warning(args: Namespace, item: Item) -> bool: # refactor to make the condition for get item with definition in one place - if args.query and not get_utils.should_retrieve_definition(args.query): - # Query is metadata-only, skip sensitivity warning + if args.query and get_utils.is_metadata_property_query(args.query): return True try: diff --git a/src/fabric_cli/commands/fs/get/fab_fs_get_item.py b/src/fabric_cli/commands/fs/get/fab_fs_get_item.py index 856fd099..20a8eb28 100644 --- a/src/fabric_cli/commands/fs/get/fab_fs_get_item.py +++ b/src/fabric_cli/commands/fs/get/fab_fs_get_item.py @@ -19,8 +19,10 @@ def exec( verbose: bool = True, decode: Optional[bool] = True, ) -> dict: - # Determine if we need to obtain definition based on query - obtain_definition = utils_get.should_retrieve_definition(args.query) + # If no payload query, no need to obtain definition + obtain_definition = True + if args.query and utils_get.is_metadata_property_query(args.query): + obtain_definition = False item_def = item_utils.get_item_with_definition( item, args, decode, obtain_definition diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index 51f5b658..51ef0e75 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -316,6 +316,17 @@ INTERACTIVE_HELP_COMMANDS = ["help", "h", "-h", "--help"] INTERACTIVE_VERSION_COMMANDS = ["version", "v", "-v", "--version"] +# Platform metadata +ITEM_METADATA_PROPERTIES = { + "id", + "type", + "displayName", + "description", + "workspaceId", + "folderId", + "properties", +} + # Item set constants ITEM_QUERY_DEFINITION = "definition" ITEM_QUERY_PROPERTIES = "properties" diff --git a/src/fabric_cli/utils/fab_cmd_get_utils.py b/src/fabric_cli/utils/fab_cmd_get_utils.py index 58b8728f..4d69ad98 100644 --- a/src/fabric_cli/utils/fab_cmd_get_utils.py +++ b/src/fabric_cli/utils/fab_cmd_get_utils.py @@ -9,44 +9,31 @@ from fabric_cli.client import fab_api_mirroring as mirroring_api from fabric_cli.utils import fab_jmespath as utils_jmespath from fabric_cli.utils import fab_storage as utils_storage -from fabric_cli.utils import fab_ui from fabric_cli.utils import fab_ui as utils_ui -def should_retrieve_definition(query: str) -> bool: +def is_metadata_property_query(query: str) -> bool: """ - Determine if a query should retrieve item definition data. - - Definition data includes sensitive information that may have sensitivity labels, - so this function helps determine when definition retrieval is needed. + Check if the query is for a metadata property or nested sub-property. Examples: - - "definition" -> True (definition data needed) - - "definition.parts" -> True (nested definition data needed) - - "." -> True (full item - includes definition) - - "properties.connectionString" -> False (just metadata, no definition needed) - - "displayName" -> False (just metadata, no definition needed) + - "properties" -> True (exact match) + - "properties.connectionString" -> True (nested property) + - "id" -> True (exact match) + - "someOtherField" -> False (not a metadata property) Args: - query: The query string to check + query: The query string to check (assumed to be non-empty) Returns: - bool: True if query requires definition retrieval, False if metadata-only + bool: True if query matches a metadata property or its nested properties """ - if not query: - # No query means full item retrieval (includes definition) - return True - - # Full item query - retrieves everything including definition - if query == ".": - return True - - # Definition queries - direct or nested - if query == "definition" or query.startswith("definition."): - return True - - # All other queries are metadata-only - return False + from fabric_cli.core import fab_constant + + # Extract root property and check if it's a valid metadata property + # This handles both exact matches and nested properties in one step + root_property = query.split('.')[0] + return root_property in fab_constant.ITEM_METADATA_PROPERTIES def query_and_export( diff --git a/tests/test_commands/test_get.py b/tests/test_commands/test_get.py index ec5bb801..16ed083c 100644 --- a/tests/test_commands/test_get.py +++ b/tests/test_commands/test_get.py @@ -355,6 +355,26 @@ def test_get_folder_success( # endregion + def test_metadata_property_query_validation(self): + """Test the enhanced metadata property validation for nested properties.""" + from fabric_cli.utils.fab_cmd_get_utils import is_metadata_property_query + + # Test exact matches + assert is_metadata_property_query("properties") is True + assert is_metadata_property_query("id") is True + assert is_metadata_property_query("displayName") is True + assert is_metadata_property_query("description") is True + + # Test nested properties - the key enhancement + assert is_metadata_property_query("properties.connectionString") is True + assert is_metadata_property_query("properties.nested.value") is True + assert is_metadata_property_query("displayName.localized") is True + + # Test invalid properties + assert is_metadata_property_query("someField") is False + assert is_metadata_property_query("definition") is False + assert is_metadata_property_query("definition.content") is False + # region Helper Methods def get(path, output=None, query=None, deep_traversal=False, force=False): diff --git a/tests/test_utils/test_fab_cmd_get_utils.py b/tests/test_utils/test_fab_cmd_get_utils.py deleted file mode 100644 index 66d3ec47..00000000 --- a/tests/test_utils/test_fab_cmd_get_utils.py +++ /dev/null @@ -1,73 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. - -from fabric_cli.utils.fab_cmd_get_utils import should_retrieve_definition - - -def test_should_retrieve_definition_scenarios(): - """Test the should_retrieve_definition function with various query scenarios.""" - - # Definition queries should return True (need definition retrieval) - definition_queries = [ - "definition", - "definition.parts", - "definition.content.sql", - "definition.nested.deeply.property", - ] - - for query in definition_queries: - result = should_retrieve_definition(query) - assert result is True, f"Query '{query}' should return True (definition retrieval needed)" - - # Full item query should return True (needs definition retrieval) - assert should_retrieve_definition(".") is True, "Full item query '.' should return True (definition retrieval needed)" - - # Empty/None query should return True (full item retrieval includes definition) - assert should_retrieve_definition("") is True, "Empty query should return True (full item includes definition)" - assert should_retrieve_definition(None) is True, "None query should return True (full item includes definition)" - - # Metadata-only queries should return False (no definition retrieval needed) - metadata_queries = [ - "properties", - "properties.connectionString", - "id", - "type", - "displayName", - "description", - "workspaceId", - "folderId", - "properties.nested.value", - "displayName.localized", - ] - - for query in metadata_queries: - result = should_retrieve_definition(query) - assert result is False, f"Query '{query}' should return False (metadata only, no definition needed)" - - # Other non-definition queries should return False - other_queries = [ - "someField", - "config.settings", - "data.values", - "random", - "definitionrelated", # similar but not definition - "definitions", # plural, not definition - "user.definition", # definition not at root - ] - - for query in other_queries: - result = should_retrieve_definition(query) - assert result is False, f"Query '{query}' should return False (no definition retrieval needed)" - - # Edge cases with dots and special formatting - edge_cases = [ - ("definition.", True), # definition with trailing dot (still needs definition) - (".definition", False), # definition not at start (metadata query) - ("definition..", True), # definition with double dot (still needs definition) - ("..definition", False), # definition not at start (metadata query) - ("..", False), # just dots (metadata query) - ] - - for query, expected in edge_cases: - result = should_retrieve_definition(query) - assert result == expected, f"Query '{query}' should return {expected}" \ No newline at end of file From a83a64a75a3dd5262da45ce39fff325fd783868a Mon Sep 17 00:00:00 2001 From: aviatco <32952699+aviatco@users.noreply.github.com> Date: Wed, 14 Jan 2026 12:33:06 +0200 Subject: [PATCH 5/5] Update .changes/unreleased/fixed-20260111-145147.yaml --- .changes/unreleased/fixed-20260111-145147.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/fixed-20260111-145147.yaml b/.changes/unreleased/fixed-20260111-145147.yaml index 87fd88ae..b068c71c 100644 --- a/.changes/unreleased/fixed-20260111-145147.yaml +++ b/.changes/unreleased/fixed-20260111-145147.yaml @@ -1,5 +1,5 @@ kind: fixed -body: display sensitivity label prompt only when item definition is requested +body: Add ‘properties’ field to the item metadata list to eliminate unnecessary calls to the getItemDefinition API time: 2026-01-11T14:51:47.974991712Z custom: Author: aviatco