feat: add Service and Upstream refactoring#497
Merged
Conversation
juzhiyuan
approved these changes
Sep 21, 2020
juzhiyuan
left a comment
juzhiyuan
left a comment
Member
There was a problem hiding this comment.
Looks good to me from the codes style, but needs @ShiningRush 's help to review the logics.
moonming
approved these changes
Sep 27, 2020
ShiningRush
requested changes
Sep 27, 2020
ShiningRush
left a comment
ShiningRush
left a comment
Contributor
There was a problem hiding this comment.
Pending comment again, my fault : (
| return nil, nil | ||
| } | ||
|
|
||
| func (h *Handler) Patch(c droplet.Context) (interface{}, error) { |
Contributor
There was a problem hiding this comment.
Missing route applying
| } else { | ||
| patch, err = jsonpatch.MakePatch(stored, input.Service) | ||
| if err != nil { | ||
| panic(err) |
Contributor
There was a problem hiding this comment.
panic is not a good practice, please using return nil, fmt.Errorf("make json patch failed:%w", err)
|
|
||
| err = patch.Apply(&stored) | ||
| if err != nil { | ||
| panic(err) |
Contributor
There was a problem hiding this comment.
return nil, fmt.Errorf("apply json patch failed:%w", err)
| return nil, nil | ||
| } | ||
|
|
||
| func (h *Handler) Patch(c droplet.Context) (interface{}, error) { |
Contributor
There was a problem hiding this comment.
Missing route applying
| return nil, err | ||
| } | ||
|
|
||
| var patch jsonpatch.Patch |
Contributor
There was a problem hiding this comment.
Hmm, all patch method looks similar, we should extract common logic to avoid repeat code.
Contributor
|
@nic-chen The pr has merged, we can open a new pr to fix problems. |
Member
|
cc @nic-chen |
juzhiyuan
pushed a commit
that referenced
this pull request
Oct 20, 2020
* feat: refactor some codes and append store core * refference droplet and write a API demo * fmt project * feat: add validator for generic store; add demo error * feat: upgrade droplet * chore: add structures (#484) * add upstream struct * fix structures * feat: append stock check for generic store * feat: add consumer CURD refactoring (#486) * feat: add consumer CURD refactoring * remove debug * remove useless slashes * feat: add SSL refactoring (#488) Co-authored-by: Vinci Xu <[email protected]> * feat: add Service and Upstream refactoring (#497) * feat: add Service and Upstream refactoring * fix add license * feat: add store hub and flake id (#534) * feat: add store hub and flake id * feat: add store interface to easy est * fix: add test cases for refactored apis and fix bugs (#528) * test: delete mysql version test cases. * fix: add ssl test cases and bug fix * fix: list api should return an empty array not a null for client * test: consumer test cases * fix: code style * test: init etcd in github action * fix: skip checking generated file's license * feat: add store hub and flake id (#534) * feat: add store hub and flake id * feat: add store interface to easy est * fix CI failed Co-authored-by: Vinci Xu <[email protected]> * feat: refactor apis for existing check and other apis (#535) * fix code style * fix code style * feat support query * feat: support query * change: `like` to `equal` * fix api status * feat: upstream existing check * feat: refactor api for upstream names * fix: license * test: add unit test cases * fix: update bug * test: add test cases for route * fix: unified respond format * test: remove test bug * feat: ssl existing check * fix bug: auto generate id * fix: improve consumer * fix: remove key and keys in ssl respond * fix: when list is empty, should respond an empty array * fix code style * feat: plugin orchestration * fix delete bug * fix bug * fix: keep the same request params and respond with the old format * fix: append sort for list and using sync.Map instead of map * feature: sync json schema from APISIX and check schema when create or update resource (#551) * feat: json schema check * fix: don't need to define struct for each resource, because that may cause json schema check fail. * test: add handler test cases * test: complete consumer test cases * test: add test cases for schema check * fix code style and license * feat: add schema check for plugins * test: add ssl handler test cases * test: add test cases for upstream and service * test: add test cases for route * test: add note for route create * test: update CI * fix: remove useless file * test: fix CI * fix: ci fail * test: fix lib `dag-to-lua`'s path in CI * fix: URI for route may be empty * fix: remove empty lines * fix: refactor validator of json schema * fix code style * fix cicd * chore: update docker file * fix: should check schema after id generated * fix code style * chore: page_number -> page * fix: schema sync script * fix: code style * feat: support search for resource list (#557) * feat: support search for resource list * fix ci * feature: refactor plugin api and auth api (#556) * feat: refactor plugin and healthy api * feat: refactor authentication api * fix: remove useless files * chore: update json schema * test: add login test * test: add test for plugin * fix: license * fix auth bug * fix route search by uri * feat: compatible with PUT method of `admin api` and nodes of upstream (#561) * feat: support labels * feat: compatible with PUT method of `admin api` * fix mock test fail * feat: upstream nodes format * test: add test case * fix code style * fix: update schema sync tool * feat: compatible with HTTP status of `admin api` (#563) * feat: compatible with HTTP status of `admin api` * test cases and improve * fix: check input.ID before using it * fix: remove CI branch * fix: remove useless dependences * add license for json.lua * fix license issue * remove * fix: remove viper that depend github.com/hashicorp/hcl * fix license issue * fix: skip license check temporarily for CI Co-authored-by: vincixu <[email protected]> Co-authored-by: ShiningRush <[email protected]> Co-authored-by: WenMing <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please answer these questions before submitting a pull request
add Service and Upstream refactoring