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: import route from OpenAPI Specification3.0 #1102

Merged
merged 51 commits into from
Jan 27, 2021

Conversation

Jaycean
Copy link
Member

@Jaycean Jaycean commented Dec 23, 2020

Signed-off-by: Jacean [email protected]

Import & Export route from OpenAPI Specification3.0


New feature or improvement

speed of progress:

It has implemented the basic information and authentication plugins of OpenAPI configuration file: basic-auth, JWT-auth, key-auth; parameter authentication plugin, request-validation and request header parameter parsing.

Remaining problems:

When writing etcd for parameter verification, the system will report the following error because there are no upstream and service related parameters in OpenAPI.

scheme validate fail: (root): Must validate at least one schema (anyOf)
(root): plugins is required

It is difficult to write to etcd in batch because it is necessary to verify the properties of upstream and service when writing data to etcd, and there is no relevant interface in the underlying layer.

@Jaycean Jaycean marked this pull request as draft December 23, 2020 02:24
@juzhiyuan
Copy link
Member

Any update about this feature?

# Conflicts:
#	api/go.mod
#	api/go.sum
#	api/internal/core/entity/entity.go
@codecov-io
Copy link

codecov-io commented Dec 29, 2020

Codecov Report

Merging #1102 (8c6ffa9) into master (9cb9aa7) will decrease coverage by 21.64%.
The diff coverage is 29.13%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1102       +/-   ##
===========================================
- Coverage   65.42%   43.77%   -21.65%     
===========================================
  Files          43       35        -8     
  Lines        2655     2508      -147     
===========================================
- Hits         1737     1098      -639     
- Misses        713     1253      +540     
+ Partials      205      157       -48     
Impacted Files Coverage Δ
api/internal/core/entity/entity.go 0.00% <ø> (-81.25%) ⬇️
api/internal/filter/schema.go 0.00% <ø> (-55.47%) ⬇️
api/internal/utils/utils.go 47.56% <0.00%> (-23.66%) ⬇️
api/internal/handler/data_loader/route_import.go 27.41% <27.41%> (ø)
api/internal/core/store/validate.go 58.10% <75.00%> (-10.83%) ⬇️
api/internal/core/store/store.go 81.81% <100.00%> (-4.26%) ⬇️
api/internal/filter/request_id.go 0.00% <0.00%> (-100.00%) ⬇️
api/internal/core/store/storehub.go 0.00% <0.00%> (-70.41%) ⬇️
api/internal/filter/cors.go 0.00% <0.00%> (-66.67%) ⬇️
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9cb9aa7...8c6ffa9. Read the comment docs.

@juzhiyuan juzhiyuan changed the title Import & Export route from OpenAPI Specification3.0 feat: import & Export route from OpenAPI Specification3.0 Dec 29, 2020
@liuxiran
Copy link
Contributor

Any update about this feature?

After communicated with @nic-chen , he will continue doing import on current pr, and @Jaycean will go on doing export route on another pr, and it is planed to be ready later next week.

cc @juzhiyuan

@juzhiyuan juzhiyuan added this to the 2.4 milestone Jan 6, 2021
@nic-chen nic-chen changed the title feat: import & Export route from OpenAPI Specification3.0 feat: import route from OpenAPI Specification3.0 Jan 11, 2021
@nic-chen nic-chen marked this pull request as ready for review January 11, 2021 13:56
@liuxiran
Copy link
Contributor

Do we need to add a test case about a route with as many props as possible?

@nic-chen
Copy link
Member

Do we need to add a test case about a route with as many props as possible

sure

api/internal/handler/data_loader/import.go Outdated Show resolved Hide resolved
api/internal/handler/data_loader/import.go Outdated Show resolved Hide resolved
api/internal/handler/data_loader/import.go Outdated Show resolved Hide resolved
api/internal/handler/data_loader/import.go Outdated Show resolved Hide resolved
api/internal/handler/data_loader/import.go Outdated Show resolved Hide resolved
api/internal/handler/data_loader/import.go Outdated Show resolved Hide resolved
api/internal/handler/data_loader/import.go Outdated Show resolved Hide resolved
api/internal/handler/route/route.go Outdated Show resolved Hide resolved
api/internal/route.go Outdated Show resolved Hide resolved
api/test/e2e/http.go Outdated Show resolved Hide resolved
Copy link
Contributor

@liuxiran liuxiran left a comment

Choose a reason for hiding this comment

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

this pr covered all the requirements refer to #825

@liuxiran
Copy link
Contributor

ping @starsz @imjoey @Jaycean for review, thanks

@liuxiran liuxiran requested a review from imjoey January 27, 2021 09:30
Copy link
Contributor

@starsz starsz left a comment

Choose a reason for hiding this comment

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

LGTM. Very grateful to you.

@liuxiran liuxiran merged commit 6ee8dd1 into apache:master Jan 27, 2021
@liuxiran
Copy link
Contributor

Thanks for @nic-chen and all reviewers, pr megerd

@Jaycean Jaycean deleted the import_export_route_from_openapi branch February 3, 2021 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants