fix(repository-json-schema): update to pass openapi validation in azure #3504

ericzon · 2019-08-04T14:19:12Z

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

This PR is related with my comment in #1466 about problems found during the process of deploy an API in Azure.
With this PR is possible to deploy an API as "App Service" and connect it with the API Manager as OpenAPI.
I added the optional key 'itExtendsCompatibility' (boolean) in JsonSchemaOptions to tackle these cases. By default uses the current behaviour.
Added some tests, applied prettier & linter of the project.

P.S: by the way, trying to sign your CLA gives a 504 Error in Cloudfront.

packages/repository-json-schema/src/build-schema.ts

raymondfeng · 2019-08-05T21:35:23Z

@ericzon Thank you for the patch. I had a similar concern in the past - see #3309 (comment).

dhmlau · 2019-08-06T15:20:32Z

FYI - the failure on Travis is a timeout error:

1) cloneExampleFromGitHub (SLOW)
       extracts project files:
     Error: Timeout of 20000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/travis/build/strongloop/loopback-next/packages/cli/test/integration/generators/clone-example.integration.js)
      at listOnTimeout (internal/timers.js:531:17)
      at processTimers (internal/timers.js:475:7)

dhmlau · 2019-08-06T15:21:36Z

@slnode test please

raymondfeng · 2019-08-07T16:48:11Z

@nabdelgadir Please review the PR.

nabdelgadir

Mostly LGTM, just a couple comments.

packages/repository-json-schema/src/__tests__/unit/build-schema.unit.ts

packages/repository-json-schema/src/build-schema.ts

dhmlau · 2019-08-13T17:36:16Z

@nabdelgadir, is this good to land?

nabdelgadir · 2019-08-13T18:23:01Z

@dhmlau not yet.

@ericzon, what do you think about @raymondfeng's proposal?

Make schema title compatible by default with a bit less expressivity (IIRC, the title needs to be unique)

Populate schema description to make it readable

dhmlau · 2019-08-14T17:29:57Z

Thanks @nabdelgadir . Let me assign it to you since you're already helping this PR.

ericzon · 2019-08-15T20:54:37Z

@dhmlau not yet.

@ericzon, what do you think about @raymondfeng's proposal?

Make schema title compatible by default with a bit less expressivity (IIRC, the title needs to be unique)

Populate schema description to make it readable

@nabdelgadir for me it's ok, compatible by default. @raymondfeng in 2 you propose to add some kind of extra & dynamic description like:

"description": "[original description]... Product excluding fields id, name"

right?

bajtos

Thank you @ericzon for the pull request! I agree we should emit schema titles that are compatible with a wide range of OpenAPI/JSON Schema consumers 👍

I disagree with the proposed implementation, let's find a better one.

packages/repository-json-schema/src/build-schema.ts

raymondfeng · 2019-08-20T20:13:10Z

@ericzon Would you please make the final update based on the team agreement? Thanks!

ericzon · 2019-08-21T22:51:07Z

Sure! I'm on that, I'll update the PR with the agreements.

ericzon · 2019-08-24T10:27:45Z

Updated. Now is compatible by default & description includes information about model options.

Example of model names & descriptions:

"TechnicalData"
"Description about TechnicalData model"

"TechnicalDataExcluding_id-createdAt_"
"Description about TechnicalData model (Model options: Excluding[id,createdAt])"

"TechnicalDataPartial"
"Description about TechnicalData model (Model options: Partial)"

"TechnicalDataWithRelations"
"Description about TechnicalData model (Model options: WithRelations)"

I tested in Azure to import the generated definition and is accepted.
...

packages/repository-json-schema/src/__tests__/integration/build-schema.integration.ts

packages/repository-json-schema/src/build-schema.ts

The major issues have been addressed.

bajtos · 2019-09-10T16:37:30Z

@ericzon I would love to see this pull request landed soon, let me know how can I help.

raymondfeng · 2019-09-12T20:35:17Z

@ericzon I have rebased your branch against latest master and squashed all commits into one.

bajtos · 2019-09-13T14:33:51Z

I would like the code building title suffix to be kept simple with hardcoded start/delimiter/end characters and also improve the code building options description to use JSON.stringify or stringify-object.

I feel the current proposal is adding too much complexity that will make the code more difficult to maintain for us.

ericzon · 2019-09-15T15:45:27Z

@bajtos pushed a simplified version using stringify-object.
I couldn't reuse stringifyModelSettings from utils because it creates a wrapper with settings field and we want the stringified version in a single line.

raymondfeng · 2019-09-16T19:42:48Z

@ericzon I have rebased your branch against latest master of strongloop/loopback-next. In the future, please use the following to pull in latest changes from upstream:

git remote add upstream [email protected]:strongloop/loopback-next.git
git fetch --all
git rebase upstream/master

bajtos · 2019-09-20T14:57:24Z

pushed a simplified version using stringify-object.
I couldn't reuse stringifyModelSettings from utils because it creates a wrapper with settings field and we want the stringified version in a single line.

