From bd932ce85f456e42617340103de443b53c0d6ce6 Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Wed, 2 Sep 2026 10:56:34 -0700 Subject: [PATCH] feat(import): remember the user's import selection Yaak now remembers, per item of a linked import source, whether the user wants it. A mapping row that still points at a model means wanted, so re-imports merge into it; a row with no model behind it means not wanted, so re-imports leave it alone. Wanted-ness is structural rather than a flag: deleting the model locally, or turning the item down in the preview, is what makes it not wanted. - import_source_resources drops `snapshot` for a nullable `content_hash` and lets `model_id` be NULL. The migration keeps every beta row's key to model mapping and starts hashes empty; until a hash is recorded, a difference can't be attributed to either side, so the item is offered once as a conflict that keeps local changes. - comparable() ignores sortPriority. Importers number it from source order, so inserting one operation used to plan a fake update for everything after it. The trade is that pure reorders don't propagate. - Keys the user turned down come back as unchecked "not imported" rows instead of being re-offered as new resources or resurrected. Checking one imports it under the same source key and pulls in the folders it needs. - An imported resource the source moves into a folder that isn't imported is offered as a deletion, with a tooltip pointing at the folder. - Plan-time source resolution binds by source-key overlap instead of by path, so a renamed or moved file merges into what it created and heals the stored origin. Several sources sharing keys never guess-merge: the plan says so and imports everything as new. Co-Authored-By: Claude Opus 5 --- Cargo.lock | 1 + .../components/ImportDataDialog.tsx | 50 +- .../components/core/CheckboxTree.tsx | 6 +- .../yaak-cli/src/commands/import_export.rs | 4 + .../yaak-cli/tests/import_export_commands.rs | 58 ++ .../yaak-rpc-schema/bindings/gen_models.ts | 11 - .../yaak-rpc-schema/bindings/gen_util.ts | 96 ++- crates/yaak-models/bindings/gen_models.ts | 10 +- crates/yaak-models/bindings/gen_util.ts | 96 ++- ...60902000000_import-source-content-hash.sql | 24 + crates/yaak-models/src/models.rs | 10 +- .../src/queries/import_source_resources.rs | 8 +- crates/yaak-models/src/util.rs | 12 + crates/yaak-plugins/bindings/gen_models.ts | 13 + crates/yaak/Cargo.toml | 1 + crates/yaak/src/import.rs | 710 +++++++++++++++--- 16 files changed, 923 insertions(+), 187 deletions(-) create mode 100644 crates/yaak-models/migrations/20260902000000_import-source-content-hash.sql diff --git a/Cargo.lock b/Cargo.lock index 4adf00f7..c81eef00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11228,6 +11228,7 @@ dependencies = [ "md5 0.8.0", "rusqlite", "serde_json", + "sha2", "tempfile", "thiserror 2.0.17", "tokio", diff --git a/apps/yaak-client/components/ImportDataDialog.tsx b/apps/yaak-client/components/ImportDataDialog.tsx index 95161301..aa612fef 100644 --- a/apps/yaak-client/components/ImportDataDialog.tsx +++ b/apps/yaak-client/components/ImportDataDialog.tsx @@ -261,13 +261,20 @@ function LoadedImportDataDialog({ const itemTree = useMemo(() => buildItemTree(items), [items]); // A folder row's checkbox aggregates its subtree the way the git commit tree does: creates and - // updates toggle together, while removals only ever cascade beneath a removed folder. + // updates toggle together, while removals only ever cascade beneath a removed folder. Checking + // anything also brings back the folders it needs to live in. const toggleNode = (node: CheckboxTreeNode, checked: boolean) => { const targets = new Set( collectItems(node) .filter((i) => togglesWith(node.data, i)) .map((i) => i.modelId), ); + if (checked) { + const byId = new Map(items.map((i) => [i.modelId, i])); + for (const ancestor of ancestorsOf(node.data, byId)) { + if (ancestor.action === "not_imported") targets.add(ancestor.modelId); + } + } setItems((prev) => prev.map((i) => (targets.has(i.modelId) ? { ...i, selected: checked } : i))); }; @@ -283,19 +290,17 @@ function LoadedImportDataDialog({ const disabled = new Set(); const byId = new Map(items.map((i) => [i.modelId, i])); for (const item of items) { - const seen = new Set(); - let parentId = item.parentId; - while (parentId != null && !seen.has(parentId)) { - seen.add(parentId); - const parent = byId.get(parentId); - if (parent == null || parent.model !== "folder") break; - if (parent.action === "create" && !parent.selected && item.action !== "delete") { + for (const parent of ancestorsOf(item, byId)) { + if (parent.model !== "folder") break; + const missing = + (parent.action === "create" || parent.action === "not_imported") && !parent.selected; + // A not-imported row stays checkable: checking it brings its folders back with it + if (missing && item.action !== "delete" && item.action !== "not_imported") { disabled.add(item.modelId); } if (parent.action === "delete" && parent.selected && item.action === "delete") { disabled.add(item.modelId); } - parentId = parent.parentId; } } return disabled; @@ -358,6 +363,7 @@ function LoadedImportDataDialog({ checked={nodeCheckedStatus} onCheck={toggleNode} isCheckboxDisabled={(n) => disabledIds.has(n.key)} + isCollapsedByDefault={(n) => n.data.action === "not_imported"} isRelevant={(n) => n.data.model === "workspace" || n.data.action !== "unchanged"} renderRow={(n) => } /> @@ -589,6 +595,7 @@ function ImportTreeRow({ item.action === "update" && "text-info", item.action === "delete" && "text-danger", item.action === "keep_local" && item.selected && "text-warning", + item.action === "not_imported" && "text-text-subtlest", )} > {actionLabel(item)} @@ -610,6 +617,8 @@ function actionLabel(item: ImportPlanItem): string | null { return "removed"; case "keep_local": return "edited"; + case "not_imported": + return "not imported"; default: return null; } @@ -622,16 +631,35 @@ function actionHelp(item: ImportPlanItem): string | null { case "update": return "Changed since the last import"; case "delete": - return "Deleted since the last import"; + return item.reason === "moved_into_not_imported_folder" + ? "Moved into a folder that isn't imported. Import that folder instead to follow the move" + : "Deleted since the last import"; case "keep_local": return "Local edits made since the last import. Importing will revert them if checked"; case "conflict": return "Changed both here and in the file since the last import"; + case "not_imported": + return "In the file, but not imported. Check it to import it"; default: return null; } } +/** Every plan item above `item`, nearest first. */ +function ancestorsOf(item: ImportPlanItem, byId: Map): ImportPlanItem[] { + const ancestors: ImportPlanItem[] = []; + const seen = new Set(); + let parentId = item.parentId; + while (parentId != null && !seen.has(parentId)) { + seen.add(parentId); + const parent = byId.get(parentId); + if (parent == null) break; + ancestors.push(parent); + parentId = parent.parentId; + } + return ancestors; +} + function buildItemTree(items: ImportPlanItem[]): CheckboxTreeNode[] { const byId = new Map(items.map((i) => [i.modelId, i])); const childrenOf = new Map(); @@ -678,7 +706,7 @@ function togglesWith(root: ImportPlanItem, item: ImportPlanItem): boolean { if (item.action === "keep_local") { return root.modelId === item.modelId && item.model !== "folder"; } - return item.action === "create" || item.action === "update"; + return item.action === "create" || item.action === "update" || item.action === "not_imported"; } function nodeCheckedStatus( diff --git a/apps/yaak-client/components/core/CheckboxTree.tsx b/apps/yaak-client/components/core/CheckboxTree.tsx index 0f714e35..d6d4423d 100644 --- a/apps/yaak-client/components/core/CheckboxTree.tsx +++ b/apps/yaak-client/components/core/CheckboxTree.tsx @@ -21,6 +21,8 @@ interface Props { isCheckboxDisabled?: (node: CheckboxTreeNode) => boolean; /** An irrelevant row is hidden unless one of its descendants is relevant */ isRelevant: (node: CheckboxTreeNode) => boolean; + /** A node that starts collapsed, so a large subtree doesn't crowd out the rest */ + isCollapsedByDefault?: (node: CheckboxTreeNode) => boolean; renderRow: (node: CheckboxTreeNode) => ReactNode; onSelectRow?: (node: CheckboxTreeNode) => void; canSelectRow?: (node: CheckboxTreeNode) => boolean; @@ -29,7 +31,9 @@ interface Props { export function CheckboxTree(props: Props) { const { node, depth = 0 } = props; - const [collapsed, setCollapsed] = useState(false); + const [collapsed, setCollapsed] = useState( + () => props.isCollapsedByDefault?.(node) ?? false, + ); if (!hasRelevantNode(node, props.isRelevant)) return null; const checked = props.checked(node); diff --git a/crates-cli/yaak-cli/src/commands/import_export.rs b/crates-cli/yaak-cli/src/commands/import_export.rs index cf6bc2f9..bc2728ef 100644 --- a/crates-cli/yaak-cli/src/commands/import_export.rs +++ b/crates-cli/yaak-cli/src/commands/import_export.rs @@ -113,6 +113,10 @@ fn format_skipped(items: &[ImportPlanItem]) -> Option { if keep_local > 0 { parts.push(format!("{keep_local} with local edits")); } + let not_imported = count(ImportPlanAction::NotImported); + if not_imported > 0 { + parts.push(format!("{not_imported} previously not imported")); + } let unchanged = count(ImportPlanAction::Unchanged); if unchanged > 0 { parts.push(format!("{unchanged} unchanged")); diff --git a/crates-cli/yaak-cli/tests/import_export_commands.rs b/crates-cli/yaak-cli/tests/import_export_commands.rs index f4d05e9e..f77c5c17 100644 --- a/crates-cli/yaak-cli/tests/import_export_commands.rs +++ b/crates-cli/yaak-cli/tests/import_export_commands.rs @@ -4,6 +4,7 @@ use common::{cli_cmd, parse_created_id, query_manager, seed_request}; use predicates::str::contains; use serde_json::Value; use tempfile::TempDir; +use yaak_models::util::UpdateSource; #[test] fn export_writes_yaak_workspace_file() { @@ -257,3 +258,60 @@ fn re_import_merges_into_linked_workspace() { assert!(requests.iter().any(|r| r.name == "Request B"), "removal must not auto-apply"); assert!(requests.iter().any(|r| r.name == "Request C")); } + +#[test] +fn re_import_leaves_deleted_resources_alone() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let data_dir = temp_dir.path(); + let import_path = temp_dir.path().join("linked.json"); + + write_linked_fixture( + &import_path, + &[ + ("req_a", "Request A", "https://example.com/a"), + ("req_b", "Request B", "https://example.com/b"), + ], + ); + cli_cmd(data_dir) + .args(["import", import_path.to_str().expect("import path is utf-8")]) + .assert() + .success(); + + let workspace_id = { + let query_manager = query_manager(data_dir); + let db = query_manager.connect(); + let workspace_id = db + .list_workspaces() + .expect("list workspaces") + .into_iter() + .find(|w| w.name == "Linked Workspace") + .expect("workspace imported") + .id; + let request_b = db + .list_http_requests(&workspace_id) + .expect("list requests") + .into_iter() + .find(|r| r.name == "Request B") + .expect("request B imported"); + db.delete_http_request_by_id(&request_b.id, &UpdateSource::Sync) + .expect("delete request B"); + workspace_id + }; + + cli_cmd(data_dir) + .args([ + "import", + import_path.to_str().expect("import path is utf-8"), + "--workspace-id", + &workspace_id, + ]) + .assert() + .success() + .stdout(contains("Skipped 1 previously not imported")); + + let query_manager = query_manager(data_dir); + let requests = + query_manager.connect().list_http_requests(&workspace_id).expect("list requests"); + assert_eq!(requests.len(), 1, "a deleted request must not come back: {requests:?}"); + assert_eq!(requests[0].name, "Request A"); +} diff --git a/crates/common/yaak-rpc-schema/bindings/gen_models.ts b/crates/common/yaak-rpc-schema/bindings/gen_models.ts index aa5fa233..ec4a53a4 100644 --- a/crates/common/yaak-rpc-schema/bindings/gen_models.ts +++ b/crates/common/yaak-rpc-schema/bindings/gen_models.ts @@ -331,17 +331,6 @@ export type ImportSource = { lastImportedAt: string; }; -export type ImportSourceResource = { - model: "import_source_resource"; - createdAt: string; - updatedAt: string; - importSourceId: string; - sourceKey: string; - modelType: string; - modelId: string; - snapshot: string; -}; - export type InheritedBoolSetting = { enabled?: boolean; value: boolean }; export type InheritedHttpVersionSetting = { enabled?: boolean; value: HttpVersion }; diff --git a/crates/common/yaak-rpc-schema/bindings/gen_util.ts b/crates/common/yaak-rpc-schema/bindings/gen_util.ts index 49d52e9f..f599771f 100644 --- a/crates/common/yaak-rpc-schema/bindings/gen_util.ts +++ b/crates/common/yaak-rpc-schema/bindings/gen_util.ts @@ -1,7 +1,21 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { Environment, Folder, GrpcRequest, HttpRequest, WebsocketRequest, Workspace } from "./gen_models"; +import type { + Environment, + Folder, + GrpcRequest, + HttpRequest, + WebsocketRequest, + Workspace, +} from "./gen_models"; -export type BatchUpsertResult = { workspaces: Array, environments: Array, folders: Array, httpRequests: Array, grpcRequests: Array, websocketRequests: Array, }; +export type BatchUpsertResult = { + workspaces: Array; + environments: Array; + folders: Array; + httpRequests: Array; + grpcRequests: Array; + websocketRequests: Array; +}; export type ImportConflictResolution = "keep_mine" | "take_source"; @@ -11,38 +25,74 @@ export type ImportConflictResolution = "keep_mine" | "take_source"; * The destination workspace and optional folder IDs are captured in the plan so the preview describes * the exact destination that confirmation will use. */ -export type ImportDestination = { "type": "new_workspace" } | { "type": "existing_workspace", workspaceId: string, folderId?: string, }; +export type ImportDestination = + | { type: "new_workspace" } + | { type: "existing_workspace"; workspaceId: string; folderId?: string }; /** * Where an import's contents came from, used to link the committed workspace back to it. */ -export type ImportOrigin = { -/** - * The absolute file path or URL the contents were read from. - */ -origin: string, label: string, }; +export type ImportOrigin = { + /** + * The absolute file path or URL the contents were read from. + */ + origin: string; + label: string; +}; -export type ImportPlan = { importer: string, destination: ImportDestination, resources: BatchUpsertResult, warnings: Array, -/** - * Stable source key for every model in `resources`, keyed by its planned ID. - */ -sourceKeys: { [key in string]?: string }, -/** - * One entry per plannable resource; commit applies only the selected ones. - */ -items: Array, origin?: ImportOrigin, }; +export type ImportPlan = { + importer: string; + destination: ImportDestination; + resources: BatchUpsertResult; + warnings: Array; + /** + * Stable source key for every model in `resources`, keyed by its planned ID. + */ + sourceKeys: { [key in string]?: string }; + /** + * One entry per plannable resource; commit applies only the selected ones. + */ + items: Array; + origin?: ImportOrigin; +}; -export type ImportPlanAction = "create" | "update" | "delete" | "unchanged" | "keep_local" | "conflict"; +export type ImportPlanAction = + | "create" + | "update" + | "delete" + | "unchanged" + | "keep_local" + | "conflict" + | "not_imported"; + +export type ImportPlanItem = { + action: ImportPlanAction; + model: ImportResourceType; + modelId: string; + name: string; + /** + * Planned parent folder ID for incoming resources; current parent for deletions. + */ + parentId?: string; + selected: boolean; + resolution?: ImportConflictResolution; + reason?: ImportPlanReason; +}; -export type ImportPlanItem = { action: ImportPlanAction, model: ImportResourceType, modelId: string, name: string, /** - * Planned parent folder ID for incoming resources; current parent for deletions. + * Extra context for an action that would otherwise be indistinguishable from its plain form. */ -parentId?: string, selected: boolean, resolution?: ImportConflictResolution, }; +export type ImportPlanReason = "moved_into_not_imported_folder"; -export type ImportPlanWarning = { title: string, detail: string, }; +export type ImportPlanWarning = { title: string; detail: string }; /** * The model types an import plan can contain. */ -export type ImportResourceType = "environment" | "folder" | "grpc_request" | "http_request" | "websocket_request" | "workspace"; +export type ImportResourceType = + | "environment" + | "folder" + | "grpc_request" + | "http_request" + | "websocket_request" + | "workspace"; diff --git a/crates/yaak-models/bindings/gen_models.ts b/crates/yaak-models/bindings/gen_models.ts index 5965fecd..c4a4a1f4 100644 --- a/crates/yaak-models/bindings/gen_models.ts +++ b/crates/yaak-models/bindings/gen_models.ts @@ -356,8 +356,14 @@ export type ImportSourceResource = { importSourceId: string; sourceKey: string; modelType: string; - modelId: string; - snapshot: string; + /** + * `None` once the user has decided not to import this key + */ + modelId?: string; + /** + * Hash of the resource as last applied or decided from the source, if one was recorded + */ + contentHash?: string; }; export type InheritedBoolSetting = { enabled?: boolean; value: boolean }; diff --git a/crates/yaak-models/bindings/gen_util.ts b/crates/yaak-models/bindings/gen_util.ts index 49d52e9f..f599771f 100644 --- a/crates/yaak-models/bindings/gen_util.ts +++ b/crates/yaak-models/bindings/gen_util.ts @@ -1,7 +1,21 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { Environment, Folder, GrpcRequest, HttpRequest, WebsocketRequest, Workspace } from "./gen_models"; +import type { + Environment, + Folder, + GrpcRequest, + HttpRequest, + WebsocketRequest, + Workspace, +} from "./gen_models"; -export type BatchUpsertResult = { workspaces: Array, environments: Array, folders: Array, httpRequests: Array, grpcRequests: Array, websocketRequests: Array, }; +export type BatchUpsertResult = { + workspaces: Array; + environments: Array; + folders: Array; + httpRequests: Array; + grpcRequests: Array; + websocketRequests: Array; +}; export type ImportConflictResolution = "keep_mine" | "take_source"; @@ -11,38 +25,74 @@ export type ImportConflictResolution = "keep_mine" | "take_source"; * The destination workspace and optional folder IDs are captured in the plan so the preview describes * the exact destination that confirmation will use. */ -export type ImportDestination = { "type": "new_workspace" } | { "type": "existing_workspace", workspaceId: string, folderId?: string, }; +export type ImportDestination = + | { type: "new_workspace" } + | { type: "existing_workspace"; workspaceId: string; folderId?: string }; /** * Where an import's contents came from, used to link the committed workspace back to it. */ -export type ImportOrigin = { -/** - * The absolute file path or URL the contents were read from. - */ -origin: string, label: string, }; +export type ImportOrigin = { + /** + * The absolute file path or URL the contents were read from. + */ + origin: string; + label: string; +}; -export type ImportPlan = { importer: string, destination: ImportDestination, resources: BatchUpsertResult, warnings: Array, -/** - * Stable source key for every model in `resources`, keyed by its planned ID. - */ -sourceKeys: { [key in string]?: string }, -/** - * One entry per plannable resource; commit applies only the selected ones. - */ -items: Array, origin?: ImportOrigin, }; +export type ImportPlan = { + importer: string; + destination: ImportDestination; + resources: BatchUpsertResult; + warnings: Array; + /** + * Stable source key for every model in `resources`, keyed by its planned ID. + */ + sourceKeys: { [key in string]?: string }; + /** + * One entry per plannable resource; commit applies only the selected ones. + */ + items: Array; + origin?: ImportOrigin; +}; -export type ImportPlanAction = "create" | "update" | "delete" | "unchanged" | "keep_local" | "conflict"; +export type ImportPlanAction = + | "create" + | "update" + | "delete" + | "unchanged" + | "keep_local" + | "conflict" + | "not_imported"; + +export type ImportPlanItem = { + action: ImportPlanAction; + model: ImportResourceType; + modelId: string; + name: string; + /** + * Planned parent folder ID for incoming resources; current parent for deletions. + */ + parentId?: string; + selected: boolean; + resolution?: ImportConflictResolution; + reason?: ImportPlanReason; +}; -export type ImportPlanItem = { action: ImportPlanAction, model: ImportResourceType, modelId: string, name: string, /** - * Planned parent folder ID for incoming resources; current parent for deletions. + * Extra context for an action that would otherwise be indistinguishable from its plain form. */ -parentId?: string, selected: boolean, resolution?: ImportConflictResolution, }; +export type ImportPlanReason = "moved_into_not_imported_folder"; -export type ImportPlanWarning = { title: string, detail: string, }; +export type ImportPlanWarning = { title: string; detail: string }; /** * The model types an import plan can contain. */ -export type ImportResourceType = "environment" | "folder" | "grpc_request" | "http_request" | "websocket_request" | "workspace"; +export type ImportResourceType = + | "environment" + | "folder" + | "grpc_request" + | "http_request" + | "websocket_request" + | "workspace"; diff --git a/crates/yaak-models/migrations/20260902000000_import-source-content-hash.sql b/crates/yaak-models/migrations/20260902000000_import-source-content-hash.sql new file mode 100644 index 00000000..b1001c0c --- /dev/null +++ b/crates/yaak-models/migrations/20260902000000_import-source-content-hash.sql @@ -0,0 +1,24 @@ +-- Replace the per-resource snapshot with a content hash, and let a row exist without a model +-- so a resource the user chose not to import can be remembered. +CREATE TABLE import_source_resources_new +( + model TEXT DEFAULT 'import_source_resource' NOT NULL, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + updated_at DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, + import_source_id TEXT NOT NULL, + source_key TEXT NOT NULL, + model_type TEXT NOT NULL, + model_id TEXT, + content_hash TEXT, + PRIMARY KEY (import_source_id, source_key) +); + +INSERT INTO import_source_resources_new (model, created_at, updated_at, import_source_id, + source_key, model_type, model_id, content_hash) +SELECT model, created_at, updated_at, import_source_id, source_key, model_type, model_id, NULL +FROM import_source_resources; + +DROP TABLE import_source_resources; + +ALTER TABLE import_source_resources_new + RENAME TO import_source_resources; diff --git a/crates/yaak-models/src/models.rs b/crates/yaak-models/src/models.rs index ae335efc..87501228 100644 --- a/crates/yaak-models/src/models.rs +++ b/crates/yaak-models/src/models.rs @@ -3118,8 +3118,12 @@ pub struct ImportSourceResource { pub import_source_id: String, pub source_key: String, pub model_type: String, - pub model_id: String, - pub snapshot: String, + /// `None` once the user has decided not to import this key + #[ts(optional)] + pub model_id: Option, + /// Hash of the resource as last applied or decided from the source, if one was recorded + #[ts(optional)] + pub content_hash: Option, } impl<'s> TryFrom<&Row<'s>> for ImportSourceResource { @@ -3134,7 +3138,7 @@ impl<'s> TryFrom<&Row<'s>> for ImportSourceResource { source_key: r.get("source_key")?, model_type: r.get("model_type")?, model_id: r.get("model_id")?, - snapshot: r.get("snapshot")?, + content_hash: r.get("content_hash")?, }) } } diff --git a/crates/yaak-models/src/queries/import_source_resources.rs b/crates/yaak-models/src/queries/import_source_resources.rs index aac7f0b0..6c0abb63 100644 --- a/crates/yaak-models/src/queries/import_source_resources.rs +++ b/crates/yaak-models/src/queries/import_source_resources.rs @@ -34,7 +34,7 @@ impl<'a> ClientDb<'a> { ImportSourceResourceIden::SourceKey, ImportSourceResourceIden::ModelType, ImportSourceResourceIden::ModelId, - ImportSourceResourceIden::Snapshot, + ImportSourceResourceIden::ContentHash, ]) .values_panic([ CurrentTimestamp.into(), @@ -42,8 +42,8 @@ impl<'a> ClientDb<'a> { resource.import_source_id.as_str().into(), resource.source_key.as_str().into(), resource.model_type.as_str().into(), - resource.model_id.as_str().into(), - resource.snapshot.as_str().into(), + resource.model_id.clone().into(), + resource.content_hash.clone().into(), ]) .on_conflict( OnConflict::columns([ @@ -54,7 +54,7 @@ impl<'a> ClientDb<'a> { ImportSourceResourceIden::UpdatedAt, ImportSourceResourceIden::ModelType, ImportSourceResourceIden::ModelId, - ImportSourceResourceIden::Snapshot, + ImportSourceResourceIden::ContentHash, ]) .to_owned(), ) diff --git a/crates/yaak-models/src/util.rs b/crates/yaak-models/src/util.rs index f3c0befe..f44d09dd 100644 --- a/crates/yaak-models/src/util.rs +++ b/crates/yaak-models/src/util.rs @@ -169,6 +169,16 @@ pub enum ImportPlanAction { Unchanged, KeepLocal, Conflict, + /// Present in the source but previously turned down; selecting it imports it again + NotImported, +} + +/// Extra context for an action that would otherwise be indistinguishable from its plain form. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, TS)] +#[serde(rename_all = "snake_case")] +#[ts(export, export_to = "gen_util.ts")] +pub enum ImportPlanReason { + MovedIntoNotImportedFolder, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, TS)] @@ -193,6 +203,8 @@ pub struct ImportPlanItem { pub selected: bool, #[ts(optional)] pub resolution: Option, + #[ts(optional)] + pub reason: Option, } #[derive(Debug, Deserialize, Serialize, TS)] diff --git a/crates/yaak-plugins/bindings/gen_models.ts b/crates/yaak-plugins/bindings/gen_models.ts index 1f09c532..c03e2595 100644 --- a/crates/yaak-plugins/bindings/gen_models.ts +++ b/crates/yaak-plugins/bindings/gen_models.ts @@ -11,6 +11,7 @@ export type AnyModel = | HttpRequest | HttpResponse | HttpResponseEvent + | ImportSource | KeyValue | Plugin | Settings @@ -318,6 +319,18 @@ export type HttpUrlParameter = { export type HttpVersion = "auto" | "http1" | "http2"; +export type ImportSource = { + model: "import_source"; + id: string; + createdAt: string; + updatedAt: string; + workspaceId: string; + importer: string; + origin: string; + originLabel: string; + lastImportedAt: string; +}; + export type InheritedBoolSetting = { enabled?: boolean; value: boolean }; export type InheritedHttpVersionSetting = { enabled?: boolean; value: HttpVersion }; diff --git a/crates/yaak/Cargo.toml b/crates/yaak/Cargo.toml index 59e5a0fa..dadade2a 100644 --- a/crates/yaak/Cargo.toml +++ b/crates/yaak/Cargo.toml @@ -9,6 +9,7 @@ async-trait = "0.1" base64 = "0.22.1" # For carrying body chunks over a text-only plugin transport log = { workspace = true } md5 = "0.8.0" +sha2 = { workspace = true } chrono = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } diff --git a/crates/yaak/src/import.rs b/crates/yaak/src/import.rs index 82501f1e..6e1056a5 100644 --- a/crates/yaak/src/import.rs +++ b/crates/yaak/src/import.rs @@ -2,6 +2,7 @@ use crate::Result; use chrono::Utc; use log::info; use serde_json::Value; +use sha2::{Digest, Sha256}; use std::collections::{BTreeMap, BTreeSet}; use yaak_models::client_db::ClientDb; use yaak_models::models::{ @@ -11,7 +12,8 @@ use yaak_models::models::{ use yaak_models::query_manager::QueryManager; use yaak_models::util::{ BatchUpsertResult, ImportConflictResolution, ImportDestination, ImportOrigin, ImportPlan, - ImportPlanAction, ImportPlanItem, ImportPlanWarning, ImportResourceType, UpdateSource, + ImportPlanAction, ImportPlanItem, ImportPlanReason, ImportPlanWarning, ImportResourceType, + UpdateSource, }; use yaak_plugins::events::{ImportResources, PluginContext}; use yaak_plugins::manager::PluginManager; @@ -306,9 +308,10 @@ fn commit_plan_in_tx(db: &ClientDb, plan: ImportPlan) -> Result true, Some(item) => match item.action { - ImportPlanAction::Create | ImportPlanAction::Update | ImportPlanAction::KeepLocal => { - item.selected - } + ImportPlanAction::Create + | ImportPlanAction::Update + | ImportPlanAction::KeepLocal + | ImportPlanAction::NotImported => item.selected, ImportPlanAction::Conflict => { item.resolution == Some(ImportConflictResolution::TakeSource) } @@ -316,9 +319,13 @@ fn commit_plan_in_tx(db: &ClientDb, plan: ImportPlan) -> Result = plan .resources .folders @@ -436,12 +443,13 @@ fn delete_existing_model(db: &ClientDb, resource: ImportResourceType, id: &str) Ok(()) } -/// Link the committed workspace to the import's origin and store a snapshot per resource, so the -/// next import from the same origin can three-way merge instead of duplicating everything. +/// Link the committed workspace to the import's source and record, per source key, whether the +/// user wants that resource and which version of it they last decided on. /// -/// Snapshots advance for everything the user decided on this round — applied items and keep-mine -/// conflicts alike — while deselected updates and deletions keep their old snapshot so they are -/// offered again next time. +/// Hashes advance for everything the user decided on this round — applied items and keep-mine +/// conflicts alike — while deselected updates and deletions keep their old hash so they are +/// offered again next time. A resource the user turned down is remembered as a row without a +/// model, so it is neither re-offered nor resurrected. fn record_import_source( db: &ClientDb, plan: &ImportPlan, @@ -460,7 +468,17 @@ fn record_import_source( }, }; - let existing = db.find_import_source(&workspace_id, &plan.importer, &origin.origin)?; + let incoming_keys: BTreeSet = plan.source_keys.values().cloned().collect(); + let existing = match resolve_linked_source( + db, + &workspace_id, + &plan.importer, + origin, + &incoming_keys, + )? { + LinkedSource::Linked(source) => Some(source), + LinkedSource::Ambiguous(_) | LinkedSource::Unlinked => None, + }; let import_source = db.upsert_import_source( &ImportSource { id: existing.map(|s| s.id).unwrap_or_default(), @@ -476,83 +494,89 @@ fn record_import_source( let mut committed: BTreeMap<&str, String> = BTreeMap::new(); for v in &upserted.environments { - committed.insert(&v.id, serde_json::to_string(v)?); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); } for v in &upserted.folders { - committed.insert(&v.id, serde_json::to_string(v)?); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); } for v in &upserted.http_requests { - committed.insert(&v.id, serde_json::to_string(v)?); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); } for v in &upserted.grpc_requests { - committed.insert(&v.id, serde_json::to_string(v)?); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); } for v in &upserted.websocket_requests { - committed.insert(&v.id, serde_json::to_string(v)?); + committed.insert(&v.id, content_hash(serde_json::to_value(v)?)); } - let write_row = |model_id: &str, resource: ImportResourceType, incoming: &dyn Fn() -> Result| -> Result<()> { + let write_row = |model_id: &str, + resource: ImportResourceType, + incoming: &dyn Fn() -> Result| + -> Result<()> { let Some(source_key) = plan.source_keys.get(model_id) else { return Ok(()); }; - let snapshot = match committed.get(model_id) { - Some(json) => json.clone(), - None => match items.get(model_id).map(|i| i.action) { + let item = items.get(model_id); + let next_row = match committed.get(model_id) { + Some(hash) => Some((Some(model_id.to_string()), Some(hash.clone()))), + None => match item.map(|i| i.action) { Some( ImportPlanAction::Unchanged | ImportPlanAction::KeepLocal | ImportPlanAction::Conflict, - ) => incoming()?, - // A deselected create, update, or delete stays offered next import - Some( - ImportPlanAction::Create - | ImportPlanAction::Update - | ImportPlanAction::Delete, - ) - | None => return Ok(()), + ) => Some((Some(model_id.to_string()), Some(content_hash(incoming()?)))), + // Turned down, so remember it as not wanted rather than offering it again + Some(ImportPlanAction::Create | ImportPlanAction::NotImported) => Some((None, None)), + Some(ImportPlanAction::Delete) if item.is_some_and(|i| i.selected) => { + Some((None, None)) + } + // A deselected update or deletion stays offered next import + Some(ImportPlanAction::Update | ImportPlanAction::Delete) | None => None, }, }; + let Some((model_id, content_hash)) = next_row else { + return Ok(()); + }; db.upsert_import_source_resource(&ImportSourceResource { import_source_id: import_source.id.clone(), source_key: source_key.clone(), model_type: resource.as_str().to_string(), - model_id: model_id.to_string(), - snapshot, + model_id, + content_hash, ..Default::default() })?; Ok(()) }; for v in &plan.resources.environments { - write_row(&v.id, ImportResourceType::Environment, &|| Ok(serde_json::to_string(v)?))?; + write_row(&v.id, ImportResourceType::Environment, &|| Ok(serde_json::to_value(v)?))?; } for v in &plan.resources.folders { - write_row(&v.id, ImportResourceType::Folder, &|| Ok(serde_json::to_string(v)?))?; + write_row(&v.id, ImportResourceType::Folder, &|| Ok(serde_json::to_value(v)?))?; } for v in &plan.resources.http_requests { - write_row(&v.id, ImportResourceType::HttpRequest, &|| Ok(serde_json::to_string(v)?))?; + write_row(&v.id, ImportResourceType::HttpRequest, &|| Ok(serde_json::to_value(v)?))?; } for v in &plan.resources.grpc_requests { - write_row(&v.id, ImportResourceType::GrpcRequest, &|| Ok(serde_json::to_string(v)?))?; + write_row(&v.id, ImportResourceType::GrpcRequest, &|| Ok(serde_json::to_value(v)?))?; } for v in &plan.resources.websocket_requests { - write_row(&v.id, ImportResourceType::WebsocketRequest, &|| Ok(serde_json::to_string(v)?))?; + write_row(&v.id, ImportResourceType::WebsocketRequest, &|| Ok(serde_json::to_value(v)?))?; } - let incoming_keys: BTreeSet<&String> = plan.source_keys.values().collect(); for row in db.list_import_source_resources(&import_source.id)? { if incoming_keys.contains(&row.source_key) { continue; } // Keep only rows that back a deletion the user deselected; it will be offered again. - let keep = match ImportResourceType::from_str(&row.model_type) { - Some(resource) => { + let keep = match (ImportResourceType::from_str(&row.model_type), &row.model_id) { + (Some(resource), Some(model_id)) => { items - .get(&row.model_id) + .get(model_id) .is_some_and(|i| i.action == ImportPlanAction::Delete && !i.selected) - && existing_model_json(db, resource, &row.model_id)?.is_some() + && existing_model_json(db, resource, model_id)?.is_some() } - None => false, + _ => false, }; if !keep { db.delete_import_source_resource(&import_source.id, &row.source_key)?; @@ -562,9 +586,64 @@ fn record_import_source( Ok(()) } +/// How an import's contents relate to what the destination workspace already has linked. +enum LinkedSource { + Linked(ImportSource), + /// Several linked sources contain these resources, so merging would have to guess + Ambiguous(Vec), + Unlinked, +} + +/// A re-import is recognized by the source keys it carries rather than by where it was read from, +/// so a file that moved or was renamed still merges into what it created. +fn resolve_linked_source( + db: &ClientDb, + workspace_id: &str, + importer: &str, + origin: &ImportOrigin, + incoming_keys: &BTreeSet, +) -> Result { + let sources = db.list_import_sources(workspace_id)?; + let same_origin = |source: &ImportSource| { + source.importer == importer && source.origin == origin.origin + }; + + let mut overlapping = Vec::new(); + for source in &sources { + let overlaps = db + .list_import_source_resources(&source.id)? + .iter() + .any(|row| incoming_keys.contains(&row.source_key)); + if overlaps { + overlapping.push(source.clone()); + } + } + + Ok(match overlapping.len() { + 0 => match sources.into_iter().find(same_origin) { + Some(source) => LinkedSource::Linked(source), + None => LinkedSource::Unlinked, + }, + 1 => LinkedSource::Linked(overlapping.remove(0)), + _ => match overlapping.iter().find(|s| same_origin(s)) { + Some(source) => LinkedSource::Linked(source.clone()), + None => LinkedSource::Ambiguous(overlapping), + }, + }) +} + +fn ambiguous_source_warning(sources: &[ImportSource]) -> ImportPlanWarning { + let labels = sources.iter().map(|s| s.origin_label.as_str()).collect::>(); + ImportPlanWarning { + title: "Imported as new".to_string(), + detail: format!("{} already contain these resources", display_list(&labels)), + } +} + /// Rewrite a plan against the destination's linked import source, if it has one: resources whose -/// source key was seen before adopt the existing model's ID, and every resource gets a plan item -/// describing the create / update / delete / conflict decision to preview. +/// source key still has a model adopt that model's ID, and every resource gets a plan item +/// describing the decision to preview. Keys the user has turned down come back as offers to +/// import them after all, rather than as new resources. fn merge_with_linked_source( query_manager: &QueryManager, plan: &mut ImportPlan, @@ -572,16 +651,25 @@ fn merge_with_linked_source( ) -> Result<()> { let db = query_manager.connect(); + let incoming_keys: BTreeSet = plan.source_keys.values().cloned().collect(); let linked = match (&plan.origin, &plan.destination) { (Some(origin), ImportDestination::ExistingWorkspace { workspace_id, .. }) => { - db.find_import_source(workspace_id, &plan.importer, &origin.origin)? + resolve_linked_source(&db, workspace_id, &plan.importer, origin, &incoming_keys)? } - (None, _) | (Some(_), ImportDestination::NewWorkspace) => None, + (None, _) | (Some(_), ImportDestination::NewWorkspace) => LinkedSource::Unlinked, }; - let Some(source) = linked else { - plan.items = create_only_items(plan); - return Ok(()); + let source = match linked { + LinkedSource::Linked(source) => source, + LinkedSource::Ambiguous(sources) => { + plan.warnings.push(ambiguous_source_warning(&sources)); + plan.items = create_only_items(plan); + return Ok(()); + } + LinkedSource::Unlinked => { + plan.items = create_only_items(plan); + return Ok(()); + } }; let workspace_id = source.workspace_id.clone(); @@ -601,14 +689,15 @@ fn merge_with_linked_source( if ImportResourceType::from_str(&row.model_type) != Some(resource) { return Ok(()); } - let Some(current) = existing_model_json(&db, resource, &row.model_id)? else { + let Some(model_id) = row.model_id.as_deref() else { return Ok(()) }; + let Some(current) = existing_model_json(&db, resource, model_id)? else { return Ok(()); }; if current.get("workspaceId").and_then(|v| v.as_str()) != Some(workspace_id.as_str()) { return Ok(()); } - remap.insert(planned_id.to_string(), row.model_id.clone()); - current_models.insert(row.model_id.clone(), current); + remap.insert(planned_id.to_string(), model_id.to_string()); + current_models.insert(model_id.to_string(), current); Ok(()) }; for v in &plan.resources.folders { @@ -693,6 +782,50 @@ fn merge_with_linked_source( }); } + // A key whose row still points at a model in this workspace is one the user wants; a key + // whose row has lost its model is one they turned down. + let status = |planned_id: &str, resource: ImportResourceType| -> KeyStatus { + let Some(row) = plan.source_keys.get(planned_id).and_then(|key| rows.get(key)) else { + return KeyStatus::New; + }; + if ImportResourceType::from_str(&row.model_type) != Some(resource) { + return KeyStatus::New; + } + if row.model_id.as_deref() == Some(planned_id) && current_models.contains_key(planned_id) { + KeyStatus::Wanted(row) + } else { + KeyStatus::NotWanted + } + }; + + let not_imported_folders = plan + .resources + .folders + .iter() + .filter(|v| matches!(status(&v.id, ImportResourceType::Folder), KeyStatus::NotWanted)) + .map(|v| v.id.as_str()) + .collect::>(); + let folder_parents = plan + .resources + .folders + .iter() + .map(|v| (v.id.as_str(), v.folder_id.as_deref())) + .collect::>(); + let inside_a_not_imported_folder = |parent_id: Option<&str>| { + let mut seen = BTreeSet::new(); + let mut next = parent_id; + while let Some(id) = next { + if !seen.insert(id) { + break; + } + if not_imported_folders.contains(id) { + return true; + } + next = folder_parents.get(id).copied().flatten(); + } + false + }; + let mut items = Vec::new(); { let mut classify = |any: AnyModel, @@ -700,38 +833,53 @@ fn merge_with_linked_source( parent_id: Option| -> Result<()> { let planned_id = any.id().to_string(); - let name = any.resolved_name(); - - let mapped = plan - .source_keys - .get(&planned_id) - .and_then(|key| rows.get(key)) - .filter(|row| row.model_id == planned_id); - let Some(row) = mapped else { - items.push(ImportPlanItem { - action: ImportPlanAction::Create, - model: resource, - model_id: planned_id, - name, - parent_id, - selected: true, - resolution: None, - }); - return Ok(()); + let item = |action, selected, resolution, reason| ImportPlanItem { + action, + model: resource, + model_id: planned_id.clone(), + name: any.resolved_name(), + parent_id: parent_id.clone(), + selected, + resolution, + reason, }; - let incoming = comparable(serde_json::to_value(&any)?); - let current = current_models - .get(&planned_id) - .cloned() - .map(comparable) - .unwrap_or_default(); - let (source_changed, local_changed) = - match serde_json::from_str::(&row.snapshot).ok().map(comparable) { - Some(snapshot) => (incoming != snapshot, current != snapshot), - // An unreadable snapshot can't prove anything unchanged, so surface a conflict. - None => (true, true), - }; + // Nothing can be created inside a folder that isn't imported, so it starts unchecked + // and comes along only if the folder does. + let reachable = !inside_a_not_imported_folder(parent_id.as_deref()); + let row = match status(&planned_id, resource) { + KeyStatus::New => { + items.push(item(ImportPlanAction::Create, reachable, None, None)); + return Ok(()); + } + KeyStatus::NotWanted => { + items.push(item(ImportPlanAction::NotImported, false, None, None)); + return Ok(()); + } + KeyStatus::Wanted(row) => row, + }; + + // The source put it somewhere the workspace has no folder for, so it can't stay + if !reachable { + items.push(item( + ImportPlanAction::Delete, + false, + None, + Some(ImportPlanReason::MovedIntoNotImportedFolder), + )); + return Ok(()); + } + + let incoming = serde_json::to_value(&any)?; + let current = current_models.get(&planned_id).cloned().unwrap_or_default(); + let (source_changed, local_changed) = match recorded_hash(row) { + Some(hash) => (content_hash(incoming) != hash, content_hash(current) != hash), + // Without a recorded version, all that can be told is whether the two sides differ + None => { + let differs = comparable(incoming) != comparable(current); + (differs, differs) + } + }; let (action, selected, resolution) = match (source_changed, local_changed) { (false, false) => (ImportPlanAction::Unchanged, false, None), @@ -743,15 +891,7 @@ fn merge_with_linked_source( Some(ImportConflictResolution::KeepMine), ), }; - items.push(ImportPlanItem { - action, - model: resource, - model_id: planned_id, - name, - parent_id, - selected, - resolution, - }); + items.push(item(action, selected, resolution, None)); Ok(()) }; @@ -773,7 +913,6 @@ fn merge_with_linked_source( } // Mapped models the source no longer has become deletion offers, deselected by default. - let incoming_keys: BTreeSet<&String> = plan.source_keys.values().collect(); for (key, row) in &rows { if incoming_keys.contains(key) { continue; @@ -781,7 +920,10 @@ fn merge_with_linked_source( let Some(resource) = ImportResourceType::from_str(&row.model_type) else { continue; }; - let Some(current) = existing_model_json(&db, resource, &row.model_id)? else { + let Some(model_id) = row.model_id.as_deref() else { + continue; + }; + let Some(current) = existing_model_json(&db, resource, model_id)? else { continue; }; if current.get("workspaceId").and_then(|v| v.as_str()) != Some(workspace_id.as_str()) { @@ -798,11 +940,12 @@ fn merge_with_linked_source( items.push(ImportPlanItem { action: ImportPlanAction::Delete, model: resource, - model_id: row.model_id.clone(), + model_id: model_id.to_string(), name, parent_id, selected: false, resolution: None, + reason: None, }); } @@ -821,6 +964,7 @@ fn create_only_items(plan: &ImportPlan) -> Vec { parent_id, selected: true, resolution: None, + reason: None, }); }; for v in &plan.resources.folders { @@ -841,17 +985,57 @@ fn create_only_items(plan: &ImportPlan) -> Vec { items } +/// What a source key means for the resource the plan wants to put behind it. +enum KeyStatus<'a> { + /// The source has never been imported under this key + New, + Wanted(&'a ImportSourceResource), + /// Imported under this key before, and since turned down or deleted + NotWanted, +} + /// Strip identity and bookkeeping fields so equality means "same content in the same place". /// The deprecated environment `base` flag mirrors `parentModel`, which is compared already. +/// Importers number `sortPriority` from source order, so comparing it would turn one insertion +/// into an update of everything after it. fn comparable(mut value: Value) -> Value { if let Some(object) = value.as_object_mut() { - for field in ["id", "model", "workspaceId", "createdAt", "updatedAt", "base"] { + for field in + ["id", "model", "workspaceId", "createdAt", "updatedAt", "base", "sortPriority"] + { object.remove(field); } } value } +const CONTENT_HASH_VERSION: &str = "v1:"; + +/// Identifies a resource's content well enough to tell "changed since the last import" from +/// "unchanged", without keeping a copy of every imported resource around. +fn content_hash(value: Value) -> String { + let canonical = serde_json::to_string(&sorted_keys(comparable(value))).unwrap_or_default(); + format!("{CONTENT_HASH_VERSION}{:x}", Sha256::digest(canonical.as_bytes())) +} + +/// A hash written by a version this build doesn't understand says nothing about the resource. +fn recorded_hash(row: &ImportSourceResource) -> Option<&str> { + let hash = row.content_hash.as_deref()?; + hash.starts_with(CONTENT_HASH_VERSION).then_some(hash) +} + +fn sorted_keys(value: Value) -> Value { + match value { + Value::Object(object) => { + let mut entries = object.into_iter().collect::>(); + entries.sort_by(|(a, _), (b, _)| a.cmp(b)); + Value::Object(entries.into_iter().map(|(k, v)| (k, sorted_keys(v))).collect()) + } + Value::Array(items) => Value::Array(items.into_iter().map(sorted_keys).collect()), + other => other, + } +} + fn existing_model_json( db: &ClientDb, resource: ImportResourceType, @@ -928,9 +1112,13 @@ fn validate_plan(plan: &ImportPlan) -> Result<()> { // A merging plan may update the base environment it created earlier, which its plan // item records; anything else must not replace the destination's base environment. let updates_own_base = |id: &str| { - plan.items - .iter() - .any(|i| i.model_id == id && i.action != ImportPlanAction::Create) + plan.items.iter().any(|i| { + i.model_id == id + && !matches!( + i.action, + ImportPlanAction::Create | ImportPlanAction::NotImported + ) + }) }; if plan .resources @@ -1701,6 +1889,7 @@ mod tests { ("rq_root".to_string(), "op:root".to_string()), ("rq_nested".to_string(), "op:nested".to_string()), ("rq_extra".to_string(), "op:extra".to_string()), + ("fl_extra".to_string(), "folder:extra".to_string()), ]) } @@ -1721,6 +1910,15 @@ mod tests { query_manager: &QueryManager, workspace_id: &str, resources: ImportResources, + ) -> ImportPlan { + replan_from(query_manager, workspace_id, resources, linked_origin()) + } + + fn replan_from( + query_manager: &QueryManager, + workspace_id: &str, + resources: ImportResources, + origin: ImportOrigin, ) -> ImportPlan { plan_import_resources( query_manager, @@ -1731,11 +1929,39 @@ mod tests { }, resources, Some(importer_keys()), - Some(linked_origin()), + Some(origin), ) .expect("plan re-import") } + fn extra_request() -> HttpRequest { + HttpRequest { + id: "rq_extra".to_string(), + model: "http_request".to_string(), + workspace_id: "wk_source".to_string(), + name: "Extra Request".to_string(), + method: "GET".to_string(), + url: "https://example.com/extra".to_string(), + ..Default::default() + } + } + + fn select(plan: &mut ImportPlan, name: &str, selected: bool) { + let item = plan + .items + .iter_mut() + .find(|i| i.name == name) + .unwrap_or_else(|| panic!("no plan item named {name}")); + item.selected = selected; + } + + fn source_rows(query_manager: &QueryManager, workspace_id: &str) -> Vec { + let db = query_manager.connect(); + let sources = db.list_import_sources(workspace_id).expect("list import sources"); + assert_eq!(sources.len(), 1, "expected one linked source: {sources:?}"); + db.list_import_source_resources(&sources[0].id).expect("list resource rows") + } + fn item_by_name<'a>(plan: &'a ImportPlan, name: &str) -> &'a ImportPlanItem { plan.items .iter() @@ -1744,7 +1970,7 @@ mod tests { } #[test] - fn commit_records_the_linked_source_and_snapshots() { + fn commit_records_the_linked_source_and_hashes() { let (query_manager, _blob_manager, _rx) = yaak_models::init_in_memory().expect("initialize database"); let committed = first_import(&query_manager); @@ -1761,13 +1987,13 @@ mod tests { let rows = db.list_import_source_resources(&source.id).expect("list resource rows"); assert_eq!(rows.len(), 4, "one row per non-workspace resource: {rows:?}"); for row in &rows { - let snapshot: Value = serde_json::from_str(&row.snapshot).expect("parse snapshot"); + let model_id = row.model_id.as_deref().expect("row is wanted"); let resource = ImportResourceType::from_str(&row.model_type) .expect("row has a known resource type"); - let current = existing_model_json(&db, resource, &row.model_id) + let current = existing_model_json(&db, resource, model_id) .expect("query current model") .expect("row target exists"); - assert_eq!(comparable(snapshot), comparable(current)); + assert_eq!(row.content_hash.as_deref(), Some(content_hash(current).as_str())); } source }; @@ -2038,7 +2264,7 @@ mod tests { } #[test] - fn locally_deleted_mapped_model_plans_as_create() { + fn locally_deleted_mapped_model_is_not_resurrected() { let (query_manager, _blob_manager, _rx) = yaak_models::init_in_memory().expect("initialize database"); let committed = first_import(&query_manager); @@ -2058,13 +2284,277 @@ mod tests { let plan = replan(&query_manager, &workspace_id, imported_resources()); let root = item_by_name(&plan, "Root Request"); - assert_eq!(root.action, ImportPlanAction::Create, "no silent resurrection as an update"); + assert_eq!(root.action, ImportPlanAction::NotImported, "deleting it means not wanting it"); + assert!(!root.selected); assert_ne!(root.model_id, root_id); - commit_import_plan(&query_manager, plan).expect("commit re-create"); + commit_import_plan(&query_manager, plan).expect("commit re-import"); assert_eq!( query_manager.connect().list_http_requests(&workspace_id).expect("list").len(), - 2 + 1, + "a locally deleted resource must not come back" + ); + + let plan = replan(&query_manager, &workspace_id, imported_resources()); + assert_eq!(item_by_name(&plan, "Root Request").action, ImportPlanAction::NotImported); + } + + #[test] + fn deselected_create_is_not_offered_again() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let committed = first_import(&query_manager); + let workspace_id = committed.workspaces[0].id.clone(); + + let mut resources = imported_resources(); + resources.http_requests.push(extra_request()); + + let mut plan = replan(&query_manager, &workspace_id, resources.clone()); + assert_eq!(item_by_name(&plan, "Extra Request").action, ImportPlanAction::Create); + select(&mut plan, "Extra Request", false); + commit_import_plan(&query_manager, plan).expect("commit with deselected create"); + assert_eq!( + query_manager.connect().list_http_requests(&workspace_id).expect("list").len(), + 2, + "a deselected create must not be created" + ); + + let plan = replan(&query_manager, &workspace_id, resources); + let extra = item_by_name(&plan, "Extra Request"); + assert_eq!(extra.action, ImportPlanAction::NotImported, "deselecting it is remembered"); + assert!(!extra.selected); + } + + #[test] + fn restoring_a_not_imported_item_relinks_its_source_key() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let committed = first_import(&query_manager); + let workspace_id = committed.workspaces[0].id.clone(); + + let mut resources = imported_resources(); + resources.http_requests.push(extra_request()); + + let mut plan = replan(&query_manager, &workspace_id, resources.clone()); + select(&mut plan, "Extra Request", false); + commit_import_plan(&query_manager, plan).expect("commit with deselected create"); + + let mut plan = replan(&query_manager, &workspace_id, resources.clone()); + assert_eq!(item_by_name(&plan, "Extra Request").action, ImportPlanAction::NotImported); + select(&mut plan, "Extra Request", true); + commit_import_plan(&query_manager, plan).expect("commit with restored item"); + + let extra = query_manager + .connect() + .list_http_requests(&workspace_id) + .expect("list") + .into_iter() + .find(|r| r.name == "Extra Request") + .expect("restored request exists"); + let rows = source_rows(&query_manager, &workspace_id); + let row = rows.iter().find(|r| r.source_key == "op:extra").expect("row for the same key"); + assert_eq!(row.model_id.as_deref(), Some(extra.id.as_str()), "the key links to the model"); + + let plan = replan(&query_manager, &workspace_id, resources); + assert_eq!(item_by_name(&plan, "Extra Request").action, ImportPlanAction::Unchanged); + } + + #[test] + fn moving_into_a_not_imported_folder_offers_a_delete() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let committed = first_import(&query_manager); + let workspace_id = committed.workspaces[0].id.clone(); + + let with_extra_folder = |root_inside: bool| { + let mut resources = imported_resources(); + resources.folders.push(Folder { + id: "fl_extra".to_string(), + model: "folder".to_string(), + workspace_id: "wk_source".to_string(), + name: "Extra Folder".to_string(), + ..Default::default() + }); + if root_inside { + resources.http_requests[0].folder_id = Some("fl_extra".to_string()); + } + resources + }; + + let mut plan = replan(&query_manager, &workspace_id, with_extra_folder(false)); + select(&mut plan, "Extra Folder", false); + commit_import_plan(&query_manager, plan).expect("commit without the new folder"); + + // The source moves an imported request into the folder that was turned down. + let plan = replan(&query_manager, &workspace_id, with_extra_folder(true)); + assert_eq!(item_by_name(&plan, "Extra Folder").action, ImportPlanAction::NotImported); + let root = item_by_name(&plan, "Root Request"); + assert_eq!(root.action, ImportPlanAction::Delete); + assert!(!root.selected, "a deletion is never applied by default"); + assert_eq!(root.reason, Some(ImportPlanReason::MovedIntoNotImportedFolder)); + + // Anything new inside that folder can't be created either, so it waits for the folder. + let mut resources = with_extra_folder(true); + resources.http_requests.push(HttpRequest { + folder_id: Some("fl_extra".to_string()), + ..extra_request() + }); + let plan = replan(&query_manager, &workspace_id, resources); + let extra = item_by_name(&plan, "Extra Request"); + assert_eq!(extra.action, ImportPlanAction::Create); + assert!(!extra.selected, "a create with nowhere to go starts unchecked"); + + let mut plan = replan(&query_manager, &workspace_id, with_extra_folder(true)); + select(&mut plan, "Root Request", true); + commit_import_plan(&query_manager, plan).expect("commit the move as a deletion"); + + let db = query_manager.connect(); + let requests = db.list_http_requests(&workspace_id).expect("list"); + assert!(!requests.iter().any(|r| r.name == "Root Request"), "accepting deletes it"); + drop(db); + + let plan = replan(&query_manager, &workspace_id, with_extra_folder(true)); + assert_eq!( + item_by_name(&plan, "Root Request").action, + ImportPlanAction::NotImported, + "accepting the deletion means the resource is no longer wanted" + ); + } + + #[test] + fn a_source_that_moved_still_merges_and_updates_its_origin() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let committed = first_import(&query_manager); + let workspace_id = committed.workspaces[0].id.clone(); + + let moved = + ImportOrigin { origin: "/tmp/api/v1.yaml".to_string(), label: "v1.yaml".to_string() }; + let mut resources = imported_resources(); + resources.http_requests[0].url = "https://example.com/root-v2".to_string(); + + let plan = replan_from(&query_manager, &workspace_id, resources, moved.clone()); + assert!(plan.warnings.iter().all(|w| w.title != "Imported as new"), "{:?}", plan.warnings); + assert_eq!(item_by_name(&plan, "Root Request").action, ImportPlanAction::Update); + commit_import_plan(&query_manager, plan).expect("commit merge from the new path"); + + let db = query_manager.connect(); + assert_eq!( + db.list_http_requests(&workspace_id).expect("list").len(), + 2, + "a renamed file must not duplicate what it created" + ); + let sources = db.list_import_sources(&workspace_id).expect("list import sources"); + assert_eq!(sources.len(), 1, "the link follows the file: {sources:?}"); + assert_eq!(sources[0].origin, moved.origin); + assert_eq!(sources[0].origin_label, moved.label); + } + + #[test] + fn an_ambiguous_overlap_warns_instead_of_merging() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let committed = first_import(&query_manager); + let workspace_id = committed.workspaces[0].id.clone(); + + // A second source claiming the same keys leaves nothing to merge into safely. + { + let db = query_manager.connect(); + let other = db + .upsert_import_source( + &ImportSource { + workspace_id: workspace_id.clone(), + importer: "OpenAPI".to_string(), + origin: "/tmp/copy.yaml".to_string(), + origin_label: "copy.yaml".to_string(), + ..Default::default() + }, + &UpdateSource::Import, + ) + .expect("create second source"); + db.upsert_import_source_resource(&ImportSourceResource { + import_source_id: other.id, + source_key: "op:root".to_string(), + model_type: "http_request".to_string(), + ..Default::default() + }) + .expect("claim the same key"); + } + + let third = + ImportOrigin { origin: "/tmp/third.yaml".to_string(), label: "third.yaml".to_string() }; + let plan = replan_from(&query_manager, &workspace_id, imported_resources(), third); + assert!( + plan.items.iter().all(|i| i.action == ImportPlanAction::Create), + "an ambiguous link must never guess: {:?}", + plan.items + ); + let warning = plan + .warnings + .iter() + .find(|w| w.title == "Imported as new") + .expect("ambiguity is surfaced"); + assert!(warning.detail.contains("api.yaml"), "{warning:?}"); + assert!(warning.detail.contains("copy.yaml"), "{warning:?}"); + } + + #[test] + fn an_unrecognized_hash_degrades_to_a_conflict() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let committed = first_import(&query_manager); + let workspace_id = committed.workspaces[0].id.clone(); + + { + let db = query_manager.connect(); + let sources = db.list_import_sources(&workspace_id).expect("list import sources"); + let row = db + .list_import_source_resources(&sources[0].id) + .expect("list rows") + .into_iter() + .find(|r| r.source_key == "op:root") + .expect("row for the root request"); + db.upsert_import_source_resource(&ImportSourceResource { + content_hash: Some("v99:from-the-future".to_string()), + ..row + }) + .expect("write an unreadable hash"); + } + + let plan = replan(&query_manager, &workspace_id, imported_resources()); + assert_eq!( + item_by_name(&plan, "Root Request").action, + ImportPlanAction::Unchanged, + "with nothing to compare against, matching sides are still unchanged" + ); + + let mut resources = imported_resources(); + resources.http_requests[0].url = "https://example.com/root-v2".to_string(); + let plan = replan(&query_manager, &workspace_id, resources); + let root = item_by_name(&plan, "Root Request"); + assert_eq!(root.action, ImportPlanAction::Conflict, "a difference can't be attributed"); + assert_eq!(root.resolution, Some(ImportConflictResolution::KeepMine)); + } + + #[test] + fn reordering_the_source_document_changes_nothing() { + let (query_manager, _blob_manager, _rx) = + yaak_models::init_in_memory().expect("initialize database"); + let committed = first_import(&query_manager); + let workspace_id = committed.workspaces[0].id.clone(); + + // Inserting anything ahead of an existing resource renumbers everything after it. + let mut resources = imported_resources(); + for (i, request) in resources.http_requests.iter_mut().enumerate() { + request.sort_priority = (i as f64 + 1.0) * 1000.0; + } + resources.folders[0].sort_priority = 500.0; + + let plan = replan(&query_manager, &workspace_id, resources); + assert!( + plan.items.iter().all(|i| i.action == ImportPlanAction::Unchanged), + "a reorder is not a change: {:?}", + plan.items ); } @@ -2104,7 +2594,9 @@ mod tests { .expect("query import source") .expect("import source recorded"); let rows = db.list_import_source_resources(&source.id).expect("list resource rows"); - assert_eq!(rows.len(), 2, "skipped resources must not advance snapshots: {rows:?}"); + assert_eq!(rows.len(), 4, "every key is remembered, wanted or not: {rows:?}"); + let not_wanted = rows.iter().filter(|r| r.model_id.is_none()).count(); + assert_eq!(not_wanted, 2, "the skipped folder and its request are remembered: {rows:?}"); }