Skip to content

Commit

Permalink
🎉 🪟 [Connector Builder] Resolve manifest before converting to builder…
Browse files Browse the repository at this point in the history 
… form values (#21898)

* call /manifest/resolve before converting to builder form values

* fix test

---------

Co-authored-by: Joe Reuter <joe@airbyte.io>
  • Loading branch information
@lmossman
lmossman and Joe Reuter committed Jan 27, 2023
1 parent 00a1de5 commit 9583fc1
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 140 deletions.
14 changes: 6 additions & 8 deletions airbyte-webapp/src/components/connectorBuilder/YamlEditor/YamlEditor.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@ import { Action, Namespace } from "core/analytics";
import { ConnectorManifest } from "core/request/ConnectorManifest";
import { useAnalyticsService } from "hooks/services/Analytics";
import { useConfirmationModalService } from "hooks/services/ConfirmationModal";
import {
useConnectorBuilderFormState,
useConnectorBuilderTestState,
} from "services/connectorBuilder/ConnectorBuilderStateService";
import { useConnectorBuilderFormState } from "services/connectorBuilder/ConnectorBuilderStateService";

import styles from "./YamlEditor.module.scss";
import { UiYamlToggleButton } from "../Builder/UiYamlToggleButton";
import { DownloadYamlButton } from "../DownloadYamlButton";
import { convertToBuilderFormValues } from "../manifestToBuilderForm";
import { convertToManifest } from "../types";
import { useManifestToBuilderForm } from "../useManifestToBuilderForm";

interface YamlEditorProps {
toggleYamlEditor: () => void;
Expand All @@ -41,8 +38,8 @@ export const YamlEditor: React.FC<YamlEditorProps> = ({ toggleYamlEditor }) => {
setYamlIsValid,
setJsonManifest,
} = useConnectorBuilderFormState();
const { streamListErrorMessage } = useConnectorBuilderTestState();
const [yamlValue, setYamlValue] = useState(yamlManifest);
const { convertToBuilderFormValues } = useManifestToBuilderForm();

// debounce the setJsonManifest calls so that it doesnt result in a network call for every keystroke
const debouncedSetJsonManifest = useMemo(() => debounce(setJsonManifest, 200), [setJsonManifest]);
Expand Down Expand Up @@ -90,10 +87,11 @@ export const YamlEditor: React.FC<YamlEditorProps> = ({ toggleYamlEditor }) => {
return !isEqual(convertToManifest(builderFormValues), jsonManifest);
}, [jsonManifest, builderFormValues]);

const handleToggleYamlEditor = () => {
const handleToggleYamlEditor = async () => {
if (yamlIsDirty) {
try {
setValues(convertToBuilderFormValues(jsonManifest, builderFormValues, streamListErrorMessage));
const convertedFormValues = await convertToBuilderFormValues(jsonManifest, builderFormValues);
setValues(convertedFormValues);
toggleYamlEditor();
} catch (e) {
openConfirmationModal({
Expand Down
166 changes: 48 additions & 118 deletions ...ctorBuilder/manifestToBuilderForm.test.ts → ...rBuilder/useManifestToBuilderForm.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
import merge from "lodash/merge";

import {
ConnectorManifest,
DeclarativeStream,
DeclarativeStreamRetriever,
HttpRequester,
} from "core/request/ConnectorManifest";
import { ConnectorManifest, DeclarativeStream } from "core/request/ConnectorManifest";

import { convertToBuilderFormValues } from "./manifestToBuilderForm";
import { DEFAULT_BUILDER_FORM_VALUES } from "./types";
import { convertToBuilderFormValues } from "./useManifestToBuilderForm";

const baseManifest: ConnectorManifest = {
type: "DeclarativeSource",
Expand Down Expand Up @@ -53,89 +48,24 @@ const stream2: DeclarativeStream = merge({}, stream1, {
},
});

describe("Conversion throws error when", () => {
it("streamListErrorMessage is present", () => {
const streamListErrorMessage = "Some error message";
const convert = () => {
convertToBuilderFormValues({} as ConnectorManifest, DEFAULT_BUILDER_FORM_VALUES, streamListErrorMessage);
};
expect(convert).toThrow(streamListErrorMessage);
});
const noOpResolve = (manifest: ConnectorManifest) => {
return Promise.resolve({ manifest });
};

it("manifest contains refs", () => {
const convert = () => {
const manifest: ConnectorManifest = {
...baseManifest,
definitions: {
retriever: {
name: "pokemon",
primary_key: "id",
requester: {
name: "pokemon",
path: "/pokemon/{{config['pokemon_name']}}",
url_base: "https://pokeapi.co/api/v2",
http_method: "GET",
},
record_selector: {
extractor: {
type: "DpathExtractor",
field_pointer: [],
},
},
},
},
streams: [
{
type: "DeclarativeStream",
name: "pokemon",
retriever: {
$ref: "*ref(definitions.retriever)",
} as unknown as DeclarativeStreamRetriever,
},
],
};
convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
describe("Conversion throws error when", () => {
it("resolve throws error", async () => {
const errorMessage = "Some resolve error message";
const resolve = (_manifest: ConnectorManifest) => {
throw new Error(errorMessage);
};
expect(convert).toThrow("contains refs");
});

it("manifest contains options", () => {
const convert = () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
{
type: "DeclarativeStream",
$options: {
name: "pokemon",
primary_key: "id",
path: "/pokemon/{{config['pokemon_name']}}",
},
retriever: {
type: "SimpleRetriever",
requester: {
type: "HttpRequester",
url_base: "https://pokeapi.co/api/v2",
http_method: "GET",
} as unknown as HttpRequester,
record_selector: {
type: "RecordSelector",
extractor: {
type: "DpathExtractor",
field_pointer: [],
},
},
},
},
],
};
convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const convert = async () => {
return convertToBuilderFormValues(resolve, {} as ConnectorManifest, DEFAULT_BUILDER_FORM_VALUES);
};
expect(convert).toThrow("contains refs");
await expect(convert).rejects.toThrow(errorMessage);
});

it("manifests has incorrect retriever type", () => {
const convert = () => {
it("manifests has incorrect retriever type", async () => {
const convert = async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand All @@ -149,13 +79,13 @@ describe("Conversion throws error when", () => {
},
],
};
convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
return convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
};
expect(convert).toThrow("doesn't use a SimpleRetriever");
await expect(convert).rejects.toThrow("doesn't use a SimpleRetriever");
});

it("manifest has incorrect requester type", () => {
const convert = () => {
it("manifest has incorrect requester type", async () => {
const convert = async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand All @@ -169,13 +99,13 @@ describe("Conversion throws error when", () => {
}),
],
};
convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
return convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
};
expect(convert).toThrow("doesn't use a HttpRequester");
await expect(convert).rejects.toThrow("doesn't use a HttpRequester");
});

it("manifest has an authenticator with a non-interpolated secret key", () => {
const convert = () => {
it("manifest has an authenticator with a non-interpolated secret key", async () => {
const convert = async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand All @@ -192,12 +122,12 @@ describe("Conversion throws error when", () => {
}),
],
};
convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
return convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
};
expect(convert).toThrow("api_token value must be of the form {{ config[");
await expect(convert).rejects.toThrow("api_token value must be of the form {{ config[");
});

it("manifest has an OAuthAuthenticator with a refresh_request_body containing non-string values", () => {
it("manifest has an OAuthAuthenticator with a refresh_request_body containing non-string values", async () => {
const convert = () => {
const manifest: ConnectorManifest = {
...baseManifest,
Expand Down Expand Up @@ -225,19 +155,19 @@ describe("Conversion throws error when", () => {
}),
],
};
convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
return convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
};
expect(convert).toThrow("OAuthAuthenticator contains a refresh_request_body with non-string values");
await expect(convert).rejects.toThrow("OAuthAuthenticator contains a refresh_request_body with non-string values");
});
});

describe("Conversion successfully results in", () => {
it("default values if manifest is empty", () => {
const formValues = convertToBuilderFormValues(baseManifest, DEFAULT_BUILDER_FORM_VALUES);
it("default values if manifest is empty", async () => {
const formValues = await convertToBuilderFormValues(noOpResolve, baseManifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues).toEqual(DEFAULT_BUILDER_FORM_VALUES);
});

it("spec properties converted to inputs if no streams present", () => {
it("spec properties converted to inputs if no streams present", async () => {
const manifest: ConnectorManifest = {
...baseManifest,
spec: {
Expand All @@ -256,7 +186,7 @@ describe("Conversion successfully results in", () => {
},
},
};
const formValues = convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const formValues = await convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues.inferredInputOverrides).toEqual({});
expect(formValues.inputs).toEqual([
{
Expand All @@ -267,7 +197,7 @@ describe("Conversion successfully results in", () => {
]);
});

it("spec properties converted to input overrides on matching auth keys", () => {
it("spec properties converted to input overrides on matching auth keys", async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand Down Expand Up @@ -303,7 +233,7 @@ describe("Conversion successfully results in", () => {
},
},
};
const formValues = convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const formValues = await convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues.inputs).toEqual([
{
key: "numeric_key",
Expand All @@ -316,7 +246,7 @@ describe("Conversion successfully results in", () => {
});
});

it("request options converted to key-value list", () => {
it("request options converted to key-value list", async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand All @@ -335,14 +265,14 @@ describe("Conversion successfully results in", () => {
}),
],
};
const formValues = convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const formValues = await convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues.streams[0].requestOptions.requestParameters).toEqual([
["k1", "v1"],
["k2", "v2"],
]);
});

it("primary key string converted to array", () => {
it("primary key string converted to array", async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand All @@ -354,11 +284,11 @@ describe("Conversion successfully results in", () => {
}),
],
};
const formValues = convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const formValues = await convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues.streams[0].primaryKey).toEqual(["id"]);
});

it("cartesian product stream slicer converted to builder cartesian product slicer", () => {
it("cartesian product stream slicer converted to builder cartesian product slicer", async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand Down Expand Up @@ -387,11 +317,11 @@ describe("Conversion successfully results in", () => {
}),
],
};
const formValues = convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const formValues = await convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues.streams[0].streamSlicer).toEqual(manifest.streams[0].retriever.stream_slicer);
});

it("substream stream slicer converted to builder substream slicer", () => {
it("substream stream slicer converted to builder substream slicer", async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand All @@ -413,7 +343,7 @@ describe("Conversion successfully results in", () => {
}),
],
};
const formValues = convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const formValues = await convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues.streams[1].streamSlicer).toEqual({
type: "SubstreamSlicer",
parent_key: "key",
Expand All @@ -422,7 +352,7 @@ describe("Conversion successfully results in", () => {
});
});

it("schema loader converted to schema", () => {
it("schema loader converted to schema", async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand All @@ -436,15 +366,15 @@ describe("Conversion successfully results in", () => {
}),
],
};
const formValues = convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const formValues = await convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues.streams[0].schema).toEqual(
`{
"key": "value"
}`
);
});

it("stores unsupported fields", () => {
it("stores unsupported fields", async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand Down Expand Up @@ -473,7 +403,7 @@ describe("Conversion successfully results in", () => {
}),
],
};
const formValues = convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const formValues = await convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues.streams[0].unsupportedFields).toEqual({
transformations: manifest.streams[0].transformations,
checkpoint_interval: manifest.streams[0].checkpoint_interval,
Expand All @@ -488,7 +418,7 @@ describe("Conversion successfully results in", () => {
});
});

it("OAuth authenticator refresh_request_body converted to array", () => {
it("OAuth authenticator refresh_request_body converted to array", async () => {
const manifest: ConnectorManifest = {
...baseManifest,
streams: [
Expand All @@ -512,7 +442,7 @@ describe("Conversion successfully results in", () => {
}),
],
};
const formValues = convertToBuilderFormValues(manifest, DEFAULT_BUILDER_FORM_VALUES);
const formValues = await convertToBuilderFormValues(noOpResolve, manifest, DEFAULT_BUILDER_FORM_VALUES);
expect(formValues.global.authenticator).toEqual({
type: "OAuthAuthenticator",
client_id: "{{ config['client_id'] }}",
Expand Down
Loading

0 comments on commit 9583fc1

Please sign in to comment.