Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎉 🪟 [Connector Builder] Resolve manifest before converting to builder form values #21898

Merged
merged 4 commits into from
Jan 27, 2023
Merged

🎉 🪟 [Connector Builder] Resolve manifest before converting to builder form values #21898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6a940d6
call /manifest/resolve before converting to builder form values
lmossman Jan 26, 2023
d400fcc
Merge branch 'master' into lmossman/resolve-refs-before-convert
lmossman Jan 27, 2023
d579c34
fix test
lmossman Jan 27, 2023
6259872
Merge branch 'master' into lmossman/resolve-refs-before-convert
Jan 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions airbyte-webapp/src/components/connectorBuilder/YamlEditor/YamlEditor.tsx
View file
Open in desktop
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
Open in desktop
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 () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of small async/await changes in this file, since convertToBuilderFormValues is now an async function

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