Skip to content

Commit

Permalink
fix: get api host from middleware url (#6680)
Browse files Browse the repository at this point in the history
* Apply suggestions from code review

Co-authored-by: Filip Sobol <filipsobol@users.noreply.github.com>

* changelog update

Co-authored-by: Filip Sobol <filipsobol@users.noreply.github.com>
  • Loading branch information
dawid-ziobro and filipsobol authored Mar 22, 2022
1 parent 9020e17 commit 43b92c9
Show file tree
Hide file tree
Showing 8 changed files with 161 additions and 99 deletions.
81 changes: 0 additions & 81 deletions packages/core/core/__tests__/factories/proxyUtils.spec.ts

This file was deleted.

82 changes: 82 additions & 0 deletions packages/core/core/__tests__/utils/nuxt/proxyUtils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import * as utils from '../../../src/utils/nuxt/_proxyUtils';

describe('[CORE - utils] _proxyUtils', () => {
it('returns proxy for defined api', () => {
const givenApi = {
getProduct: jest.fn()
};

const client = {
post: jest.fn(() => ({ then: jest.fn() }))
};

const proxiedApi = utils.createProxiedApi({ givenApi, client, tag: 'ct' });

proxiedApi.getProduct({ product: 1 });
proxiedApi.getCategory({ category: 1 });

expect(givenApi.getProduct).toBeCalled();
expect(client.post).toBeCalledWith('/ct/getCategory', [{ category: 1 }]);
});

it('reads cookies from incoming request', () => {
expect(utils.getCookies(null)).toEqual('');
expect(utils.getCookies({} as any)).toEqual('');
expect(utils.getCookies({ req: { headers: {} } } as any)).toEqual('');
expect(utils.getCookies({ req: { headers: { cookie: { someCookie: 1 } } } } as any)).toEqual({ someCookie: 1 });
});

it('it combines config with the current one', () => {
jest.spyOn(utils, 'getCookies').mockReturnValue('');

expect(utils.getIntegrationConfig(
{
$config: {
middlewareUrl: 'some-url'
}
} as any,
{ someGivenOption: 1 }
)).toEqual({
axios: {
baseURL: 'some-url',
headers: {}
},
someGivenOption: 1
});
});

it('it combines config with the current one and adds a cookie', () => {
jest.spyOn(utils, 'getCookies').mockReturnValue('xxx');

expect(utils.getIntegrationConfig(
{
$config: {
middlewareUrl: 'some-url'
}
} as any,
{}
)).toEqual({
axios: {
baseURL: 'some-url',
headers: {
cookie: 'xxx'
}
}
});
});

it('it throws error when no middlewareUrl is passed in config', () => {
jest.spyOn(utils, 'getCookies').mockReturnValue('');

expect(() => {
utils.getIntegrationConfig(
{
$config: {
middlewareUrl: ''
}
} as any,
{}
);
}).toThrowError();
});
});
19 changes: 7 additions & 12 deletions packages/core/core/src/utils/nuxt/_proxyUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { IncomingMessage } from 'http';
import { Context as NuxtContext } from '@nuxt/types';
import merge from 'lodash-es/merge';
import { ApiClientMethod } from './../../types';
Expand All @@ -9,16 +8,6 @@ interface CreateProxiedApiParams {
tag: string;
}

export const getBaseUrl = (req: IncomingMessage, basePath: string | undefined = '/'): string => {
if (!req) return `${basePath}api/`;
const { headers } = req;
const isHttps = require('is-https')(req);
const scheme = isHttps ? 'https' : 'http';
const host = headers['x-forwarded-host'] || headers.host;

return `${scheme}://${host}${basePath}api/`;
};

export const createProxiedApi = ({ givenApi, client, tag }: CreateProxiedApiParams) => new Proxy(givenApi, {
get: (target, prop, receiver) => {

Expand All @@ -37,9 +26,15 @@ export const getCookies = (context: NuxtContext) => context?.req?.headers?.cooki

export const getIntegrationConfig = (context: NuxtContext, configuration: any) => {
const cookie = getCookies(context);
const { middlewareUrl } = context.$config;

if (!middlewareUrl) {
throw new Error('Missing configuration option: middlewareUrl');
}

const initialConfig = merge({
axios: {
baseURL: getBaseUrl(context?.req, context?.base),
baseURL: middlewareUrl,
headers: {
...(cookie ? { cookie } : {})
}
Expand Down
6 changes: 0 additions & 6 deletions packages/core/core/src/utils/nuxt/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ export const integrationPlugin = (pluginFn: NuxtPlugin) => (nuxtCtx: NuxtContext
const configure = (tag, configuration) => {
const injectInContext = createAddIntegrationToCtx({ tag, nuxtCtx, inject });
const config = getIntegrationConfig(nuxtCtx, configuration);
const { middlewareUrl, ssrMiddlewareUrl } = (nuxtCtx as any).$config;

if (middlewareUrl) {
config.axios.baseURL = process.server ? ssrMiddlewareUrl || middlewareUrl : middlewareUrl;
}

const client = axios.create(config.axios);
const api = createProxiedApi({ givenApi: configuration.api || {}, client, tag });

Expand Down
15 changes: 15 additions & 0 deletions packages/core/docs/changelog/6680.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module.exports = {
description: 'get api host from middleware url',
link: 'https://github.com/vuestorefront/vue-storefront/pull/6680',
isBreaking: true,
breakingChanges: [
{
module: '@vue-storefront/core',
before: 'The `middlewareUrl` property was optional',
after: 'The `middlewareUrl` is required',
comment: 'The `middlewareUrl` property in the `nuxt.config.js` file is now required. Please follow the instruction in the Migration Guide.'
}
],
author: 'Dawid Ziobro',
linkToGitHubAccount: 'https://github.com/dawid-ziobro'
};
15 changes: 15 additions & 0 deletions packages/core/docs/getting-started/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,21 @@ You can learn more about this file and available configuration options on the [N

The `middleware.config.js` file is as essential as `nuxt.confis.js`, but much simpler and likely smaller. It configures the Server Middleware used for communication with e-commerce platforms and contains sensitive credentials, custom endpoints, queries, etc.

What is typical for every integration is the `middlewareUrl` property which defines the URL to the Server Middleware. Usually, it's your application domain followed by the `/api` path.

Example:

```javascript
// nuxt.config.js
export default {
publicRuntimeConfig: {
middlewareUrl: 'https://yourdomain.com/api/'
}
}
```

For the local development, set it to `http://localhost:3000/api/`.

You can learn more about Server Middleware and available configuration options on the [Server Middleware](/architecture/server-middleware.html) page.

## Optional configuration files
Expand Down
39 changes: 39 additions & 0 deletions packages/core/docs/reference/migrate/2.5.7/overview.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Migrating projects to 2.5.7

## Update `nuxt.config.js`

In this release, we made the `middlewareUrl` property required for security reasons. Open the `nuxt.config.js` file and add the `middlewareUrl` property like shown below:

```javascript
// nuxt.config.js
export default {
publicRuntimeConfig: {
middlewareUrl: 'https://yourdomain.com/api/' // For the local development, set it to `http://localhost:3000/api/`.
}
}
```

:::warning
Make sure to pass whole url with protocol and/or port and suffix it with `/api/`.
:::

If you don't want to hardcode the URL in the configuration file, you can use environmental variables.

Example:

```javascript
// nuxt.config.js
export default {
publicRuntimeConfig: {
middlewareUrl: process.env.API_BASE_URL
}
}
```

Then add an entry in the `.env` file or use any other method for passing environmental variables that suits your needs.

Example:
```
// .env
API_BASE_URL=https://yourdomain.com/api/
```
3 changes: 3 additions & 0 deletions packages/core/docs/reference/migrate/index.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Migration guides

## 2.5.7
- [Overview](./2.5.7/overview.md)

## 2.5.0
- [Overview](./2.5.0/overview.md)

Expand Down

0 comments on commit 43b92c9

Please sign in to comment.