membphis commented on a change in pull request #1920:
URL: https://github.com/apache/incubator-apisix/pull/1920#discussion_r461700477



##########
File path: apisix/plugins/request-validation.lua
##########
@@ -42,7 +42,28 @@ local _M = {
 
 
 function _M.check_schema(conf)
-    return core.schema.check(schema, conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return nil, err
+    end
+
+    if conf.body_schema then

Review comment:
       this style is better.
   
   ```lua
   if conf.body_schema then
       local ok, err = ....
       if not ok then
           return ....
       end
   end
   ```

##########
File path: apisix/core/schema.lua
##########
@@ -36,16 +36,27 @@ local function create_validator(schema)
     return nil, res -- error message
 end
 
-
-function _M.check(schema, json)
+local function get_validator(schema)
     local validator, err = cached_validator(schema, nil,
                                 create_validator, schema)
+
+    if not validator then
+        return false, err

Review comment:
       `false` -> `nil` is better

##########
File path: apisix/plugins/request-validation.lua
##########
@@ -42,7 +42,28 @@ local _M = {
 
 
 function _M.check_schema(conf)
-    return core.schema.check(schema, conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return nil, err
+    end
+
+    if conf.body_schema then
+        ok, err = core.schema.valid(conf.body_schema)
+    end
+
+    if not ok then
+        return nil, err

Review comment:
       we should use `false` here




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to