From 0aafb8aeea48caf2d851b4112c2cd595699291bb Mon Sep 17 00:00:00 2001 From: Prabuddha Chakraborty Date: Sun, 24 May 2026 22:45:20 +0530 Subject: [PATCH 1/2] fix and optimise explorer --- examples/explorer.py | 22 +- src/pytfe/models/explorer.py | 33 +- src/pytfe/resources/explorer.py | 682 ++++++-------------------------- tests/units/test_explorer.py | 647 +++++++----------------------- 4 files changed, 302 insertions(+), 1082 deletions(-) diff --git a/examples/explorer.py b/examples/explorer.py index 50f27b5..f3d74fb 100644 --- a/examples/explorer.py +++ b/examples/explorer.py @@ -20,14 +20,12 @@ │ create_saved_view │ Create saved Explorer view │ organization: str; options: ExplorerSavedViewCreateOptions │ ExplorerSavedView │ │ read_saved_view │ Fetch one saved view by id │ organization: str; view_id: str │ ExplorerSavedView │ │ update_saved_view │ Update saved view definition │ organization: str; view_id: str; options: ExplorerSavedViewUpdateOptions │ ExplorerSavedView │ - │ delete_saved_view │ Remove saved view by id │ organization: str; view_id: str │ ExplorerSavedView │ + │ delete_saved_view │ Remove saved view by id │ organization: str; view_id: str │ None │ │ saved_view_results │ Execute saved view, stream rows │ organization: str; view_id: str │ Iterator[ExplorerRow] │ - │ saved_view_results_csv │ Saved view results as CSV │ organization: str; view_id: str │ str (CSV; fallbacks) │ + │ saved_view_results_csv │ Saved view results as CSV │ organization: str; view_id: str │ str (CSV) │ └────────────────────────┴────────────────────────────────────┴──────────────────────────────────────────────────────────────────────────────┴──────────────────────────────┘ - delete_saved_view: if the DELETE response has no JSON body, the client returns a - minimal ExplorerSavedView with the same id. - saved_view_results_csv: tries the saved-view CSV endpoint first; on failure it may - call export_csv after read_saved_view, or build CSV from saved_view_results. + saved_view_results_csv: hits the dedicated saved-view CSV endpoint + (``/explorer/views/{id}/csv``) and returns the response text verbatim. INPUT AND OUTPUT MODELS (how to pass; allowed values) ─────────────────────────────────────────────────────── @@ -95,9 +93,9 @@ list of strings (even for a single operand). Output models (return values only; you do not instantiate these for requests) - ExplorerRow — from query(), saved_view_results(): read .id, .row_type, .attributes. - .attributes is a dict of column values; keys may be hyphenated or snake_case depending - on the API field name. + ExplorerRow — from query(), saved_view_results(): read .id, .type, .attributes. + .attributes is a dict of column values; keys are normalised to snake_case at + parse time so callers can index ``row.attributes["workspace_name"]`` directly. ExplorerSavedView — from create_saved_view, read_saved_view, update_saved_view, delete_saved_view, list_saved_views: .id, .name, .created_at, .query_type, .query. str — from export_csv, saved_view_results_csv: raw CSV document body. @@ -208,7 +206,7 @@ def main() -> None: # from ExplorerQueryOptions. Here we request the workspaces view, sort by # workspace_name descending (leading hyphen in sort), and add a single URL-style # filter (workspace_name contains "42"). The iterator yields ExplorerRow objects - # (id, row_type, attributes dict); we only print the first five rows. + # (id, type, attributes dict); we only print the first five rows. _banner( "Step 1 of 7: query()", "Workspaces view, sorted by -workspace_name, filter workspace_name contains '42'.", @@ -236,7 +234,7 @@ def main() -> None: ) print(f" Row {count}:") print(f" id: {row.id}") - print(f" row_type: {row.row_type!r}") + print(f" type: {row.type!r}") print(f" workspace_name: {name!r}") print(" ---") print(f"Summary: printed {count} row(s) (limit 5).") @@ -333,7 +331,7 @@ def main() -> None: break print(f" Result row {i + 1}:") print(f" id: {row.id}") - print(f" row_type: {row.row_type!r}") + print(f" type: {row.type!r}") print(" ---") print("Summary: saved_view_results completed (limit 3 rows printed).") except TFEError as e: diff --git a/src/pytfe/models/explorer.py b/src/pytfe/models/explorer.py index 66d73aa..732b321 100644 --- a/src/pytfe/models/explorer.py +++ b/src/pytfe/models/explorer.py @@ -27,6 +27,8 @@ class ExplorerViewType(str, Enum): class ExplorerUrlFilter(BaseModel): """One slot in ExplorerQueryOptions.filters → filter[i][field][op][idx] query keys.""" + model_config = ConfigDict(populate_by_name=True, validate_by_name=True) + index: int = Field(..., ge=0, description="Filter index in the query string") field: str = Field( ..., min_length=1, description="Explorer field name in snake_case" @@ -43,7 +45,7 @@ class ExplorerUrlFilter(BaseModel): class ExplorerQueryOptions(BaseModel): """GET /organizations/{org}/explorer (and export/csv) query string as structured fields.""" - model_config = ConfigDict(populate_by_name=True) + model_config = ConfigDict(populate_by_name=True, validate_by_name=True) view_type: ExplorerViewType = Field(..., alias="type") sort: str | None = Field( @@ -63,18 +65,25 @@ class ExplorerQueryOptions(BaseModel): class ExplorerRow(BaseModel): - """One Explorer result row: json:api id/type plus flat attributes for the view.""" + """One Explorer result row: JSON:API id/type plus flat attributes for the view. + + Attribute keys are normalised to snake_case at parse time so callers can + index ``row.attributes["workspace_name"]`` rather than juggling hyphen vs + snake variants. + """ - model_config = ConfigDict(populate_by_name=True) + model_config = ConfigDict(populate_by_name=True, validate_by_name=True) id: str - row_type: str = Field(..., alias="type") + type: str attributes: dict[str, Any] = Field(default_factory=dict) class ExplorerSavedQueryFilter(BaseModel): """One saved-view filter row (list-valued `value` matches create/update JSON).""" + model_config = ConfigDict(populate_by_name=True, validate_by_name=True) + field: str = Field(..., min_length=1) operator: str = Field(..., min_length=1) value: list[str] = Field(default_factory=list) @@ -83,7 +92,7 @@ class ExplorerSavedQueryFilter(BaseModel): class ExplorerSavedQuery(BaseModel): """Nested query on a saved view: view type, filters, optional fields and sort lists.""" - model_config = ConfigDict(populate_by_name=True) + model_config = ConfigDict(populate_by_name=True, validate_by_name=True) query_type: ExplorerViewType = Field(..., alias="type") filter: list[ExplorerSavedQueryFilter] | None = None @@ -92,9 +101,15 @@ class ExplorerSavedQuery(BaseModel): class ExplorerSavedView(BaseModel): - """Saved view resource: metadata plus embedded query (response and some request paths).""" + """Saved view resource: metadata plus embedded query. + + The HCP Terraform API returns ``query-type`` at the view level *and* + ``type`` nested inside ``query``. They are always equal in practice; the + SDK surfaces both because they appear in different positions in the + request/response payload. + """ - model_config = ConfigDict(populate_by_name=True) + model_config = ConfigDict(populate_by_name=True, validate_by_name=True) id: str name: str @@ -106,7 +121,7 @@ class ExplorerSavedView(BaseModel): class ExplorerSavedViewCreateOptions(BaseModel): """POST .../explorer/views attributes: display name, top-level query-type, nested query.""" - model_config = ConfigDict(populate_by_name=True) + model_config = ConfigDict(populate_by_name=True, validate_by_name=True) name: str = Field(..., min_length=1) query_type: ExplorerViewType = Field(..., alias="query-type") @@ -116,7 +131,7 @@ class ExplorerSavedViewCreateOptions(BaseModel): class ExplorerSavedViewUpdateOptions(BaseModel): """PATCH .../explorer/views/{id} attributes: name and full replacement query.""" - model_config = ConfigDict(populate_by_name=True) + model_config = ConfigDict(populate_by_name=True, validate_by_name=True) name: str = Field(..., min_length=1) query: ExplorerSavedQuery diff --git a/src/pytfe/resources/explorer.py b/src/pytfe/resources/explorer.py index 50d6ae5..85ff50f 100644 --- a/src/pytfe/resources/explorer.py +++ b/src/pytfe/resources/explorer.py @@ -3,25 +3,19 @@ """Explorer API resource. -Maps organization-scoped Explorer endpoints (ad hoc query, CSV export, saved views) to -typed models. Saved-view create/update reshape filter JSON; read paths normalize API -variants before validation. +Maps organization-scoped Explorer endpoints (ad-hoc query, CSV export, saved +views) to typed models. Saved-view create/update reshape filter JSON to the +nested ``{field: {operator: [values]}}`` form the server expects. """ from __future__ import annotations -import csv -import io -import logging from collections.abc import Iterator from typing import Any from ..errors import ( InvalidExplorerSavedViewIDError, InvalidOrgError, - NotFound, - ServerError, - ValidationError, ) from ..models.explorer import ( ExplorerQueryOptions, @@ -29,80 +23,15 @@ ExplorerSavedView, ExplorerSavedViewCreateOptions, ExplorerSavedViewUpdateOptions, - ExplorerUrlFilter, - ExplorerViewType, ) from ..utils import valid_string_id from ._base import _Service -_log = logging.getLogger(__name__) - - -def _explorer_single_resource_data( - resp: Any, - *, - operation: str, - organization: str, - view_id: str | None = None, -) -> dict[str, Any]: - """Parse json:api envelope for a single Explorer saved view; raise ValidationError if unusable.""" - ctx = f"org={organization!r}" - if view_id is not None: - ctx += f" view_id={view_id!r}" - try: - payload = resp.json() - except ValueError as exc: - _log.warning("explorer.%s: invalid JSON response (%s)", operation, ctx) - raise ValidationError( - f"Explorer {operation}: response body is not valid JSON ({ctx})" - ) from exc - if not isinstance(payload, dict): - _log.warning( - "explorer.%s: top-level JSON is not an object (%s)", operation, ctx - ) - raise ValidationError( - f"Explorer {operation}: expected JSON object at top level ({ctx})" - ) - data = payload.get("data") - if not isinstance(data, dict): - _log.warning( - "explorer.%s: missing or invalid 'data' (type=%s) (%s)", - operation, - type(data).__name__, - ctx, - ) - raise ValidationError( - f"Explorer {operation}: expected json:api 'data' object ({ctx})" - ) - return data - - -def _require_organization(organization: str) -> None: - """Reject blank organization identifiers before building paths.""" - if not valid_string_id(organization): - raise InvalidOrgError() - - -def _require_organization_and_view(organization: str, view_id: str) -> None: - """Validate org and saved-view id for routes under .../explorer/views/{view_id}.""" - _require_organization(organization) - if not valid_string_id(view_id): - raise InvalidExplorerSavedViewIDError() - - -def _write_attributes_with_query_shape( - options: ExplorerSavedViewCreateOptions | ExplorerSavedViewUpdateOptions, -) -> dict[str, Any]: - """Serialize create/update options; map saved-query filters to the map shape POST/PATCH expect.""" - attrs = options.model_dump(by_alias=True, exclude_none=True, mode="json") - raw_query = attrs.get("query") - if isinstance(raw_query, dict): - attrs["query"] = _saved_query_to_api_shape(raw_query) - return attrs - def _query_params(options: ExplorerQueryOptions) -> dict[str, Any]: - # mode="json" keeps ExplorerViewType as strings; filters are expanded separately (Explorer URL grammar). + """Serialise ExplorerQueryOptions to query-string params, expanding filters.""" + # mode="json" keeps ExplorerViewType as strings; filters are expanded + # separately into the Explorer URL grammar. params = options.model_dump( by_alias=True, exclude_none=True, @@ -111,393 +40,156 @@ def _query_params(options: ExplorerQueryOptions) -> dict[str, Any]: ) if options.filters: for flt in options.filters: - params[ + key = ( f"filter[{flt.index}][{flt.field}][{flt.operator}][{flt.value_index}]" - ] = flt.value + ) + params[key] = flt.value return params -def _parse_row(item: dict[str, Any]) -> ExplorerRow: - return ExplorerRow.model_validate(item) +def _normalize_attribute_keys(attrs: dict[str, Any]) -> dict[str, Any]: + """Normalise JSON:API hyphen attribute keys to Python snake_case.""" + return {k.replace("-", "_"): v for k, v in attrs.items()} -def _normalize_filter_field_name(raw_field: Any) -> str: - """Normalize filter field names to SDK model style.""" - return str(raw_field).replace("-", "_") +def _parse_row(item: dict[str, Any]) -> ExplorerRow: + return ExplorerRow.model_validate( + { + "id": item.get("id", ""), + "type": item.get("type", ""), + "attributes": _normalize_attribute_keys(item.get("attributes") or {}), + } + ) def _saved_query_to_api_shape(raw_query: dict[str, Any]) -> dict[str, Any]: - """Map {field, operator, value} filter rows to nested {field: {operator: [...]}} JSON.""" + """Map the model's flat filter rows and field list into the shapes the + API expects on create/update: + + * ``filter`` rows: ``{field, operator, value}`` → ``{field: {operator: [values]}}`` + * ``fields``: ``[col, ...]`` → ``{view_type: [col, ...]}`` + """ query = dict(raw_query) + raw_filter = query.get("filter") if isinstance(raw_filter, list): - mapped_filters: list[dict[str, Any]] = [] + mapped: list[dict[str, Any]] = [] for entry in raw_filter: if not isinstance(entry, dict): continue - # Already API-compatible map style. + # Already in API map shape — pass through. if "field" not in entry or "operator" not in entry: - mapped_filters.append(entry) + mapped.append(entry) continue - field = _normalize_filter_field_name(entry.get("field", "")) + field = str(entry.get("field", "")).replace("-", "_") operator = str(entry.get("operator", "")) values = entry.get("value", []) if not isinstance(values, list): values = [values] - mapped_filters.append({field: {operator: [str(v) for v in values]}}) - query["filter"] = mapped_filters + mapped.append({field: {operator: [str(v) for v in values]}}) + query["filter"] = mapped + + # The API stores `fields` as `{view_type: [columns]}`. Wrap a flat list. + raw_fields = query.get("fields") + view_type = query.get("type") + if isinstance(raw_fields, list) and isinstance(view_type, str): + query["fields"] = {view_type: list(raw_fields)} + return query -def _normalize_saved_query( - raw_query: dict[str, Any], raw_query_type: str | None +def _write_attributes( + options: ExplorerSavedViewCreateOptions | ExplorerSavedViewUpdateOptions, ) -> dict[str, Any]: - """Coerce saved-view query JSON into the flat filter + list fields shape our models use.""" - query = dict(raw_query) + """Serialise create/update options with filters reshaped for the API.""" + attrs = options.model_dump(by_alias=True, exclude_none=True, mode="json") + raw_query = attrs.get("query") + if isinstance(raw_query, dict): + attrs["query"] = _saved_query_to_api_shape(raw_query) + return attrs + - if "type" not in query and raw_query_type: - query["type"] = raw_query_type +def _saved_query_from_api(raw_query: dict[str, Any]) -> dict[str, Any]: + """Coerce a saved query's API response shape into the flat model shape. + + The Explorer API stores filters as ``{field_name: {operator: [values]}}`` + and ``fields`` as ``{view_type: [...]}``. We flatten both for the model. + """ + query = dict(raw_query) raw_filter = query.get("filter") if isinstance(raw_filter, list): - normalized_filters: list[dict[str, Any]] = [] + flat: list[dict[str, Any]] = [] for entry in raw_filter: - # Variant A (documented): {"field": "...", "operator": "...", "value": [...]} - if isinstance(entry, dict) and "field" in entry and "operator" in entry: - value = entry.get("value") - if value is None: - value = [] - if not isinstance(value, list): - value = [str(value)] - normalized_filters.append( - { - "field": _normalize_filter_field_name(entry["field"]), - "operator": str(entry["operator"]), - "value": [str(v) for v in value], - } - ) + if not isinstance(entry, dict): continue + for field_name, operators in entry.items(): + if not isinstance(operators, dict): + continue + for operator, values in operators.items(): + vals = values if isinstance(values, list) else [values] + flat.append( + { + "field": str(field_name).replace("-", "_"), + "operator": str(operator), + "value": [str(v) for v in vals], + } + ) + query["filter"] = flat - # Variant B (observed): {"workspace-name": {"contains": ["foo"]}} - if isinstance(entry, dict): - for field_name, operators in entry.items(): - if not isinstance(operators, dict): - continue - for operator, values in operators.items(): - vals = values if isinstance(values, list) else [values] - normalized_filters.append( - { - "field": _normalize_filter_field_name(field_name), - "operator": str(operator), - "value": [str(v) for v in vals], - } - ) - query["filter"] = normalized_filters - + # `fields` arrives as {view_type: [...]}; flatten to a single list. raw_fields = query.get("fields") - # Some responses return fields as {"workspaces": [...]}. if isinstance(raw_fields, dict): - list_values: list[str] = [] + flat_fields: list[str] = [] for value in raw_fields.values(): if isinstance(value, list): - list_values.extend(str(v) for v in value) - query["fields"] = list_values + flat_fields.extend(str(v) for v in value) + query["fields"] = flat_fields return query def _parse_saved_view(item: dict[str, Any]) -> ExplorerSavedView: - # json:api envelope: attributes carry name, timestamps, nested query and query-type. - attrs = item.get("attributes", {}) - query_type = attrs.get("query-type") - query = attrs.get("query", {}) + attrs = item.get("attributes") or {} + query = attrs.get("query") or {} if not isinstance(query, dict): query = {} - return ExplorerSavedView.model_validate( { "id": item.get("id"), "name": attrs.get("name"), "created-at": attrs.get("created-at"), - "query": _normalize_saved_query(query, query_type), - "query-type": query_type, + "query": _saved_query_from_api(query), + "query-type": attrs.get("query-type"), } ) -def _query_options_from_saved_view( - saved_view: ExplorerSavedView, -) -> ExplorerQueryOptions: - """Replay a stored saved query as GET /explorer query params (used by CSV fallback).""" - query = saved_view.query - filters: list[ExplorerUrlFilter] = [] - if query.filter: - for idx, flt in enumerate(query.filter): - for value_index, value in enumerate(flt.value or []): - filters.append( - ExplorerUrlFilter( - index=idx, - field=flt.field, - operator=flt.operator, - value=str(value), - value_index=value_index, - ) - ) - return ExplorerQueryOptions.model_validate( - { - "type": saved_view.query_type, - "sort": ",".join(query.sort) if query.sort else None, - "fields": ",".join(query.fields) if query.fields else None, - "filters": filters or None, - } - ) - - -# Column order matches HashiCorp Explorer API docs (view-type field tables and export/csv -# workspaces sample): https://developer.hashicorp.com/terraform/cloud-docs/api-docs/explorer -_EXPLORER_CSV_COLUMNS: dict[ExplorerViewType, tuple[str, ...]] = { - ExplorerViewType.WORKSPACES: ( - "all_checks_succeeded", - "current_rum_count", - "checks_errored", - "checks_failed", - "checks_passed", - "checks_unknown", - "current_run_applied_at", - "current_run_external_id", - "current_run_status", - "drifted", - "external_id", - "module_count", - "modules", - "organization_name", - "project_external_id", - "project_name", - "provider_count", - "providers", - "resources_drifted", - "resources_undrifted", - "state_version_terraform_version", - "vcs_repo_identifier", - "workspace_created_at", - "workspace_name", - "workspace_terraform_version", - "workspace_updated_at", - ), - ExplorerViewType.TF_VERSIONS: ("version", "workspace_count", "workspaces"), - ExplorerViewType.PROVIDERS: ( - "name", - "source", - "version", - "workspace_count", - "workspaces", - ), - ExplorerViewType.MODULES: ( - "name", - "source", - "version", - "workspace_count", - "workspaces", - ), -} - -_ROW_TYPE_TO_VIEW: dict[str, ExplorerViewType] = { - "visibility-workspace": ExplorerViewType.WORKSPACES, -} - - -def _infer_view_type_from_csv_header(header: list[str]) -> ExplorerViewType | None: - """Pick Explorer view type from CSV header names (no extra API call).""" - h = frozenset(header) - candidates: list[tuple[int, int, str, ExplorerViewType]] = [] - for vt, cols in _EXPLORER_CSV_COLUMNS.items(): - colset = frozenset(cols) - overlap = len(h & colset) - if overlap == 0: - continue - # Prefer more matching columns; tie-break to a narrower schema (e.g. tf_versions). - candidates.append((overlap, -len(colset), vt.value, vt)) - if not candidates: - return None - _, _, _, vt = max(candidates) - return vt - - -def _explorer_attribute_value(attrs: dict[str, Any], logical_snake: str) -> Any: - """Resolve API attribute keys (snake_case or kebab-case) for one logical Explorer column.""" - hyphen = logical_snake.replace("_", "-") - if logical_snake in attrs: - return attrs[logical_snake] - if hyphen in attrs: - return attrs[hyphen] - return "" - - -def _csv_fieldnames_for_explorer_rows( - rows: list[ExplorerRow], - view_type: ExplorerViewType | None, -) -> tuple[list[str], frozenset[str]]: - """Doc-ordered columns first; trailing columns for attributes not in the doc schema.""" - all_raw: set[str] = set() - for row in rows: - all_raw.update(row.attributes.keys()) - - order = _EXPLORER_CSV_COLUMNS.get(view_type) if view_type is not None else None - if not order: - seen: set[str] = set() - visit: list[str] = [] - for row in rows: - for k in row.attributes: - if k not in seen: - seen.add(k) - visit.append(k) - return visit, frozenset() - - canonical_set = frozenset(order) - matched_raw: set[str] = set() - for raw in all_raw: - for col in order: - if raw == col or raw == col.replace("_", "-"): - matched_raw.add(raw) - break - - extras: list[str] = [] - seen_extras: set[str] = set() - for row in rows: - for raw in row.attributes: - if raw not in canonical_set and raw not in seen_extras: - seen_extras.add(raw) - extras.append(raw) - return list(order) + extras, canonical_set - - -def _infer_view_type_from_rows(rows: list[ExplorerRow]) -> ExplorerViewType | None: - if not rows: - return None - return _ROW_TYPE_TO_VIEW.get(rows[0].row_type) - - -def _normalize_explorer_csv_column_order( - csv_text: str, view_type: ExplorerViewType | None -) -> str: - """Reorder CSV header/data columns to match Explorer API doc order (GET CSV varies).""" - if not csv_text.strip() or view_type is None: - return csv_text - order = _EXPLORER_CSV_COLUMNS.get(view_type) - if not order: - return csv_text - try: - reader = csv.reader(io.StringIO(csv_text)) - rows = list(reader) - except csv.Error: - return csv_text - if not rows or not rows[0]: - return csv_text - header = rows[0] - idx = {name: i for i, name in enumerate(header)} - order_set = frozenset(order) - canonical = [c for c in order if c in idx] - extras = [h for h in header if h not in order_set] - new_header = canonical + extras - if new_header == header: - return csv_text - perm = [idx[h] for h in new_header] - ncols = len(header) - out_rows: list[list[str]] = [new_header] - for row in rows[1:]: - padded = list(row) + [""] * max(0, ncols - len(row)) - padded = padded[:ncols] - out_rows.append([padded[i] for i in perm]) - buf = io.StringIO() - writer = csv.writer(buf, lineterminator="\n") - writer.writerows(out_rows) - return buf.getvalue() - - -def _rows_to_csv( - rows: list[ExplorerRow], - *, - view_type: ExplorerViewType | None = None, -) -> str: - """Build CSV from result rows; column order follows Explorer API docs when view_type is known.""" - if not rows: - return "" - vt = view_type if view_type is not None else _infer_view_type_from_rows(rows) - fieldnames, canonical_set = _csv_fieldnames_for_explorer_rows(rows, vt) - buf = io.StringIO() - writer = csv.DictWriter(buf, fieldnames=fieldnames, extrasaction="ignore") - writer.writeheader() - for row in rows: - attrs = row.attributes - row_out: dict[str, Any] = {} - for name in fieldnames: - if name in canonical_set: - row_out[name] = _explorer_attribute_value(attrs, name) - else: - row_out[name] = attrs.get(name, "") - writer.writerow(row_out) - return buf.getvalue() - - class Explorer(_Service): - """Organization Explorer: ad hoc queries, CSV export, and saved view CRUD.""" + """Organization Explorer: ad-hoc queries, CSV export, and saved view CRUD.""" def query( self, organization: str, options: ExplorerQueryOptions ) -> Iterator[ExplorerRow]: - """Execute an Explorer query and iterate result rows across all pages. - - Args: - organization: Organization slug that owns the Explorer data. - options: Query options including view type, filters, sort, and paging. - - Yields: - ExplorerRow items returned by the Explorer endpoint. - """ - _require_organization(organization) - _log.debug( - "explorer.query org=%r view_type=%s", - organization, - options.view_type.value, - ) - # GET .../explorer — paginated JSON rows for the given view and filters. + """Execute an Explorer query and iterate result rows across all pages.""" + if not valid_string_id(organization): + raise InvalidOrgError() path = f"/api/v2/organizations/{organization}/explorer" for item in self._list(path, params=_query_params(options)): yield _parse_row(item) def export_csv(self, organization: str, options: ExplorerQueryOptions) -> str: - """Run an Explorer query and return CSV text from the export endpoint. - - Args: - organization: Organization slug that owns the Explorer data. - options: Query options including view type, filters, sort, and paging. - - Returns: - Raw CSV text returned by the server. - """ - _require_organization(organization) - _log.debug( - "explorer.export_csv org=%r view_type=%s", - organization, - options.view_type.value, - ) - # Same query string as query(); response is a single unpaged CSV document. + """Run an Explorer query and return CSV text from the export endpoint.""" + if not valid_string_id(organization): + raise InvalidOrgError() path = f"/api/v2/organizations/{organization}/explorer/export/csv" resp = self.t.request("GET", path, params=_query_params(options)) return resp.text def list_saved_views(self, organization: str) -> Iterator[ExplorerSavedView]: - """Iterate all saved Explorer views in an organization. - - Args: - organization: Organization slug that owns the saved views. - - Yields: - ExplorerSavedView resources from the list endpoint. - """ - _require_organization(organization) - _log.debug("explorer.list_saved_views org=%r", organization) - # GET collection of explorer-saved-queries for the org. + """Iterate all saved Explorer views in an organization.""" + if not valid_string_id(organization): + raise InvalidOrgError() path = f"/api/v2/organizations/{organization}/explorer/views" for item in self._list(path): yield _parse_saved_view(item) @@ -505,58 +197,29 @@ def list_saved_views(self, organization: str) -> Iterator[ExplorerSavedView]: def create_saved_view( self, organization: str, options: ExplorerSavedViewCreateOptions ) -> ExplorerSavedView: - """Create a saved Explorer view. - - Args: - organization: Organization slug that owns the saved view. - options: Saved-view name and query definition to persist. - - Returns: - The created ExplorerSavedView as returned by the API. - """ - _require_organization(organization) - # POST json:api explorer-saved-queries; filters rewritten for server expectations. - attrs = _write_attributes_with_query_shape(options) + """Create a saved Explorer view.""" + if not valid_string_id(organization): + raise InvalidOrgError() body = { "data": { "type": "explorer-saved-queries", - "attributes": attrs, + "attributes": _write_attributes(options), } } path = f"/api/v2/organizations/{organization}/explorer/views" resp = self.t.request("POST", path, json_body=body) - data = _explorer_single_resource_data( - resp, operation="create_saved_view", organization=organization - ) - view = _parse_saved_view(data) - _log.info("explorer.create_saved_view org=%r id=%r", organization, view.id) - return view + data = (resp.json() or {}).get("data") or {} + return _parse_saved_view(data) def read_saved_view(self, organization: str, view_id: str) -> ExplorerSavedView: - """Read one saved Explorer view by id. - - Args: - organization: Organization slug that owns the saved view. - view_id: Saved-view id (for example, ``sq-...``). - - Returns: - The saved view definition and query metadata. - """ - _require_organization_and_view(organization, view_id) - _log.debug( - "explorer.read_saved_view org=%r view_id=%r", - organization, - view_id, - ) - # Returns stored definition only; does not execute the query (see saved_view_results). + """Read one saved Explorer view by id.""" + if not valid_string_id(organization): + raise InvalidOrgError() + if not valid_string_id(view_id): + raise InvalidExplorerSavedViewIDError() path = f"/api/v2/organizations/{organization}/explorer/views/{view_id}" resp = self.t.request("GET", path) - data = _explorer_single_resource_data( - resp, - operation="read_saved_view", - organization=organization, - view_id=view_id, - ) + data = (resp.json() or {}).get("data") or {} return _parse_saved_view(data) def update_saved_view( @@ -565,140 +228,53 @@ def update_saved_view( view_id: str, options: ExplorerSavedViewUpdateOptions, ) -> ExplorerSavedView: - """Replace attributes of an existing saved Explorer view. - - Args: - organization: Organization slug that owns the saved view. - view_id: Saved-view id (for example, ``sq-...``). - options: Updated name and full replacement query definition. - - Returns: - The updated ExplorerSavedView as returned by the API. - """ - _require_organization_and_view(organization, view_id) - attrs = _write_attributes_with_query_shape(options) - # PATCH includes resource id in the envelope per json:api update conventions. + """Replace attributes of an existing saved Explorer view.""" + if not valid_string_id(organization): + raise InvalidOrgError() + if not valid_string_id(view_id): + raise InvalidExplorerSavedViewIDError() body = { "data": { "type": "explorer-saved-queries", "id": view_id, - "attributes": attrs, + "attributes": _write_attributes(options), } } path = f"/api/v2/organizations/{organization}/explorer/views/{view_id}" resp = self.t.request("PATCH", path, json_body=body) - data = _explorer_single_resource_data( - resp, - operation="update_saved_view", - organization=organization, - view_id=view_id, - ) - view = _parse_saved_view(data) - _log.info("explorer.update_saved_view org=%r id=%r", organization, view.id) - return view + data = (resp.json() or {}).get("data") or {} + return _parse_saved_view(data) def delete_saved_view(self, organization: str, view_id: str) -> None: - """Delete a saved Explorer view. - - Args: - organization: Organization slug that owns the saved view. - view_id: Saved-view id (for example, ``sq-...``). - - Returns: - None. - """ - _require_organization_and_view(organization, view_id) + """Delete a saved Explorer view.""" + if not valid_string_id(organization): + raise InvalidOrgError() + if not valid_string_id(view_id): + raise InvalidExplorerSavedViewIDError() path = f"/api/v2/organizations/{organization}/explorer/views/{view_id}" self.t.request("DELETE", path) def saved_view_results( self, organization: str, view_id: str ) -> Iterator[ExplorerRow]: - """Execute a saved view and iterate result rows across all pages. - - Args: - organization: Organization slug that owns the saved view. - view_id: Saved-view id (for example, ``sq-...``). - - Yields: - ExplorerRow items produced by the saved query. - """ - _require_organization_and_view(organization, view_id) - _log.debug( - "explorer.saved_view_results org=%r view_id=%r", - organization, - view_id, - ) - # Re-runs the saved query; rows match ad hoc query() shape (current data only). + """Execute a saved view and iterate result rows across all pages.""" + if not valid_string_id(organization): + raise InvalidOrgError() + if not valid_string_id(view_id): + raise InvalidExplorerSavedViewIDError() path = f"/api/v2/organizations/{organization}/explorer/views/{view_id}/results" for item in self._list(path): yield _parse_row(item) def saved_view_results_csv(self, organization: str, view_id: str) -> str: - """Return CSV for a saved view with resilient fallback behavior. - - Tries the dedicated saved-view CSV endpoint first, then falls back to replaying - the saved view through ``export_csv`` and finally to materializing rows from the - paginated results endpoint. - - Args: - organization: Organization slug that owns the saved view. - view_id: Saved-view id (for example, ``sq-...``). - - Returns: - CSV text for the saved view results. - """ - _require_organization_and_view(organization, view_id) - _log.debug( - "explorer.saved_view_results_csv org=%r view_id=%r", - organization, - view_id, + """Return CSV text for a saved view from the dedicated export endpoint.""" + if not valid_string_id(organization): + raise InvalidOrgError() + if not valid_string_id(view_id): + raise InvalidExplorerSavedViewIDError() + path = ( + f"/api/v2/organizations/{organization}" + f"/explorer/views/{view_id}/export/csv" ) - path = f"/api/v2/organizations/{organization}/explorer/views/{view_id}/csv" - try: - resp = self.t.request("GET", path) - csv_text = resp.text - try: - parsed = list(csv.reader(io.StringIO(csv_text))) - except csv.Error: - return csv_text - if parsed and parsed[0]: - vt = _infer_view_type_from_csv_header(parsed[0]) - if vt is not None: - csv_text = _normalize_explorer_csv_column_order(csv_text, vt) - return csv_text - except (NotFound, ServerError) as exc: - _log.info( - "explorer.saved_view_results_csv: primary CSV route unavailable (%s); " - "trying export_csv replay org=%r view_id=%r", - exc.__class__.__name__, - organization, - view_id, - ) - - # Fall back: replay saved definition via export_csv, then row materialization if needed. - saved_for_csv: ExplorerSavedView | None = None - try: - saved_for_csv = self.read_saved_view(organization, view_id) - options = _query_options_from_saved_view(saved_for_csv) - csv_text = self.export_csv(organization, options) - csv_text = _normalize_explorer_csv_column_order( - csv_text, saved_for_csv.query_type - ) - _log.info( - "explorer.saved_view_results_csv: used export_csv fallback org=%r view_id=%r", - organization, - view_id, - ) - return csv_text - except (NotFound, ServerError) as exc: - _log.warning( - "explorer.saved_view_results_csv: export_csv fallback failed (%s); " - "building CSV from row stream org=%r view_id=%r", - exc.__class__.__name__, - organization, - view_id, - ) - rows = list(self.saved_view_results(organization, view_id)) - vt = saved_for_csv.query_type if saved_for_csv is not None else None - return _rows_to_csv(rows, view_type=vt) + resp = self.t.request("GET", path) + return resp.text diff --git a/tests/units/test_explorer.py b/tests/units/test_explorer.py index a0b0dd2..d11cbaa 100644 --- a/tests/units/test_explorer.py +++ b/tests/units/test_explorer.py @@ -1,23 +1,18 @@ # Copyright IBM Corp. 2025, 2026 # SPDX-License-Identifier: MPL-2.0 -"""Unit tests for Explorer API resource.""" +"""Unit tests for the Explorer API resource.""" -import csv -from unittest.mock import Mock, call +from unittest.mock import Mock import pytest from pytfe.errors import ( InvalidExplorerSavedViewIDError, InvalidOrgError, - NotFound, - ServerError, - ValidationError, ) from pytfe.models import ( ExplorerQueryOptions, - ExplorerRow, ExplorerSavedQuery, ExplorerSavedQueryFilter, ExplorerSavedViewCreateOptions, @@ -25,11 +20,7 @@ ExplorerUrlFilter, ExplorerViewType, ) -from pytfe.resources.explorer import ( - Explorer, - _normalize_explorer_csv_column_order, - _rows_to_csv, -) +from pytfe.resources.explorer import Explorer ORG = "acme" VIEW_ID = "sq-1" @@ -47,37 +38,8 @@ def explorer_service(mock_transport): return Explorer(mock_transport) -def test_normalize_explorer_csv_column_order_workspaces(): - raw = "workspace_name,all_checks_succeeded\ndemo,true\n" - out = _normalize_explorer_csv_column_order(raw, ExplorerViewType.WORKSPACES) - assert out.splitlines()[0].startswith("all_checks_succeeded,workspace_name") - - -def test_rows_to_csv_workspace_column_order_matches_doc(): - """Fallback CSV header matches Explorer export/csv workspaces sample column order.""" - rows = [ - ExplorerRow.model_validate( - { - "id": "ws-1", - "type": "visibility-workspace", - "attributes": {"workspace-name": "demo-workspace"}, - } - ) - ] - csv_text = _rows_to_csv(rows, view_type=ExplorerViewType.WORKSPACES) - header = csv_text.strip().splitlines()[0] - assert header.startswith( - "all_checks_succeeded,current_rum_count,checks_errored,checks_failed," - "checks_passed,checks_unknown,current_run_applied_at,current_run_external_id," - "current_run_status,drifted,external_id,module_count,modules,organization_name," - "project_external_id,project_name,provider_count,providers,resources_drifted," - "resources_undrifted,state_version_terraform_version,vcs_repo_identifier," - "workspace_created_at,workspace_name,workspace_terraform_version,workspace_updated_at" - ) - assert "demo-workspace" in csv_text - - def _row_payload(row_id: str) -> dict: + """Server-shaped row: id + type + hyphen-keyed attributes.""" return { "id": row_id, "type": "visibility-workspace", @@ -86,6 +48,7 @@ def _row_payload(row_id: str) -> dict: def _saved_view_payload(view_id: str) -> dict: + """Server-shaped saved view (matches live API: nested filter map shape).""" return { "id": view_id, "type": "explorer-saved-queries", @@ -95,567 +58,235 @@ def _saved_view_payload(view_id: str) -> dict: "query-type": "workspaces", "query": { "type": "workspaces", - "filter": [ - { - "field": "workspace_name", - "operator": "contains", - "value": ["child"], - } - ], - }, - }, - } - - -def _saved_view_payload_live_variant(view_id: str) -> dict: - return { - "id": view_id, - "type": "explorer-saved-queries", - "attributes": { - "name": "my-view", - "created-at": "2024-10-11T16:18:51.442Z", - "query-type": "workspaces", - "query": { - "filter": [{"workspace-name": {"contains": ["r2l7cj4v"]}}], + "filter": [{"workspace_name": {"contains": ["child"]}}], "fields": {"workspaces": []}, + "sort": [], }, }, } -def _assert_single_request_call( - mock_transport, method: str, path: str, **kwargs -) -> None: - mock_transport.request.assert_called_once_with(method, path, **kwargs) - - -def _query_request_params(page_number: int) -> dict: - return { - "type": "workspaces", - "sort": "-workspace_name", - "fields": "workspace_name,organization_name", - "page[size]": 1, - "filter[0][workspace_name][contains][0]": "test", - "page[number]": page_number, +def _single_page_response(items): + """Mimic the transport response for a one-page _list iteration.""" + resp = Mock() + resp.json.return_value = { + "data": items, + "meta": { + "pagination": { + "current-page": 1, + "total-pages": 1, + "next-page": None, + } + }, } + return resp class TestExplorerQuery: - def test_query_with_filter_and_pagination(self, explorer_service, mock_transport): - first = Mock() - first.json.return_value = {"data": [_row_payload("ws-1")]} - second = Mock() - second.json.return_value = {"data": [_row_payload("ws-2")]} - third = Mock() - third.json.return_value = {"data": [_row_payload("ws-3")]} - fourth = Mock() - fourth.json.return_value = {"data": []} - mock_transport.request.side_effect = [first, second, third, fourth] - - options = ExplorerQueryOptions( - view_type=ExplorerViewType.WORKSPACES, - sort="-workspace_name", - fields="workspace_name,organization_name", - page_size=1, - filters=[ - ExplorerUrlFilter( - index=0, - field="workspace_name", - operator="contains", - value="test", - ) - ], - ) - - rows = list(explorer_service.query(ORG, options)) - assert len(rows) == 3 - assert [row.id for row in rows] == ["ws-1", "ws-2", "ws-3"] - assert all(row.row_type == "visibility-workspace" for row in rows) - - expected_calls = [ - call("GET", EXPLORER_PATH, params=_query_request_params(page_number=1)), - call("GET", EXPLORER_PATH, params=_query_request_params(page_number=2)), - call("GET", EXPLORER_PATH, params=_query_request_params(page_number=3)), - call("GET", EXPLORER_PATH, params=_query_request_params(page_number=4)), - ] - mock_transport.request.assert_has_calls(expected_calls) - assert mock_transport.request.call_count == 4 - - def test_query_uses_pagination_meta_when_server_caps_page_size( + def test_query_emits_expanded_filter_params( self, explorer_service, mock_transport ): - first = Mock() - first.json.return_value = { - "data": [_row_payload("ws-1"), _row_payload("ws-2")], - "meta": { - "pagination": { - "current-page": 1, - "page-size": 2, - "next-page": 2, - "total-pages": 2, - } - }, - } - second = Mock() - second.json.return_value = { - "data": [_row_payload("ws-3")], - "meta": { - "pagination": { - "current-page": 2, - "page-size": 2, - "next-page": None, - "total-pages": 2, - } - }, - } - mock_transport.request.side_effect = [first, second] + mock_transport.request.return_value = _single_page_response( + [_row_payload("ws-1"), _row_payload("ws-2")] + ) - options = ExplorerQueryOptions( + opts = ExplorerQueryOptions( view_type=ExplorerViewType.WORKSPACES, page_size=50, + filters=[ + ExplorerUrlFilter( + index=0, field="workspace_name", operator="contains", value="demo" + ), + ], ) + rows = list(explorer_service.query(ORG, opts)) - rows = list(explorer_service.query(ORG, options)) - assert [row.id for row in rows] == ["ws-1", "ws-2", "ws-3"] - - expected_calls = [ - call( - "GET", - EXPLORER_PATH, - params={"type": "workspaces", "page[size]": 50, "page[number]": 1}, - ), - call( - "GET", - EXPLORER_PATH, - params={"type": "workspaces", "page[size]": 50, "page[number]": 2}, - ), - ] - mock_transport.request.assert_has_calls(expected_calls) - assert mock_transport.request.call_count == 2 - - def test_query_uses_current_and_total_pages_when_next_page_missing( - self, explorer_service, mock_transport - ): - first = Mock() - first.json.return_value = { - "data": [_row_payload("ws-1")], - "meta": {"pagination": {"current-page": 1, "total-pages": 2}}, - } - second = Mock() - second.json.return_value = { - "data": [_row_payload("ws-2")], - "meta": {"pagination": {"current-page": 2, "total-pages": 2}}, - } - mock_transport.request.side_effect = [first, second] - - rows = list( - explorer_service.query( - ORG, ExplorerQueryOptions(view_type=ExplorerViewType.WORKSPACES) - ) - ) - assert [row.id for row in rows] == ["ws-1", "ws-2"] - - expected_calls = [ - call( - "GET", - EXPLORER_PATH, - params={"type": "workspaces", "page[number]": 1, "page[size]": 100}, - ), - call( - "GET", - EXPLORER_PATH, - params={"type": "workspaces", "page[number]": 2, "page[size]": 100}, - ), - ] - mock_transport.request.assert_has_calls(expected_calls) - assert mock_transport.request.call_count == 2 - - def test_query_stops_when_pagination_meta_does_not_advance( - self, explorer_service, mock_transport - ): - first = Mock() - first.json.return_value = { - "data": [_row_payload("ws-1")], - "meta": {"pagination": {"current-page": 1, "total-pages": 2}}, - } - second = Mock() - second.json.return_value = { - "data": [_row_payload("ws-1")], - "meta": {"pagination": {"current-page": 1, "total-pages": 2}}, - } - mock_transport.request.side_effect = [first, second] - - rows = list( - explorer_service.query( - ORG, ExplorerQueryOptions(view_type=ExplorerViewType.WORKSPACES) - ) - ) - assert [row.id for row in rows] == ["ws-1", "ws-1"] - assert mock_transport.request.call_count == 2 - - def test_query_stops_on_empty_page_even_if_next_page_present( - self, explorer_service, mock_transport - ): - first = Mock() - first.json.return_value = { - "data": [], - "meta": { - "pagination": {"current-page": 1, "next-page": 2, "total-pages": 5} - }, - } - mock_transport.request.return_value = first + assert [r.id for r in rows] == ["ws-1", "ws-2"] + assert rows[0].type == "visibility-workspace" + # Hyphen attribute keys must be normalised to snake_case at parse time. + assert rows[0].attributes == {"workspace_name": "demo-workspace"} - rows = list( - explorer_service.query( - ORG, ExplorerQueryOptions(view_type=ExplorerViewType.WORKSPACES) - ) + # The transport was called against the right path with the expanded + # filter key in the params. + call = mock_transport.request.call_args + assert call.args == ("GET", EXPLORER_PATH) + params = call.kwargs["params"] + assert params["type"] == "workspaces" + assert params["page[size]"] == 50 + assert ( + params["filter[0][workspace_name][contains][0]"] == "demo" ) - assert rows == [] - assert mock_transport.request.call_count == 1 def test_query_invalid_org(self, explorer_service): with pytest.raises(InvalidOrgError): list( explorer_service.query( - "", - ExplorerQueryOptions(view_type=ExplorerViewType.WORKSPACES), + "", ExplorerQueryOptions(view_type=ExplorerViewType.WORKSPACES) ) ) - @pytest.mark.parametrize("org", ["", None]) + def test_export_csv_returns_text(self, explorer_service, mock_transport): + resp = Mock() + resp.text = "workspace_name\ndemo\n" + mock_transport.request.return_value = resp + + opts = ExplorerQueryOptions(view_type=ExplorerViewType.WORKSPACES) + out = explorer_service.export_csv(ORG, opts) + assert out == "workspace_name\ndemo\n" + + call = mock_transport.request.call_args + assert call.args == ("GET", f"{EXPLORER_PATH}/export/csv") + assert call.kwargs["params"]["type"] == "workspaces" + + @pytest.mark.parametrize("org", ["", "bad/org"]) def test_export_csv_invalid_org(self, explorer_service, org): with pytest.raises(InvalidOrgError): explorer_service.export_csv( - org, - ExplorerQueryOptions(view_type=ExplorerViewType.WORKSPACES), + org, ExplorerQueryOptions(view_type=ExplorerViewType.WORKSPACES) ) - def test_export_csv(self, explorer_service, mock_transport): - response = Mock() - response.text = "workspace_name\nexample\n" - mock_transport.request.return_value = response - - csv_text = explorer_service.export_csv( - ORG, ExplorerQueryOptions(view_type=ExplorerViewType.WORKSPACES) - ) - - assert "workspace_name" in csv_text - _assert_single_request_call( - mock_transport, - "GET", - f"{EXPLORER_PATH}/export/csv", - params={"type": "workspaces"}, - ) - class TestExplorerSavedViews: def test_list_saved_views(self, explorer_service, mock_transport): - response = Mock() - response.json.return_value = {"data": [_saved_view_payload("sq-1")]} - mock_transport.request.return_value = response + mock_transport.request.return_value = _single_page_response( + [_saved_view_payload(VIEW_ID), _saved_view_payload("sq-2")] + ) views = list(explorer_service.list_saved_views(ORG)) - assert len(views) == 1 - assert views[0].id == "sq-1" + assert [v.id for v in views] == [VIEW_ID, "sq-2"] + assert views[0].name == "my-view" assert views[0].query_type == ExplorerViewType.WORKSPACES assert views[0].query.query_type == ExplorerViewType.WORKSPACES + # Filter must be flattened from server's map shape to the flat model. + assert len(views[0].query.filter) == 1 + flt = views[0].query.filter[0] + assert flt.field == "workspace_name" + assert flt.operator == "contains" + assert flt.value == ["child"] + + def test_create_saved_view_reshapes_filter_for_api( + self, explorer_service, mock_transport + ): + resp = Mock() + resp.json.return_value = {"data": _saved_view_payload(VIEW_ID)} + mock_transport.request.return_value = resp - def test_create_saved_view(self, explorer_service, mock_transport): - response = Mock() - response.json.return_value = {"data": _saved_view_payload("sq-new")} - mock_transport.request.return_value = response - - options = ExplorerSavedViewCreateOptions( + opts = ExplorerSavedViewCreateOptions( name="my-view", query_type=ExplorerViewType.WORKSPACES, query=ExplorerSavedQuery( query_type=ExplorerViewType.WORKSPACES, filter=[ ExplorerSavedQueryFilter( - field="workspace_name", operator="contains", value=["test"] + field="workspace_name", + operator="contains", + value=["child"], ) ], ), ) - view = explorer_service.create_saved_view(ORG, options) + view = explorer_service.create_saved_view(ORG, opts) + assert view.id == VIEW_ID - assert view.id == "sq-new" call = mock_transport.request.call_args - assert call[0][0] == "POST" - assert call[0][1] == VIEWS_PATH - body = call[1]["json_body"] + assert call.args == ("POST", VIEWS_PATH) + body = call.kwargs["json_body"] assert body["data"]["type"] == "explorer-saved-queries" - assert body["data"]["attributes"]["query-type"] == "workspaces" - assert body["data"]["attributes"]["query"]["filter"] == [ - {"workspace_name": {"contains": ["test"]}} + attrs = body["data"]["attributes"] + assert attrs["name"] == "my-view" + assert attrs["query-type"] == "workspaces" + # Filter rows in the request body must be in the nested map shape. + assert attrs["query"]["filter"] == [ + {"workspace_name": {"contains": ["child"]}} ] - def test_create_saved_view_invalid_json_raises( - self, explorer_service, mock_transport - ): - response = Mock() - response.json.side_effect = ValueError("invalid json") - mock_transport.request.return_value = response - - options = ExplorerSavedViewCreateOptions( - name="my-view", - query_type=ExplorerViewType.WORKSPACES, - query=ExplorerSavedQuery(query_type=ExplorerViewType.WORKSPACES), - ) - with pytest.raises(ValidationError, match="create_saved_view"): - explorer_service.create_saved_view(ORG, options) - - def test_read_saved_view_missing_data_object_raises( - self, explorer_service, mock_transport - ): - response = Mock() - response.json.return_value = {"data": []} - mock_transport.request.return_value = response - - with pytest.raises(ValidationError, match="read_saved_view"): - explorer_service.read_saved_view(ORG, VIEW_ID) - def test_read_saved_view(self, explorer_service, mock_transport): - response = Mock() - response.json.return_value = {"data": _saved_view_payload("sq-1")} - mock_transport.request.return_value = response + resp = Mock() + resp.json.return_value = {"data": _saved_view_payload(VIEW_ID)} + mock_transport.request.return_value = resp view = explorer_service.read_saved_view(ORG, VIEW_ID) - assert view.id == "sq-1" - - _assert_single_request_call(mock_transport, "GET", f"{VIEWS_PATH}/{VIEW_ID}") - - def test_read_saved_view_with_live_query_shape( - self, explorer_service, mock_transport - ): - response = Mock() - response.json.return_value = {"data": _saved_view_payload_live_variant("sq-2")} - mock_transport.request.return_value = response - - view = explorer_service.read_saved_view(ORG, "sq-2") - - assert view.id == "sq-2" + assert view.id == VIEW_ID assert view.query.query_type == ExplorerViewType.WORKSPACES - assert view.query.filter is not None - assert view.query.filter[0].field == "workspace_name" - assert view.query.filter[0].operator == "contains" - assert view.query.filter[0].value == ["r2l7cj4v"] - assert view.query.fields == [] + + call = mock_transport.request.call_args + assert call.args == ("GET", f"{VIEWS_PATH}/{VIEW_ID}") def test_update_saved_view(self, explorer_service, mock_transport): - response = Mock() - response.json.return_value = {"data": _saved_view_payload("sq-1")} - mock_transport.request.return_value = response + resp = Mock() + resp.json.return_value = {"data": _saved_view_payload(VIEW_ID)} + mock_transport.request.return_value = resp - options = ExplorerSavedViewUpdateOptions( - name="my-view-updated", + opts = ExplorerSavedViewUpdateOptions( + name="renamed", query=ExplorerSavedQuery( query_type=ExplorerViewType.WORKSPACES, filter=[ ExplorerSavedQueryFilter( - field="workspace_name", operator="contains", value=["prod"] + field="workspace_name", + operator="contains", + value=["abc"], ) ], ), ) - view = explorer_service.update_saved_view(ORG, VIEW_ID, options) - - assert view.id == "sq-1" - expected_body = { - "data": { - "type": "explorer-saved-queries", - "id": VIEW_ID, - "attributes": { - "name": "my-view-updated", - "query": { - "type": "workspaces", - "filter": [{"workspace_name": {"contains": ["prod"]}}], - }, - }, - } - } - _assert_single_request_call( - mock_transport, - "PATCH", - f"{VIEWS_PATH}/{VIEW_ID}", - json_body=expected_body, - ) - - def test_update_saved_view_invalid_json_raises( - self, explorer_service, mock_transport - ): - response = Mock() - response.json.side_effect = ValueError("invalid json") - mock_transport.request.return_value = response + view = explorer_service.update_saved_view(ORG, VIEW_ID, opts) + assert view.id == VIEW_ID - options = ExplorerSavedViewUpdateOptions( - name="my-view-updated", - query=ExplorerSavedQuery(query_type=ExplorerViewType.WORKSPACES), - ) - with pytest.raises(ValidationError, match="update_saved_view"): - explorer_service.update_saved_view(ORG, VIEW_ID, options) - - @pytest.mark.parametrize("payload", [[], "bad-payload", {"data": []}]) - def test_update_saved_view_invalid_data_shape_raises( - self, explorer_service, mock_transport, payload - ): - response = Mock() - response.json.return_value = payload - mock_transport.request.return_value = response - - options = ExplorerSavedViewUpdateOptions( - name="my-view-updated", - query=ExplorerSavedQuery(query_type=ExplorerViewType.WORKSPACES), - ) - with pytest.raises(ValidationError, match="update_saved_view"): - explorer_service.update_saved_view(ORG, VIEW_ID, options) + call = mock_transport.request.call_args + assert call.args == ("PATCH", f"{VIEWS_PATH}/{VIEW_ID}") + body = call.kwargs["json_body"] + assert body["data"]["id"] == VIEW_ID + assert body["data"]["attributes"]["name"] == "renamed" + # Same nested-map reshape on update. + assert body["data"]["attributes"]["query"]["filter"] == [ + {"workspace_name": {"contains": ["abc"]}} + ] def test_delete_saved_view(self, explorer_service, mock_transport): - result = explorer_service.delete_saved_view(ORG, VIEW_ID) - assert result is None - - _assert_single_request_call(mock_transport, "DELETE", f"{VIEWS_PATH}/{VIEW_ID}") - - def test_delete_saved_view_ignores_response_body( - self, explorer_service, mock_transport - ): - response = Mock() - response.text = '{"data":{"id":"unexpected"}}' - response.json.side_effect = ValueError("No JSON body") - mock_transport.request.return_value = response - - result = explorer_service.delete_saved_view(ORG, VIEW_ID) - assert result is None + mock_transport.request.return_value = Mock() + explorer_service.delete_saved_view(ORG, VIEW_ID) + call = mock_transport.request.call_args + assert call.args == ("DELETE", f"{VIEWS_PATH}/{VIEW_ID}") def test_saved_view_results(self, explorer_service, mock_transport): - first = Mock() - first.json.return_value = {"data": [_row_payload("ws-1")]} - second = Mock() - second.json.return_value = {"data": []} - mock_transport.request.side_effect = [first, second] + mock_transport.request.return_value = _single_page_response( + [_row_payload("ws-1")] + ) rows = list(explorer_service.saved_view_results(ORG, VIEW_ID)) - assert len(rows) == 1 - assert rows[0].id == "ws-1" + assert [r.id for r in rows] == ["ws-1"] - mock_transport.request.assert_any_call( - "GET", - f"{VIEWS_PATH}/{VIEW_ID}/results", - params={"page[number]": 1, "page[size]": 100}, - ) + call = mock_transport.request.call_args + assert call.args == ("GET", f"{VIEWS_PATH}/{VIEW_ID}/results") def test_saved_view_results_csv(self, explorer_service, mock_transport): - csv_resp = Mock() - csv_resp.text = "workspace_name,all_checks_succeeded\ndemo,true\n" - mock_transport.request.return_value = csv_resp - - csv_text = explorer_service.saved_view_results_csv(ORG, VIEW_ID) - assert csv_text.splitlines()[0].startswith( - "all_checks_succeeded,workspace_name" - ) - _assert_single_request_call( - mock_transport, "GET", f"{VIEWS_PATH}/{VIEW_ID}/csv" - ) - - def test_saved_view_results_csv_invalid_csv_returns_raw( - self, explorer_service, mock_transport, monkeypatch - ): - csv_resp = Mock() - csv_resp.text = "raw-csv" - mock_transport.request.return_value = csv_resp - - def _raise_csv_error(*_args, **_kwargs): - raise csv.Error("invalid csv") - - monkeypatch.setattr("pytfe.resources.explorer.csv.reader", _raise_csv_error) - - csv_text = explorer_service.saved_view_results_csv(ORG, VIEW_ID) - assert csv_text == "raw-csv" - - def test_saved_view_results_csv_fallback_to_export( - self, explorer_service, mock_transport - ): - first = NotFound("not found", status=404) - read_resp = Mock() - read_resp.json.return_value = {"data": _saved_view_payload("sq-1")} - export_resp = Mock() - export_resp.text = "workspace_name\nfrom-export\n" - mock_transport.request.side_effect = [first, read_resp, export_resp] + resp = Mock() + resp.text = "id,name\nws-1,demo\n" + mock_transport.request.return_value = resp - csv_text = explorer_service.saved_view_results_csv(ORG, VIEW_ID) - assert "from-export" in csv_text + out = explorer_service.saved_view_results_csv(ORG, VIEW_ID) + assert out == "id,name\nws-1,demo\n" - def test_saved_view_results_csv_server_error_fallback_to_export( - self, explorer_service, mock_transport - ): - first = ServerError("server error", status=500) - read_resp = Mock() - read_resp.json.return_value = {"data": _saved_view_payload("sq-1")} - export_resp = Mock() - export_resp.text = "workspace_name\nfrom-export\n" - mock_transport.request.side_effect = [first, read_resp, export_resp] - - csv_text = explorer_service.saved_view_results_csv(ORG, VIEW_ID) - assert "from-export" in csv_text - - def test_saved_view_results_csv_fallback_to_rows( - self, explorer_service, mock_transport - ): - not_found = NotFound("not found", status=404) - read_resp = Mock() - read_resp.json.return_value = {"data": _saved_view_payload("sq-1")} - first_results = Mock() - first_results.json.return_value = {"data": [_row_payload("ws-1")]} - second_results = Mock() - second_results.json.return_value = {"data": []} - mock_transport.request.side_effect = [ - not_found, # /csv - read_resp, # read saved view - not_found, # export_csv fallback fails - first_results, # saved_view_results page 1 - second_results, # saved_view_results page 2 - ] - - csv_text = explorer_service.saved_view_results_csv(ORG, VIEW_ID) - header = csv_text.strip().splitlines()[0] - assert header.startswith( - "all_checks_succeeded,current_rum_count,checks_errored,checks_failed," - "checks_passed,checks_unknown,current_run_applied_at,current_run_external_id," - "current_run_status,drifted,external_id,module_count,modules,organization_name," - "project_external_id,project_name,provider_count,providers,resources_drifted," - "resources_undrifted,state_version_terraform_version,vcs_repo_identifier," - "workspace_created_at,workspace_name,workspace_terraform_version,workspace_updated_at" - ) - assert "demo-workspace" in csv_text + call = mock_transport.request.call_args + assert call.args == ("GET", f"{VIEWS_PATH}/{VIEW_ID}/export/csv") - @pytest.mark.parametrize("org", ["", None]) + @pytest.mark.parametrize("org", ["", "bad/org"]) def test_saved_view_methods_invalid_org(self, explorer_service, org): with pytest.raises(InvalidOrgError): list(explorer_service.list_saved_views(org)) - with pytest.raises(InvalidOrgError): explorer_service.read_saved_view(org, VIEW_ID) + with pytest.raises(InvalidOrgError): + explorer_service.delete_saved_view(org, VIEW_ID) - @pytest.mark.parametrize("view_id", ["", None]) + @pytest.mark.parametrize("view_id", ["", "bad/view"]) def test_saved_view_methods_invalid_id(self, explorer_service, view_id): with pytest.raises(InvalidExplorerSavedViewIDError): explorer_service.read_saved_view(ORG, view_id) - - with pytest.raises(InvalidExplorerSavedViewIDError): - explorer_service.update_saved_view( - ORG, - view_id, - ExplorerSavedViewUpdateOptions( - name="updated", - query=ExplorerSavedQuery(query_type=ExplorerViewType.WORKSPACES), - ), - ) - with pytest.raises(InvalidExplorerSavedViewIDError): explorer_service.delete_saved_view(ORG, view_id) - with pytest.raises(InvalidExplorerSavedViewIDError): list(explorer_service.saved_view_results(ORG, view_id)) - with pytest.raises(InvalidExplorerSavedViewIDError): explorer_service.saved_view_results_csv(ORG, view_id) From f7b5a7203faf7862636f5ee3738883fa7ec3bffe Mon Sep 17 00:00:00 2001 From: Prabuddha Chakraborty Date: Sun, 24 May 2026 23:09:40 +0530 Subject: [PATCH 2/2] update lint --- src/pytfe/resources/explorer.py | 36 +++++++++++++++++------ tests/units/test_explorer.py | 51 +++++++++++++++++++++++++++------ 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/pytfe/resources/explorer.py b/src/pytfe/resources/explorer.py index 85ff50f..9b17175 100644 --- a/src/pytfe/resources/explorer.py +++ b/src/pytfe/resources/explorer.py @@ -40,9 +40,7 @@ def _query_params(options: ExplorerQueryOptions) -> dict[str, Any]: ) if options.filters: for flt in options.filters: - key = ( - f"filter[{flt.index}][{flt.field}][{flt.operator}][{flt.value_index}]" - ) + key = f"filter[{flt.index}][{flt.field}][{flt.operator}][{flt.value_index}]" params[key] = flt.value return params @@ -110,10 +108,17 @@ def _write_attributes( def _saved_query_from_api(raw_query: dict[str, Any]) -> dict[str, Any]: - """Coerce a saved query's API response shape into the flat model shape. + """Coerce a saved query's API response into the flat model shape. - The Explorer API stores filters as ``{field_name: {operator: [values]}}`` - and ``fields`` as ``{view_type: [...]}``. We flatten both for the model. + The Explorer API can return filter rows in either of two shapes: + + * Documented flat shape: ``{"field": ..., "operator": ..., "value": [...]}`` + * Operator-map shape: ``{field_name: {operator: [values]}}`` + + We accept both. Dropping either shape silently loses filter data and + risks consumers overwriting saved-view criteria on an update round-trip. + + ``fields`` arrives as ``{view_type: [...]}`` and is flattened to a list. """ query = dict(raw_query) @@ -123,6 +128,22 @@ def _saved_query_from_api(raw_query: dict[str, Any]) -> dict[str, Any]: for entry in raw_filter: if not isinstance(entry, dict): continue + # Variant A: flat shape with explicit field/operator/value keys. + if "field" in entry and "operator" in entry: + value = entry.get("value", []) + if value is None: + value = [] + if not isinstance(value, list): + value = [value] + flat.append( + { + "field": str(entry["field"]).replace("-", "_"), + "operator": str(entry["operator"]), + "value": [str(v) for v in value], + } + ) + continue + # Variant B: operator-map shape — what the live API actually returns. for field_name, operators in entry.items(): if not isinstance(operators, dict): continue @@ -273,8 +294,7 @@ def saved_view_results_csv(self, organization: str, view_id: str) -> str: if not valid_string_id(view_id): raise InvalidExplorerSavedViewIDError() path = ( - f"/api/v2/organizations/{organization}" - f"/explorer/views/{view_id}/export/csv" + f"/api/v2/organizations/{organization}/explorer/views/{view_id}/export/csv" ) resp = self.t.request("GET", path) return resp.text diff --git a/tests/units/test_explorer.py b/tests/units/test_explorer.py index d11cbaa..2bb8676 100644 --- a/tests/units/test_explorer.py +++ b/tests/units/test_explorer.py @@ -83,9 +83,7 @@ def _single_page_response(items): class TestExplorerQuery: - def test_query_emits_expanded_filter_params( - self, explorer_service, mock_transport - ): + def test_query_emits_expanded_filter_params(self, explorer_service, mock_transport): mock_transport.request.return_value = _single_page_response( [_row_payload("ws-1"), _row_payload("ws-2")] ) @@ -113,9 +111,7 @@ def test_query_emits_expanded_filter_params( params = call.kwargs["params"] assert params["type"] == "workspaces" assert params["page[size]"] == 50 - assert ( - params["filter[0][workspace_name][contains][0]"] == "demo" - ) + assert params["filter[0][workspace_name][contains][0]"] == "demo" def test_query_invalid_org(self, explorer_service): with pytest.raises(InvalidOrgError): @@ -164,6 +160,45 @@ def test_list_saved_views(self, explorer_service, mock_transport): assert flt.operator == "contains" assert flt.value == ["child"] + def test_read_saved_view_accepts_documented_flat_filter_shape( + self, explorer_service, mock_transport + ): + """Filters returned in the documented `{field, operator, value}` shape + must round-trip too — dropping them would silently lose criteria on + update. See the HCP API docs for the response shape: + https://developer.hashicorp.com/terraform/cloud-docs/api-docs/explorer + """ + flat_view = { + "id": VIEW_ID, + "type": "explorer-saved-queries", + "attributes": { + "name": "my-view", + "query-type": "workspaces", + "query": { + "type": "workspaces", + "filter": [ + { + "field": "workspace_name", + "operator": "contains", + "value": ["test"], + } + ], + "fields": {"workspaces": []}, + "sort": [], + }, + }, + } + resp = Mock() + resp.json.return_value = {"data": flat_view} + mock_transport.request.return_value = resp + + view = explorer_service.read_saved_view(ORG, VIEW_ID) + assert len(view.query.filter) == 1 + flt = view.query.filter[0] + assert flt.field == "workspace_name" + assert flt.operator == "contains" + assert flt.value == ["test"] + def test_create_saved_view_reshapes_filter_for_api( self, explorer_service, mock_transport ): @@ -196,9 +231,7 @@ def test_create_saved_view_reshapes_filter_for_api( assert attrs["name"] == "my-view" assert attrs["query-type"] == "workspaces" # Filter rows in the request body must be in the nested map shape. - assert attrs["query"]["filter"] == [ - {"workspace_name": {"contains": ["child"]}} - ] + assert attrs["query"]["filter"] == [{"workspace_name": {"contains": ["child"]}}] def test_read_saved_view(self, explorer_service, mock_transport): resp = Mock()