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.

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

feat[core][Builder]: ENG-6337 replace query api with content api #3529

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
150 changes: 149 additions & 1 deletion packages/core/src/builder.class.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Builder } from './builder.class';
import { Builder, GetContentOptions } from './builder.class';

describe('Builder', () => {
test('trustedHosts', () => {
Expand Down Expand Up @@ -167,3 +167,151 @@ describe('prepareComponentSpecToSend', () => {
expect(result.inputs?.[1]).toEqual(input.inputs[1]);
});
});

describe('flushGetContentQueue', () => {
const API_KEY = '25608a566fbb654ea959c1b1729e370d';
const MODEL = 'page';
const AUTH_TOKEN = '82202e99f9fb4ed1da5940f7fa191e72';
const KEY = 'my-key';
const OMIT = 'data.blocks';

let builder: Builder;

beforeEach(() => {
builder = new Builder(API_KEY, undefined, undefined, false, AUTH_TOKEN, 'v3');
builder['makeCodegenOrContentApiCall'] = jest.fn(() => {
return Promise.resolve({
json: Promise.resolve(),
});
});
});

test('throws error if apiKey is not defined', () => {
builder = new Builder();
try {
builder['flushGetContentQueue']();
} catch (err: any) {
expect(err.message).toBe(
'Fetching content failed, expected apiKey to be defined instead got: null'
);
}
});

test('throws error if apiVersion is not v3', () => {
builder = new Builder(API_KEY, undefined, undefined, false, AUTH_TOKEN, 'v1');

try {
builder['flushGetContentQueue']();
} catch (err: any) {
expect(err.message).toBe("Invalid apiVersion: 'v3', received 'v1'");
}
});

test("hits codegen url when format is 'solid'", async () => {
const expectedFormat = 'solid';

const result = await builder['flushGetContentQueue'](true, [
{
model: MODEL,
format: expectedFormat,
key: KEY,
userAttributes: { respectScheduling: true },
omit: OMIT,
fields: 'data',
},
]);

expect(builder['makeCodegenOrContentApiCall']).toBeCalledTimes(1);
expect(builder['makeCodegenOrContentApiCall']).toBeCalledWith(
`https://cdn.builder.io/api/v3/codegen/${API_KEY}/${KEY}?omit=${OMIT}&apiKey=${API_KEY}&fields=data&format=solid&userAttributes=%7B%22respectScheduling%22%3Atrue%7D&options.${KEY}.model=%22${MODEL}%22`,
{ headers: { Authorization: `Bearer ${AUTH_TOKEN}` } }
);
});

test("hits codegen url when format is 'react'", async () => {
const expectedFormat = 'react';

const result = await builder['flushGetContentQueue'](true, [
{
model: MODEL,
format: expectedFormat,
key: KEY,
userAttributes: { respectScheduling: true },
omit: OMIT,
fields: 'data',
},
]);

expect(builder['makeCodegenOrContentApiCall']).toBeCalledTimes(1);
expect(builder['makeCodegenOrContentApiCall']).toBeCalledWith(
`https://cdn.builder.io/api/v3/codegen/${API_KEY}/${KEY}?omit=${OMIT}&apiKey=${API_KEY}&fields=data&format=react&userAttributes=%7B%22respectScheduling%22%3Atrue%7D&options.${KEY}.model=%22${MODEL}%22`,
{ headers: { Authorization: `Bearer ${AUTH_TOKEN}` } }
);
});

test("hits content url when format is neither 'html'", async () => {
const expectedFormat = 'html';

const result = await builder['flushGetContentQueue'](true, [
{
model: MODEL,
format: expectedFormat,
key: KEY,
userAttributes: { respectScheduling: true },
omit: OMIT,
fields: 'data',
limit: 10,
},
]);

expect(builder['makeCodegenOrContentApiCall']).toBeCalledTimes(1);
expect(builder['makeCodegenOrContentApiCall']).toBeCalledWith(
`https://cdn.builder.io/api/v3/content/${MODEL}?omit=data.blocks&apiKey=${API_KEY}&fields=data&format=html&userAttributes=%7B%22respectScheduling%22%3Atrue%7D&limit=10&model=%22${MODEL}%22`,
{ headers: { Authorization: `Bearer ${AUTH_TOKEN}` } }
);
});

test("hits content url when format is neither 'amp'", async () => {
const expectedFormat = 'amp';

const result = await builder['flushGetContentQueue'](true, [
{
model: MODEL,
format: expectedFormat,
key: KEY,
userAttributes: { respectScheduling: true },
omit: OMIT,
fields: 'data',
limit: 10,
},
]);

expect(builder['makeCodegenOrContentApiCall']).toBeCalledTimes(1);
expect(builder['makeCodegenOrContentApiCall']).toBeCalledWith(
`https://cdn.builder.io/api/v3/content/${MODEL}?omit=data.blocks&apiKey=${API_KEY}&fields=data&format=amp&userAttributes=%7B%22respectScheduling%22%3Atrue%7D&limit=10&model=%22${MODEL}%22`,
{ headers: { Authorization: `Bearer ${AUTH_TOKEN}` } }
);
});

test("hits content url when format is neither 'email'", async () => {
const expectedFormat = 'email';

const result = await builder['flushGetContentQueue'](true, [
{
model: MODEL,
format: expectedFormat,
key: KEY,
userAttributes: { respectScheduling: true },
omit: OMIT,
fields: 'data',
limit: 10,
},
]);

expect(builder['makeCodegenOrContentApiCall']).toBeCalledTimes(1);
expect(builder['makeCodegenOrContentApiCall']).toBeCalledWith(
`https://cdn.builder.io/api/v3/content/${MODEL}?omit=data.blocks&apiKey=${API_KEY}&fields=data&format=email&userAttributes=%7B%22respectScheduling%22%3Atrue%7D&limit=10&model=%22${MODEL}%22`,
{ headers: { Authorization: `Bearer ${AUTH_TOKEN}` } }
);
});
});
57 changes: 35 additions & 22 deletions packages/core/src/builder.class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,12 @@ type AllowEnrich =
| { apiVersion?: never; enrich?: boolean };

export type GetContentOptions = AllowEnrich & {
/** The model to get content for (required) */
model?: string;

/** Your public API key (required) */
apiKey?: string;

/**
* User attribute key value pairs to be used for targeting
* https://www.builder.io/c/docs/custom-targeting-attributes
Expand All @@ -286,10 +292,11 @@ export type GetContentOptions = AllowEnrich & {
* userAttributes: {
* urlPath: '/',
* returnVisitor: true,
* device: 'mobile'
clyde-builderio marked this conversation as resolved.
Show resolved Hide resolved
* }
* ```
*/
userAttributes?: UserAttributes;
userAttributes?: (Record<string, any> & { urlPath?: string }) | null;
/**
* Alias for userAttributes.urlPath except it can handle a full URL (optionally with host,
* protocol, query, etc) and we will parse out the path.
Expand Down Expand Up @@ -350,7 +357,7 @@ export type GetContentOptions = AllowEnrich & {
*
* @see {@link https://docs.mongodb.com/manual/reference/operator/query/}
*/
query?: any;
query?: Record<string, any>;
clyde-builderio marked this conversation as resolved.
Show resolved Hide resolved
/**
* Bust through all caches. Not recommended for production (for performance),
* but can be useful for development and static builds (so the static site has
Expand All @@ -373,17 +380,13 @@ export type GetContentOptions = AllowEnrich & {
* `BuilderContent` to render instead of fetching.
*/
initialContent?: any;
/**
* The name of the model to fetch content for.
*/
model?: string;
/**
* Set to `false` to not cache responses when running on the client.
*/
cache?: boolean;

/**
* Set to the current locale in your application if you want localized inputs to be auto-resolved, should match one of the locales keys in your space settings
* Learn more about adding or removing locales [here](https://www.builder.io/c/docs/add-remove-locales)
* If provided, the API will auto-resolve localized objects to the value of this `locale` key.
clyde-builderio marked this conversation as resolved.
Show resolved Hide resolved
*/
locale?: string;
/**
Expand Down Expand Up @@ -2342,6 +2345,10 @@ export class Builder {
}
}

private makeCodegenOrContentApiCall(url: string, requestOptions: any): Promise<any> {
return getFetch()(url, requestOptions);
}

private flushGetContentQueue(usePastQueue = false, useQueue?: GetContentOptions[]) {
if (!this.apiKey) {
throw new Error(
Expand All @@ -2350,8 +2357,8 @@ export class Builder {
}

if (this.apiVersion) {
if (!['v1', 'v3'].includes(this.apiVersion)) {
throw new Error(`Invalid apiVersion: expected 'v1' or 'v3', received '${this.apiVersion}'`);
if (this.apiVersion !== 'v3') {
throw new Error(`Invalid apiVersion: 'v3', received '${this.apiVersion}'`);
clyde-builderio marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
this.apiVersion = DEFAULT_API_VERSION;
Expand Down Expand Up @@ -2457,8 +2464,10 @@ export class Builder {
}

for (const options of queue) {
if (options.format) {
queryParams.format = options.format;
const format = options.format;
const areOptionsForCodegen = format === 'solid' || format === 'react';
if (format) {
queryParams.format = format;
}
// TODO: remove me and make permodel
if (options.static) {
Expand Down Expand Up @@ -2492,9 +2501,13 @@ export class Builder {
for (const key of properties) {
const value = options[key];
if (value !== undefined) {
queryParams.options = queryParams.options || {};
queryParams.options[options.key!] = queryParams.options[options.key!] || {};
queryParams.options[options.key!][key] = JSON.stringify(value);
if (areOptionsForCodegen) {
queryParams.options = queryParams.options || {};
queryParams.options[options.key!] = queryParams.options[options.key!] || {};
queryParams.options[options.key!][key] = JSON.stringify(value);
} else {
queryParams[key] = JSON.stringify(value);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you make this change? why does that logic only apply to codegen calls?

Copy link
Author

@clyde-builderio clyde-builderio Sep 5, 2024

Choose a reason for hiding this comment

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

In this thread (https://builder-internal.slack.com/archives/C01V9EH4T1N/p1725389926830789) Steve said says

yeah query string should be reusable, just note that there might be stuff in the old SDK to support multiple fetches in one response, so there may be causes where we are doing things like ?model.page.limit=10 that should just be ?limit=10 for /content
iirc they both go through the same code paths so even the old format may technically work in /content but cleaner to remove the model.{modelName}.key=value stuff

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, I see. For the sake of backwards compatibility, let's keep these options for both codegen and query endpoints, and remove them only if we're using the new content endpoint

}
}
}
Expand Down Expand Up @@ -2525,15 +2538,15 @@ export class Builder {
};
}

const fn = format === 'solid' || format === 'react' ? 'codegen' : 'query';
const fn = format === 'solid' || format === 'react' ? 'codegen' : 'content';
let url =
fn === 'codegen'
? `${host}/api/v3/${fn}/${this.apiKey}/${keyNames}`
: `https://cdn.builder.io/api/v3/${fn}/${queue[0].model}`;
clyde-builderio marked this conversation as resolved.
Show resolved Hide resolved

// NOTE: this is a hack to get around the fact that the codegen endpoint is not yet available in v3
const apiVersionBasedOnFn = fn === 'query' ? this.apiVersion : 'v1';
const url =
`${host}/api/${apiVersionBasedOnFn}/${fn}/${this.apiKey}/${keyNames}` +
(queryParams && hasParams ? `?${queryStr}` : '');
url = url + (queryParams && hasParams ? `?${queryStr}` : '');

const promise = getFetch()(url, requestOptions)
const promise = this.makeCodegenOrContentApiCall(url, requestOptions)
.then(res => res.json())
.then(
result => {
Expand Down
Loading