spacewander commented on code in PR #8790:
URL: https://github.com/apache/apisix/pull/8790#discussion_r1100870285


##########
t/plugin/request-id.t:
##########
@@ -710,3 +710,144 @@ X-Request-ID: 123
 --- wait: 5
 --- response_body
 true
+
+
+
+=== TEST 21: check config with algorithm specified_character_id
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                        "plugins": {
+                            "request-id": {
+                                "algorithm": "specified_character_id",
+                                "specified_character_id_strs": "abcdefg",
+                                "specified_character_id_length" : 20
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1982": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/opentracing"
+                }]]
+                )
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 22: add plugin with algorithm specified_character_id (default uuid)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                        "plugins": {
+                            "request-id": {
+                                "algorithm": "specified_character_id"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1982": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/opentracing"
+                }]]
+                )
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 23: add plugin with algorithm specified_character_id (default uuid)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local http = require "resty.http"
+            local v = {}
+            local ids = {}
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                        "plugins": {
+                            "request-id": {
+                                "algorithm": "specified_character_id"
+                            }
+                        },
+                        "upstream": {

Review Comment:
   > When the test file is too large, for example > 800 lines, you should split 
it to a new file. Please take a look at t/plugin/limit-conn.t and 
t/plugin/limit-conn2.t
   
   
https://github.com/apache/apisix/blob/master/CONTRIBUTING.md#check-code-style-and-test-case-style



##########
apisix/plugins/request-id.lua:
##########
@@ -40,7 +43,17 @@ local schema = {
     properties = {
         header_name = {type = "string", default = "X-Request-Id"},
         include_in_response = {type = "boolean", default = true},
-        algorithm = {type = "string", enum = {"uuid", "snowflake", "nanoid"}, 
default = "uuid"}
+        algorithm = {
+            type = "string",
+            enum = {"uuid", "snowflake", "nanoid", "specified_character_id"},
+            default = "uuid"
+        },
+        specified_character_id_length = { type = "integer",  minimum = 6, 
default = 16 },

Review Comment:
   Can we group the fields under an algorithm like:
   ```
   specified_character = {
     length = 
   ```



##########
docs/en/latest/plugins/request-id.md:
##########
@@ -44,7 +44,9 @@ The Plugin will not add a unique ID if the request already 
has a header with the
 | ------------------- | ------- | -------- | -------------- | 
------------------------------- | 
---------------------------------------------------------------------- |
 | header_name         | string  | False    | "X-Request-Id" |                  
               | Header name for the unique request ID.                         
        |
 | include_in_response | boolean | False    | true           |                  
               | When set to `true`, adds the unique request ID in the response 
header. |
-| algorithm           | string  | False    | "uuid"         | ["uuid", 
"snowflake", "nanoid"] | Algorithm to use for generating the unique request ID. 
                |
+| algorithm           | string  | False    | "uuid"         | ["uuid", 
"snowflake", "nanoid", "specified_character_id"] | Algorithm to use for 
generating the unique request ID.                 |
+| specified_character_id_strs      | string | 否 | 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIGKLMNOPQRSTUVWXYZ0123456789| The minimum 
string length is 6 | Character set for specified_character_id |

Review Comment:
   This is an English doc



##########
apisix/plugins/request-id.lua:
##########
@@ -40,7 +43,17 @@ local schema = {
     properties = {
         header_name = {type = "string", default = "X-Request-Id"},
         include_in_response = {type = "boolean", default = true},
-        algorithm = {type = "string", enum = {"uuid", "snowflake", "nanoid"}, 
default = "uuid"}
+        algorithm = {
+            type = "string",
+            enum = {"uuid", "snowflake", "nanoid", "specified_character_id"},
+            default = "uuid"
+        },
+        specified_character_id_length = { type = "integer",  minimum = 6, 
default = 16 },
+        specified_character_id_strs = {
+            type = "string",
+            minLength = 6,

Review Comment:
   Why use `minLength = 6`? The same char can be used multiple times.



##########
apisix/plugins/request-id.lua:
##########
@@ -40,7 +43,17 @@ local schema = {
     properties = {
         header_name = {type = "string", default = "X-Request-Id"},
         include_in_response = {type = "boolean", default = true},
-        algorithm = {type = "string", enum = {"uuid", "snowflake", "nanoid"}, 
default = "uuid"}
+        algorithm = {
+            type = "string",
+            enum = {"uuid", "snowflake", "nanoid", "specified_character_id"},
+            default = "uuid"
+        },
+        specified_character_id_length = { type = "integer",  minimum = 6, 
default = 16 },
+        specified_character_id_strs = {

Review Comment:
   Would be better to call it `range`?



##########
apisix/plugins/request-id.lua:
##########
@@ -202,14 +215,28 @@ local function next_id()
     return snowflake:next_id()
 end
 
+-- generate random id
+local function get_random_id(strs, length)
+    local res = ""
+    for i = 1,length do
+        local idx = math_random(str_len(strs))
+        res = res .. str_sub(strs, idx, idx)
+    end
+    return res
+end

Review Comment:
   I provide a faster implementation:
   ```
   local function get_random_id(strs, length)
       local res = ffi.new("unsigned char[?]", length)
       for i = 0, length - 1 do
           res[i] = string.byte(strs, math_random(#strs))
       end
       return ffi.string(res, length)
   end
   ```
   
   You can write a benchmark to see the difference.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to