@ericzon I don't see this change being applied to the patch. I don't see any new commits pushed around the time you have posted the comment. Did you perhaps forgot to git push the change?

ericzon · 2019-09-23T12:19:08Z

@bajtos now the changes are ready to check. I've squashed and rebase too.

packages/repository-json-schema/src/build-schema.ts

packages/cli/lib/utils.js

raymondfeng

Please address my comments.

bajtos

LGTM with two nit-picks that can be ignored.

@raymondfeng WDYT?

packages/repository-json-schema/src/__tests__/integration/build-schema.integration.ts

packages/repository-json-schema/src/build-schema.ts

bajtos

Almost there!

bajtos · 2019-09-26T08:14:50Z

packages/repository-json-schema/package-lock.json

+				"@hapi/hoek": "8.x.x"
+			}
+		},
+		"@loopback/build": {


Please revert changes in package-lock.json file. Now that you are not adding new dependencies, there is no need to change the lock file either.

For future reference: https://github.com/strongloop/loopback-next/blob/master/docs/site/DEVELOPING.md#working-with-dependencies
Here is the important part: Dependencies resolved locally within the monorepo must be excluded from package-lock files.

Nice, this problem was catched by our linter, I forget we have one in place!

See https://travis-ci.com/strongloop/loopback-next/jobs/238791798

Invalid package-lock entries found! packages/repository-json-schema/package-lock.json -> @loopback/build -> @loopback/context -> @loopback/core -> @loopback/eslint-config -> @loopback/metadata -> @loopback/repository -> @loopback/testlab Run the following command to fix the problems: $ npm run update-package-locks

Please don't run update-package-locks and just revert changes in this file. We prefer to update lock files via https://renovatebot.com

Sorry, I followed the suggestion of travis. Reverted.

bajtos · 2019-09-26T08:16:58Z

packages/repository-json-schema/src/build-schema.ts

@@ -15,6 +15,7 @@ import {
 import * as debugFactory from 'debug';
 import {JSONSchema6 as JSONSchema} from 'json-schema';
 import {JSON_SCHEMA_KEY, MODEL_TYPE_KEYS} from './keys';
+const nativeUtils = require('util');


Please use ES6 imports.

Suggested change

const nativeUtils = require('util');

import {inspect} from 'util';

Or if you want to import everything:

Suggested change

const nativeUtils = require('util');

import * as nativeUtils from 'util';

Changes updated

…lidation in azure

bajtos

👏

Let's get approval from @raymondfeng too.

nabdelgadir

👏

ericzon requested review from bajtos and raymondfeng as code owners August 4, 2019 14:19

raymondfeng reviewed Aug 5, 2019

View reviewed changes

packages/repository-json-schema/src/build-schema.ts Outdated Show resolved Hide resolved

dhmlau added the community-contribution label Aug 6, 2019

nabdelgadir reviewed Aug 7, 2019

View reviewed changes

packages/repository-json-schema/src/__tests__/unit/build-schema.unit.ts Outdated Show resolved Hide resolved

packages/repository-json-schema/src/build-schema.ts Outdated Show resolved Hide resolved

dhmlau assigned nabdelgadir Aug 14, 2019

bajtos previously requested changes Aug 16, 2019

View reviewed changes

packages/repository-json-schema/src/build-schema.ts Outdated Show resolved Hide resolved

nabdelgadir reviewed Aug 26, 2019

View reviewed changes

packages/repository-json-schema/src/__tests__/integration/build-schema.integration.ts Outdated Show resolved Hide resolved

bajtos reviewed Sep 2, 2019

View reviewed changes

packages/repository-json-schema/src/build-schema.ts Outdated Show resolved Hide resolved

bajtos mentioned this pull request Sep 10, 2019

Semantic Error at OASv3 for 'MessageExcluding[id]': #3698

Closed

bajtos self-assigned this Sep 10, 2019

bajtos mentioned this pull request Sep 12, 2019

Custom schema title for POST requests #3716

Merged

6 tasks

raymondfeng force-pushed the fix/adapt-to-azure-openapi branch from 2505074 to c0fd6c2 Compare September 12, 2019 20:34

raymondfeng force-pushed the fix/adapt-to-azure-openapi branch from 1f8b31e to 7535e69 Compare September 16, 2019 19:40

raymondfeng approved these changes Sep 17, 2019

View reviewed changes

ericzon force-pushed the fix/adapt-to-azure-openapi branch 5 times, most recently from 86a1ef9 to fe2119f Compare September 23, 2019 10:49

raymondfeng reviewed Sep 23, 2019

View reviewed changes

packages/repository-json-schema/src/build-schema.ts Outdated Show resolved Hide resolved

raymondfeng reviewed Sep 23, 2019

View reviewed changes

packages/cli/lib/utils.js Outdated Show resolved Hide resolved

raymondfeng requested changes Sep 23, 2019

View reviewed changes

ericzon force-pushed the fix/adapt-to-azure-openapi branch 2 times, most recently from 56f9d60 to 595fc54 Compare September 24, 2019 10:57

bajtos approved these changes Sep 24, 2019

View reviewed changes

packages/repository-json-schema/src/__tests__/integration/build-schema.integration.ts Outdated Show resolved Hide resolved

packages/repository-json-schema/src/build-schema.ts Outdated Show resolved Hide resolved

ericzon force-pushed the fix/adapt-to-azure-openapi branch 2 times, most recently from 8d4af5c to a3d8c00 Compare September 24, 2019 18:06

raymondfeng reviewed Sep 24, 2019

View reviewed changes

packages/repository-json-schema/src/build-schema.ts Outdated Show resolved Hide resolved

ericzon force-pushed the fix/adapt-to-azure-openapi branch from a3d8c00 to f4799ea Compare September 24, 2019 20:39

bajtos suggested changes Sep 26, 2019

View reviewed changes

fix(repository-json-schema): generate schema title compatible with va…

c353a63

…lidation in azure

ericzon force-pushed the fix/adapt-to-azure-openapi branch from f4799ea to c353a63 Compare September 26, 2019 14:30

bajtos approved these changes Sep 27, 2019

View reviewed changes

nabdelgadir approved these changes Sep 27, 2019

View reviewed changes

raymondfeng approved these changes Sep 27, 2019

View reviewed changes

raymondfeng merged commit b518876 into loopbackio:master Sep 27, 2019

This was referenced Jan 7, 2020

Add title to filter schemas #4355

Merged

feat(rest): add openapi schema consolidation #4365

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(repository-json-schema): update to pass openapi validation in azure #3504

fix(repository-json-schema): update to pass openapi validation in azure #3504

ericzon commented Aug 4, 2019 •

edited

Loading

raymondfeng commented Aug 5, 2019

dhmlau commented Aug 6, 2019

dhmlau commented Aug 6, 2019

raymondfeng commented Aug 7, 2019

nabdelgadir left a comment

dhmlau commented Aug 13, 2019

nabdelgadir commented Aug 13, 2019

dhmlau commented Aug 14, 2019

ericzon commented Aug 15, 2019 •

edited

Loading

bajtos left a comment

raymondfeng commented Aug 20, 2019

ericzon commented Aug 21, 2019 via email •

edited

Loading

ericzon commented Aug 24, 2019 •

edited

Loading

bajtos commented Sep 10, 2019

raymondfeng commented Sep 12, 2019

bajtos commented Sep 13, 2019

ericzon commented Sep 15, 2019 •

edited

Loading

raymondfeng commented Sep 16, 2019

bajtos commented Sep 20, 2019

ericzon commented Sep 23, 2019

raymondfeng left a comment

bajtos left a comment

bajtos left a comment

bajtos Sep 26, 2019

bajtos Sep 26, 2019

ericzon Sep 26, 2019

bajtos Sep 26, 2019

ericzon Sep 26, 2019

bajtos left a comment

nabdelgadir left a comment

	const nativeUtils = require('util');
	import {inspect} from 'util';

	const nativeUtils = require('util');
	import * as nativeUtils from 'util';

fix(repository-json-schema): update to pass openapi validation in azure #3504

fix(repository-json-schema): update to pass openapi validation in azure #3504

Conversation

ericzon commented Aug 4, 2019 • edited Loading

Checklist

raymondfeng commented Aug 5, 2019

dhmlau commented Aug 6, 2019

dhmlau commented Aug 6, 2019

raymondfeng commented Aug 7, 2019

nabdelgadir left a comment

Choose a reason for hiding this comment

dhmlau commented Aug 13, 2019

nabdelgadir commented Aug 13, 2019

dhmlau commented Aug 14, 2019

ericzon commented Aug 15, 2019 • edited Loading

bajtos left a comment

Choose a reason for hiding this comment

raymondfeng commented Aug 20, 2019

ericzon commented Aug 21, 2019 via email • edited Loading

ericzon commented Aug 24, 2019 • edited Loading

bajtos commented Sep 10, 2019

raymondfeng commented Sep 12, 2019

bajtos commented Sep 13, 2019

ericzon commented Sep 15, 2019 • edited Loading

raymondfeng commented Sep 16, 2019

bajtos commented Sep 20, 2019

ericzon commented Sep 23, 2019

raymondfeng left a comment

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

bajtos Sep 26, 2019

Choose a reason for hiding this comment

bajtos Sep 26, 2019

Choose a reason for hiding this comment

ericzon Sep 26, 2019

Choose a reason for hiding this comment

bajtos Sep 26, 2019

Choose a reason for hiding this comment

ericzon Sep 26, 2019

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

nabdelgadir left a comment

Choose a reason for hiding this comment

ericzon commented Aug 4, 2019 •

edited

Loading

ericzon commented Aug 15, 2019 •

edited

Loading

ericzon commented Aug 21, 2019 via email •

edited

Loading

ericzon commented Aug 24, 2019 •

edited

Loading

ericzon commented Sep 15, 2019 •

edited

Loading