[TT-347] Proxy non-draft endpoints to get real response #335

medric · 2019-07-15T07:21:37Z

This introduces a new module used within the main server middleware to proxy requests that expect real response as opposed to mock ones. The proxy will be leveraged when an endpoint is not in a draft state.

To specify the url of the server the proxy needs to hit, users can set a --proxyBaseUrl flag when starting the mock server.

fwouts

Nice one. Unfortunately it looks like the mock server doesn't have test coverage yet (my bad). Would you mind adding some? We need to ensure that it correctly mocks draft endpoints and proxies the others unless there is no proxy URL.

fwouts · 2019-07-15T23:18:59Z

lib/src/mockserver/proxy.ts

+  pathPrefix: string;
+  proxyBaseUrl: string;
+}) {
+  const [protocol] = proxyBaseUrl.split("://");


Make sure to fail with a helpful message if we forgot to specify the protocol? Or fallback to http:// maybe?

fwouts · 2019-07-15T23:20:23Z

lib/src/mockserver/server.ts

+
+        // non-draft end points get real response
+        if (shouldProxy && proxyBaseUrl) {
+          proxyRequest({


For conciseness, consider:

Suggested change

proxyRequest({

return proxyRequest({

fwouts · 2019-07-15T23:21:28Z

cli/src/commands/mock.ts

@@ -23,6 +23,10 @@ export default class Mock extends Command {

  static flags = {
    help: flags.help({ char: "h" }),
+    proxyBaseUrl: flags.string({
+      description:
+        "If set, the server would act as a proxy and fetch data from the given remote server instead of mocking it"


Suggested change

"If set, the server would act as a proxy and fetch data from the given remote server instead of mocking it"

"If set, the server will act as a proxy and fetch data from the given remote server for all non-draft endpoints instead of mocking them"

fwouts · 2019-07-15T23:21:41Z

README.md

+  -h, --help                   show CLI help
+  -p, --port=port              (required) [default: 3010] Port on which to run the mock server
+  --pathPrefix=pathPrefix      Prefix to prepend to each endpoint path
+  --proxyBaseUrl=proxyBaseUrl  If set, the server would act as a proxy and fetch data from the given remote server


I wonder, should we only update the README.md once we release a new version?

fwouts · 2019-07-19T02:09:29Z

cli/src/common/infer-protocol.ts

+
+  if (protocol !== "http" && protocol !== "https") {
+    throw new Error(
+      'Err - could not infer protocol from proxy base url, should be either "http" or "https".'


I don't think Err - is necessary since it should be fairly clear in the stack trace that it's an error already?

Suggested change

'Err - could not infer protocol from proxy base url, should be either "http" or "https".'

'Could not infer protocol from proxy base url, should be either "http" or "https".'

lib/src/mockserver/server.spec.ts

fwouts · 2019-07-19T03:31:20Z

cli/src/common/infer-proxy-config.ts

@@ -0,0 +1,20 @@
+import { ProxyConfig } from '../../../lib/src/mockserver/server';
+
+export default function inferProxyConfig(proxyBaseUrl: string): ProxyConfig | void {


Suggested change

export default function inferProxyConfig(proxyBaseUrl: string): ProxyConfig | void {

export default function inferProxyConfig(proxyBaseUrl: string): ProxyConfig | null {

fwouts · 2019-07-19T03:32:10Z

cli/src/common/infer-proxy-config.ts

+
+export default function inferProxyConfig(proxyBaseUrl: string): ProxyConfig | void {
+  if (!proxyBaseUrl) {
+    return undefined;


I think null better represents the concept of "absence of a value" rather than undefined. Thoughts?

Suggested change

return undefined;

return null;

fwouts · 2019-07-19T03:32:43Z

cli/src/common/infer-proxy-config.ts

+
+  if (protocol !== "http" && protocol !== "https") {
+    throw new Error(
+      'Err - could not infer protocol from proxy base url, should be either "http" or "https".'


Please address #335 (comment)

- Clean up - Address PR comments - Write headers to proxied response - Add test coverage for mockserver

medric requested review from fwouts and lfportal July 15, 2019 07:21

medric force-pushed the TT-347-proxy-non-draft-endpoints branch from 91875fe to 1949cb2 Compare July 15, 2019 07:23

fwouts suggested changes Jul 15, 2019

View reviewed changes

lfportal approved these changes Jul 18, 2019

View reviewed changes

medric force-pushed the TT-347-proxy-non-draft-endpoints branch from daaf26a to 57f7051 Compare July 18, 2019 00:27

fwouts suggested changes Jul 19, 2019

View reviewed changes

medric force-pushed the TT-347-proxy-non-draft-endpoints branch from d4c9f85 to 9f9c44b Compare July 19, 2019 03:23

fwouts approved these changes Jul 19, 2019

View reviewed changes

medric added 10 commits July 19, 2019 13:51

Add logic to proxy request to given input server

018721d

Comment & remove line

8539a7c

Fix linting error

c6755f1

[TT-347]

2906ad5

- Clean up - Address PR comments - Write headers to proxied response - Add test coverage for mockserver

Updgrade nock to address nock/nock#1227

c3b5308

Use proxy config instead of inferring from base url

f419596

Add test to ensure mock data is returned when expected

79f3e87

Commit file

48d0809

Address comments

ee83b5d

Address linting errors

4bb3223

medric force-pushed the TT-347-proxy-non-draft-endpoints branch from d495881 to 4bb3223 Compare July 19, 2019 04:08

Add missing dependencie : supertest

02567ed

medric force-pushed the TT-347-proxy-non-draft-endpoints branch from 66e023c to 02567ed Compare July 19, 2019 04:15

Merge branch 'master' into TT-347-proxy-non-draft-endpoints

8c5214b

medric merged commit d18d752 into master Jul 19, 2019

medric deleted the TT-347-proxy-non-draft-endpoints branch July 19, 2019 04:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TT-347] Proxy non-draft endpoints to get real response #335

[TT-347] Proxy non-draft endpoints to get real response #335

medric commented Jul 15, 2019

fwouts left a comment •

edited

fwouts Jul 15, 2019

fwouts Jul 15, 2019

fwouts Jul 15, 2019

fwouts Jul 15, 2019

fwouts Jul 19, 2019

fwouts Jul 19, 2019

fwouts Jul 19, 2019

fwouts Jul 19, 2019

	"If set, the server would act as a proxy and fetch data from the given remote server instead of mocking it"
	"If set, the server will act as a proxy and fetch data from the given remote server for all non-draft endpoints instead of mocking them"

	'Err - could not infer protocol from proxy base url, should be either "http" or "https".'
	'Could not infer protocol from proxy base url, should be either "http" or "https".'

		@@ -0,0 +1,20 @@
		import { ProxyConfig } from '../../../lib/src/mockserver/server';

		export default function inferProxyConfig(proxyBaseUrl: string): ProxyConfig \| void {

[TT-347] Proxy non-draft endpoints to get real response #335

[TT-347] Proxy non-draft endpoints to get real response #335

Conversation

medric commented Jul 15, 2019

fwouts left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fwouts left a comment •

edited