You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/29 05:04:42 UTC
[GitHub] [apisix-dashboard] nic-chen opened a new pull request #904: feat: support disable property for json schema according to APISIX's change
nic-chen opened a new pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [ ] Bugfix
- [x] New feature provided
- [ ] Improve performance
- Related issues
#495
support disable property for json schema according to APISIX's change
refer to :
https://github.com/apache/apisix/pull/2099/files
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] moonming commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r537157938
##########
File path: api/internal/core/store/validate_test.go
##########
@@ -249,6 +248,30 @@ func TestAPISIXJsonSchemaValidator_checkUpstream(t *testing.T) {
assert.EqualError(t, err, "schema validate failed: (root): Does not match pattern '^((uri|server_name|server_addr|request_uri|remote_port|remote_addr|query_string|host|hostname)|arg_[0-9a-zA-z_-]+)$'")
}
+func TestAPISIXJsonSchemaValidator_Plugin(t *testing.T) {
+ validator, err := NewAPISIXJsonSchemaValidator("main.route")
+ assert.Nil(t, err)
+
+ // plugin empty schema
+ route := &entity.Route{}
+ reqBody := `{
Review comment:
Can't understand the test case
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (75fef71) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/c574813eea82edcd6d95a2ea0f8857b736049717?el=desc) (c574813) will **increase** coverage by `0.17%`.
> The diff coverage is `59.09%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 43.49% 43.67% +0.17%
==========================================
Files 18 18
Lines 1299 1312 +13
==========================================
+ Hits 565 573 +8
- Misses 642 647 +5
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `58.33% <59.09%> (+0.29%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [c574813...75fef71](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r537274324
##########
File path: api/internal/core/store/validate_test.go
##########
@@ -249,6 +248,30 @@ func TestAPISIXJsonSchemaValidator_checkUpstream(t *testing.T) {
assert.EqualError(t, err, "schema validate failed: (root): Does not match pattern '^((uri|server_name|server_addr|request_uri|remote_port|remote_addr|query_string|host|hostname)|arg_[0-9a-zA-z_-]+)$'")
}
+func TestAPISIXJsonSchemaValidator_Plugin(t *testing.T) {
+ validator, err := NewAPISIXJsonSchemaValidator("main.route")
+ assert.Nil(t, err)
+
+ // validate plugin's schema which has no `properties` or empty `properties`
Review comment:
need E2E test cases @nic-chen
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r537961309
##########
File path: api/internal/core/store/validate_test.go
##########
@@ -249,6 +248,30 @@ func TestAPISIXJsonSchemaValidator_checkUpstream(t *testing.T) {
assert.EqualError(t, err, "schema validate failed: (root): Does not match pattern '^((uri|server_name|server_addr|request_uri|remote_port|remote_addr|query_string|host|hostname)|arg_[0-9a-zA-z_-]+)$'")
}
+func TestAPISIXJsonSchemaValidator_Plugin(t *testing.T) {
+ validator, err := NewAPISIXJsonSchemaValidator("main.route")
+ assert.Nil(t, err)
+
+ // validate plugin's schema which has no `properties` or empty `properties`
Review comment:
@ShiningRush we need to ensure the DP works well with `disable` property too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (fdcfa7f) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/c574813eea82edcd6d95a2ea0f8857b736049717?el=desc) (c574813) will **increase** coverage by `0.17%`.
> The diff coverage is `59.09%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 43.49% 43.67% +0.17%
==========================================
Files 18 18
Lines 1299 1312 +13
==========================================
+ Hits 565 573 +8
- Misses 642 647 +5
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `58.33% <59.09%> (+0.29%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [c574813...fdcfa7f](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen merged pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen merged pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] ShiningRush commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r533846077
##########
File path: api/internal/core/store/validate.go
##########
@@ -238,22 +238,37 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
return err
}
+ pluginDisableSchema := map[string]map[string]interface{}{
+ "disable": {"type": "boolean"},
+ }
plugins, schemaType := getPlugins(obj)
- //fix lua json.encode transform lua{properties={}} to json{"properties":[]}
- reg := regexp.MustCompile(`\"properties\":\[\]`)
for pluginName, pluginConf := range plugins {
- var schemaDef string
- schemaDef = conf.Schema.Get("plugins." + pluginName + "." + schemaType).String()
- if schemaDef == "" && schemaType == "consumer_schema" {
- schemaDef = conf.Schema.Get("plugins." + pluginName + ".schema").String()
+ schemaValue := conf.Schema.Get("plugins." + pluginName + "." + schemaType).Value()
+ if schemaValue == nil && schemaType == "consumer_schema" {
+ schemaValue = conf.Schema.Get("plugins." + pluginName + ".schema").Value()
}
- if schemaDef == "" {
- log.Warnf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
+ if schemaValue == nil {
+ log.Warnf("schema validate failed: schema not found, %s, %s", "plugins."+pluginName, schemaType)
return fmt.Errorf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
}
+ schemaMap := schemaValue.(map[string]interface{})
+ if schemaMap["type"] != nil && schemaMap["type"].(string) == "object" {
+ if properties, ok := schemaMap["properties"]; ok {
+ propertiesMap := properties.(map[string]interface{})
+ if len(propertiesMap) == 0 {
+ schemaMap["properties"] = pluginDisableSchema
+ }
+ } else {
+ schemaMap["properties"] = pluginDisableSchema
+ }
+ }
+ schemaByte, err := json.Marshal(schemaMap)
+ if err != nil {
+ log.Warnf("scheme validate failed: schema not found, path: %s, %w", "plugins."+pluginName, err)
+ return fmt.Errorf("scheme validate failed: schema not found, path: %s, %w", "plugins."+pluginName, err)
Review comment:
error info is wrong, here is a json.Marshal action.
##########
File path: api/internal/core/store/validate.go
##########
@@ -238,22 +238,37 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
return err
}
+ pluginDisableSchema := map[string]map[string]interface{}{
+ "disable": {"type": "boolean"},
+ }
plugins, schemaType := getPlugins(obj)
- //fix lua json.encode transform lua{properties={}} to json{"properties":[]}
- reg := regexp.MustCompile(`\"properties\":\[\]`)
for pluginName, pluginConf := range plugins {
- var schemaDef string
- schemaDef = conf.Schema.Get("plugins." + pluginName + "." + schemaType).String()
- if schemaDef == "" && schemaType == "consumer_schema" {
- schemaDef = conf.Schema.Get("plugins." + pluginName + ".schema").String()
+ schemaValue := conf.Schema.Get("plugins." + pluginName + "." + schemaType).Value()
+ if schemaValue == nil && schemaType == "consumer_schema" {
+ schemaValue = conf.Schema.Get("plugins." + pluginName + ".schema").Value()
}
- if schemaDef == "" {
- log.Warnf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
+ if schemaValue == nil {
+ log.Warnf("schema validate failed: schema not found, %s, %s", "plugins."+pluginName, schemaType)
return fmt.Errorf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
}
+ schemaMap := schemaValue.(map[string]interface{})
+ if schemaMap["type"] != nil && schemaMap["type"].(string) == "object" {
+ if properties, ok := schemaMap["properties"]; ok {
Review comment:
Too much branch this logic, we can refactor to
```go
// guard code
if schemaMap["properties"] == nil {
schemaMap["properties"] = make(map[string]interface{})
}
// ensure
if len(schemaMap["properties"].(map[string]interface{})) == 0 {
schemaMap["properties"] = pluginDisableSchema
}
```
BTW: I am curious if it is expected logic that we do nothing when user specified `properties` even it does not contain `disable`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-736531535
> Need to deal with conflict. @nic-chen
Thanks for remind.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (76bf60f) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/47d67d61ef98b5ce8ae482dc0bf135f015e09a4f?el=desc) (47d67d6) will **increase** coverage by `0.32%`.
> The diff coverage is `65.00%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 43.02% 43.35% +0.32%
==========================================
Files 18 18
Lines 1290 1301 +11
==========================================
+ Hits 555 564 +9
- Misses 643 645 +2
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `56.37% <65.00%> (+1.30%)` | :arrow_up: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `79.22% <0.00%> (+0.64%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [47d67d6...76bf60f](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] membphis commented on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-736592807
@gxthrj @ShiningRush @tokers pls take a look at this PR when you have time
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (1d69b70) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/c574813eea82edcd6d95a2ea0f8857b736049717?el=desc) (c574813) will **increase** coverage by `0.15%`.
> The diff coverage is `61.11%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 43.49% 43.65% +0.15%
==========================================
Files 18 18
Lines 1299 1308 +9
==========================================
+ Hits 565 571 +6
- Misses 642 644 +2
- Partials 92 93 +1
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `58.55% <61.11%> (+0.51%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [c574813...1d69b70](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io commented on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (659823d) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/478cbebd82cd7912f8cdac5f85a6086ceea191c2?el=desc) (478cbeb) will **increase** coverage by `0.33%`.
> The diff coverage is `65.00%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 42.95% 43.29% +0.33%
==========================================
Files 18 18
Lines 1271 1282 +11
==========================================
+ Hits 546 555 +9
- Misses 633 635 +2
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `56.37% <65.00%> (+1.30%)` | :arrow_up: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.94% <0.00%> (+0.65%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [478cbeb...659823d](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (e63099e) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/c574813eea82edcd6d95a2ea0f8857b736049717?el=desc) (c574813) will **increase** coverage by `0.10%`.
> The diff coverage is `59.09%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 43.49% 43.59% +0.10%
==========================================
Files 18 18
Lines 1299 1312 +13
==========================================
+ Hits 565 572 +7
- Misses 642 648 +6
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `58.33% <59.09%> (+0.29%)` | :arrow_up: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.57% <0.00%> (-0.65%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [c574813...e63099e](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r535762118
##########
File path: api/internal/core/store/validate.go
##########
@@ -238,22 +238,37 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
return err
}
+ pluginDisableSchema := map[string]map[string]interface{}{
+ "disable": {"type": "boolean"},
+ }
plugins, schemaType := getPlugins(obj)
- //fix lua json.encode transform lua{properties={}} to json{"properties":[]}
- reg := regexp.MustCompile(`\"properties\":\[\]`)
for pluginName, pluginConf := range plugins {
- var schemaDef string
- schemaDef = conf.Schema.Get("plugins." + pluginName + "." + schemaType).String()
- if schemaDef == "" && schemaType == "consumer_schema" {
- schemaDef = conf.Schema.Get("plugins." + pluginName + ".schema").String()
+ schemaValue := conf.Schema.Get("plugins." + pluginName + "." + schemaType).Value()
+ if schemaValue == nil && schemaType == "consumer_schema" {
+ schemaValue = conf.Schema.Get("plugins." + pluginName + ".schema").Value()
}
- if schemaDef == "" {
- log.Warnf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
+ if schemaValue == nil {
+ log.Warnf("schema validate failed: schema not found, %s, %s", "plugins."+pluginName, schemaType)
return fmt.Errorf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
}
+ schemaMap := schemaValue.(map[string]interface{})
+ if schemaMap["type"] != nil && schemaMap["type"].(string) == "object" {
+ if properties, ok := schemaMap["properties"]; ok {
+ propertiesMap := properties.(map[string]interface{})
+ if len(propertiesMap) == 0 {
+ schemaMap["properties"] = pluginDisableSchema
+ }
+ } else {
+ schemaMap["properties"] = pluginDisableSchema
+ }
+ }
+ schemaByte, err := json.Marshal(schemaMap)
+ if err != nil {
+ log.Warnf("scheme validate failed: schema not found, path: %s, %w", "plugins."+pluginName, err)
Review comment:
fixed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] moonming commented on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-739591803
What is this pr used for? I can not understand it too
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r535761793
##########
File path: api/internal/core/store/validate.go
##########
@@ -238,22 +238,37 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
return err
}
+ pluginDisableSchema := map[string]map[string]interface{}{
+ "disable": {"type": "boolean"},
+ }
plugins, schemaType := getPlugins(obj)
- //fix lua json.encode transform lua{properties={}} to json{"properties":[]}
- reg := regexp.MustCompile(`\"properties\":\[\]`)
for pluginName, pluginConf := range plugins {
- var schemaDef string
- schemaDef = conf.Schema.Get("plugins." + pluginName + "." + schemaType).String()
- if schemaDef == "" && schemaType == "consumer_schema" {
- schemaDef = conf.Schema.Get("plugins." + pluginName + ".schema").String()
+ schemaValue := conf.Schema.Get("plugins." + pluginName + "." + schemaType).Value()
+ if schemaValue == nil && schemaType == "consumer_schema" {
+ schemaValue = conf.Schema.Get("plugins." + pluginName + ".schema").Value()
}
- if schemaDef == "" {
- log.Warnf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
+ if schemaValue == nil {
+ log.Warnf("schema validate failed: schema not found, %s, %s", "plugins."+pluginName, schemaType)
return fmt.Errorf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
}
+ schemaMap := schemaValue.(map[string]interface{})
+ if schemaMap["type"] != nil && schemaMap["type"].(string) == "object" {
+ if properties, ok := schemaMap["properties"]; ok {
+ propertiesMap := properties.(map[string]interface{})
+ if len(propertiesMap) == 0 {
+ schemaMap["properties"] = pluginDisableSchema
+ }
+ } else {
+ schemaMap["properties"] = pluginDisableSchema
+ }
+ }
+ schemaByte, err := json.Marshal(schemaMap)
+ if err != nil {
+ log.Warnf("scheme validate failed: schema not found, path: %s, %w", "plugins."+pluginName, err)
+ return fmt.Errorf("scheme validate failed: schema not found, path: %s, %w", "plugins."+pluginName, err)
Review comment:
updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (ccd2d1d) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/478cbebd82cd7912f8cdac5f85a6086ceea191c2?el=desc) (478cbeb) will **increase** coverage by `0.33%`.
> The diff coverage is `65.00%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 42.95% 43.29% +0.33%
==========================================
Files 18 18
Lines 1271 1282 +11
==========================================
+ Hits 546 555 +9
- Misses 633 635 +2
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `56.37% <65.00%> (+1.30%)` | :arrow_up: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.94% <0.00%> (+0.65%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [478cbeb...ccd2d1d](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (6bd6c8d) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/b8c5c0d64cf46dbd1a6b9f0e8127dfccb5b37c04?el=desc) (b8c5c0d) will **increase** coverage by `0.55%`.
> The diff coverage is `76.19%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 43.49% 44.04% +0.55%
==========================================
Files 18 18
Lines 1299 1310 +11
==========================================
+ Hits 565 577 +12
Misses 642 642
+ Partials 92 91 -1
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `61.68% <76.19%> (+3.64%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [b8c5c0d...6bd6c8d](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r535762067
##########
File path: api/internal/core/store/validate.go
##########
@@ -238,22 +238,37 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
return err
}
+ pluginDisableSchema := map[string]map[string]interface{}{
+ "disable": {"type": "boolean"},
+ }
plugins, schemaType := getPlugins(obj)
- //fix lua json.encode transform lua{properties={}} to json{"properties":[]}
- reg := regexp.MustCompile(`\"properties\":\[\]`)
for pluginName, pluginConf := range plugins {
- var schemaDef string
- schemaDef = conf.Schema.Get("plugins." + pluginName + "." + schemaType).String()
- if schemaDef == "" && schemaType == "consumer_schema" {
- schemaDef = conf.Schema.Get("plugins." + pluginName + ".schema").String()
+ schemaValue := conf.Schema.Get("plugins." + pluginName + "." + schemaType).Value()
+ if schemaValue == nil && schemaType == "consumer_schema" {
+ schemaValue = conf.Schema.Get("plugins." + pluginName + ".schema").Value()
}
- if schemaDef == "" {
- log.Warnf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
+ if schemaValue == nil {
+ log.Warnf("schema validate failed: schema not found, %s, %s", "plugins."+pluginName, schemaType)
return fmt.Errorf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
}
+ schemaMap := schemaValue.(map[string]interface{})
+ if schemaMap["type"] != nil && schemaMap["type"].(string) == "object" {
+ if properties, ok := schemaMap["properties"]; ok {
Review comment:
@ShiningRush we only care about `properties` is nil or empty. please refer: https://github.com/apache/apisix/pull/2099/files
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r537384142
##########
File path: api/internal/core/store/validate_test.go
##########
@@ -249,6 +248,30 @@ func TestAPISIXJsonSchemaValidator_checkUpstream(t *testing.T) {
assert.EqualError(t, err, "schema validate failed: (root): Does not match pattern '^((uri|server_name|server_addr|request_uri|remote_port|remote_addr|query_string|host|hostname)|arg_[0-9a-zA-z_-]+)$'")
}
+func TestAPISIXJsonSchemaValidator_Plugin(t *testing.T) {
+ validator, err := NewAPISIXJsonSchemaValidator("main.route")
+ assert.Nil(t, err)
+
+ // validate plugin's schema which has no `properties` or empty `properties`
Review comment:
@membphis added.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] ShiningRush commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r537530822
##########
File path: api/internal/core/store/validate_test.go
##########
@@ -249,6 +248,30 @@ func TestAPISIXJsonSchemaValidator_checkUpstream(t *testing.T) {
assert.EqualError(t, err, "schema validate failed: (root): Does not match pattern '^((uri|server_name|server_addr|request_uri|remote_port|remote_addr|query_string|host|hostname)|arg_[0-9a-zA-z_-]+)$'")
}
+func TestAPISIXJsonSchemaValidator_Plugin(t *testing.T) {
+ validator, err := NewAPISIXJsonSchemaValidator("main.route")
+ assert.Nil(t, err)
+
+ // validate plugin's schema which has no `properties` or empty `properties`
Review comment:
Hmmm, I think we can not put all test case to e2e test, e2e test is too heavy to maintain.So e2e usually used for ensuring key path of the software, If some cases could covered by unit test, just maintain unit test cases is fine.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] LiteSun commented on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
LiteSun commented on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735887801
Need to deal with conflict. @nic-chen
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] ShiningRush commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r537538857
##########
File path: api/internal/core/store/validate_test.go
##########
@@ -249,6 +248,30 @@ func TestAPISIXJsonSchemaValidator_checkUpstream(t *testing.T) {
assert.EqualError(t, err, "schema validate failed: (root): Does not match pattern '^((uri|server_name|server_addr|request_uri|remote_port|remote_addr|query_string|host|hostname)|arg_[0-9a-zA-z_-]+)$'")
}
+func TestAPISIXJsonSchemaValidator_Plugin(t *testing.T) {
Review comment:
This case coverage is too low, it does not cover any failed case : (
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-739585208
@ShiningRush please review again，thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-739587712
@gxthrj @tokers please help review at your convenience，thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-739617830
> What is this pr used for? I can not understand it too
updated the description. please have a look.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (d2629f4) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/c574813eea82edcd6d95a2ea0f8857b736049717?el=desc) (c574813) will **increase** coverage by `0.05%`.
> The diff coverage is `51.42%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 43.49% 43.54% +0.05%
==========================================
Files 18 18
Lines 1299 1325 +26
==========================================
+ Hits 565 577 +12
- Misses 642 656 +14
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `56.80% <51.42%> (-1.24%)` | :arrow_down: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.57% <0.00%> (-0.65%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [c574813...d2629f4](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (32329a3) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/c574813eea82edcd6d95a2ea0f8857b736049717?el=desc) (c574813) will **increase** coverage by `0.15%`.
> The diff coverage is `61.11%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 43.49% 43.65% +0.15%
==========================================
Files 18 18
Lines 1299 1308 +9
==========================================
+ Hits 565 571 +6
- Misses 642 644 +2
- Partials 92 93 +1
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `58.55% <61.11%> (+0.51%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [c574813...1d69b70](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-735344204
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=h1) Report
> Merging [#904](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=desc) (b6ccd4a) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/c574813eea82edcd6d95a2ea0f8857b736049717?el=desc) (c574813) will **increase** coverage by `0.25%`.
> The diff coverage is `68.75%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #904 +/- ##
==========================================
+ Coverage 43.49% 43.75% +0.25%
==========================================
Files 18 18
Lines 1299 1305 +6
==========================================
+ Hits 565 571 +6
- Misses 642 643 +1
+ Partials 92 91 -1
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `60.40% <68.75%> (+2.36%)` | :arrow_up: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/904/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.57% <0.00%> (-0.65%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=footer). Last update [c574813...b6ccd4a](https://codecov.io/gh/apache/apisix-dashboard/pull/904?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] idbeta commented on a change in pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#discussion_r533962675
##########
File path: api/internal/core/store/validate.go
##########
@@ -238,22 +238,37 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
return err
}
+ pluginDisableSchema := map[string]map[string]interface{}{
+ "disable": {"type": "boolean"},
+ }
plugins, schemaType := getPlugins(obj)
- //fix lua json.encode transform lua{properties={}} to json{"properties":[]}
- reg := regexp.MustCompile(`\"properties\":\[\]`)
for pluginName, pluginConf := range plugins {
- var schemaDef string
- schemaDef = conf.Schema.Get("plugins." + pluginName + "." + schemaType).String()
- if schemaDef == "" && schemaType == "consumer_schema" {
- schemaDef = conf.Schema.Get("plugins." + pluginName + ".schema").String()
+ schemaValue := conf.Schema.Get("plugins." + pluginName + "." + schemaType).Value()
+ if schemaValue == nil && schemaType == "consumer_schema" {
+ schemaValue = conf.Schema.Get("plugins." + pluginName + ".schema").Value()
}
- if schemaDef == "" {
- log.Warnf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
+ if schemaValue == nil {
+ log.Warnf("schema validate failed: schema not found, %s, %s", "plugins."+pluginName, schemaType)
return fmt.Errorf("schema validate failed: schema not found, path: %s", "plugins."+pluginName)
}
+ schemaMap := schemaValue.(map[string]interface{})
+ if schemaMap["type"] != nil && schemaMap["type"].(string) == "object" {
+ if properties, ok := schemaMap["properties"]; ok {
+ propertiesMap := properties.(map[string]interface{})
+ if len(propertiesMap) == 0 {
+ schemaMap["properties"] = pluginDisableSchema
+ }
+ } else {
+ schemaMap["properties"] = pluginDisableSchema
+ }
+ }
+ schemaByte, err := json.Marshal(schemaMap)
+ if err != nil {
+ log.Warnf("scheme validate failed: schema not found, path: %s, %w", "plugins."+pluginName, err)
Review comment:
`scheme` -> `schema`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on pull request #904: feat: support disable property for json schema according to APISIX's change
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #904:
URL: https://github.com/apache/apisix-dashboard/pull/904#issuecomment-739443360
update?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org