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/02/23 15:31:20 UTC

[GitHub] [incubator-apisix] agile6v opened a new pull request #1153: feature: support for proxy caching plugin based on disk.

agile6v opened a new pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153
 
 
   ### Summary
   
   This is a prototype implementation and there is a lot of work to be done.
   
   Hopefully it does not affect the review. Thanks.
   
   The admin api looks like the following:
   
   ``` json
   {
       "plugins": {
           "proxy-cache": {
              "cache_zone": "disk_cache_one",
              "cache_key": "${uri}-cache-key",
              "cache_bypass": "${arg_bypass}",
              "cache_method": ["GET"],
              "cache_http_status": [200],
              "hide_cache_headers": true,
              "no_cache": "$arg_no_cache"
           }
       },
       "upstream": {
           "nodes": {
               "127.0.0.1:1999": 1
           },
           "type": "roundrobin"
       },
       "uri": "/hello"
   }
   ```
   
   
   Fix #1127 
   

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392637695
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,258 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
 
 Review comment:
   OK. But I found if the type is array, the default will not take effect. I'm looking at why.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-599217208
 
 
   merged, many thx @agile6v 

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-593185571
 
 
   > How about the new style? I think the new style is simpler.
   > 
   > ```json
   > # current style
   > "proxy-cache": {
   >     "cache_key": ["$uri"]
   >     "cache_bypass": ["$arg_bypass"],
   >     ... ...
   > }
   > ```
   > 
   > ```json
   > # new style
   > "proxy-cache": {
   >     "cache_key": ["uri"]
   >     "cache_bypass": ["arg_bypass"],
   >     ... ...
   > }
   > ```
   
   Here can use variables or strings, so need an identifier to distinguish it.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392629937
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,258 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
 
 Review comment:
   How about setting a default value for "cache_key"?

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-599168105
 
 
   @agile6v It seems that you have to rebase your branch, this branch has conflicted with master branch.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r386752162
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -178,6 +158,91 @@ local function match_method_and_status(conf, ctx)
     return false
 end
 
+-- refer to https://gist.github.com/titpetric/ed6ec548af160e82c650cf39074878fb
+local function file_exists(name)
+    local f = io_open(name, "r")
+    if f~=nil then io_close(f) return true else return false end
+end
+
+
+local function explode(d, p)
+    local t, ll
+    t={}
+    ll=0
+    if(#p == 1) then return {p} end
+    while true do
+        local l=string.find(p, d, ll, true) -- find the next d in the string
+        if l~=nil then -- if "not not" found then..
+                tab_insert(t, string.sub(p, ll, l-1)) -- Save it in our array.
+                ll=l+1 -- save just after where we found it for searching next time.
+        else
+                tab_insert(t, string.sub(p, ll)) -- Save what's left in our array.
+                break -- Break at end, as it should be, according to the lua manual.
+        end
+    end
+    return t
+end
+
+
+local function generate_cache_filename(cache_path, cache_levels, cache_key)
+    local md5sum = ngx.md5(cache_key)
+    local levels = explode(":", cache_levels)
+    local filename = ""
+
+    local index = string.len(md5sum)
+    for k, v in pairs(levels) do
+            local length = tonumber(v)
+            -- add trailing [length] chars to index
+            index = index - length;
+            filename = filename .. md5sum:sub(index+1, index+length) .. "/";
+    end
+    if cache_path:sub(-1) ~= "/" then
+            cache_path = cache_path .. "/";
+    end
+    filename = cache_path .. filename .. md5sum
+    return filename
+end
+
+
+local function cache_purge(conf, ctx)
+    local cache_zone_info = ngx_re.split(ctx.var.upstream_cache_zone_info, ",")
+
+    local filename = generate_cache_filename(cache_zone_info[1], cache_zone_info[2],
+                                             ctx.var.upstream_cache_key)
+    if file_exists(filename) then
+        os.remove(filename)
 
 Review comment:
   Only privileged agents can delete cached files. 
   The worker process defaults to the nobody user, which does not have this permission.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-593721657
 
 
   https://github.com/apache/incubator-apisix/pull/1153/files/56a1f42fd1e8bd30575856758a51e8a21acf2e4c..b3ccdb372520e2849871afe017591a148be4664c#diff-df639fd03e25e10ae720a3ccd80a9430R187
   
   that is cool.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r384856435
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
+            minLength = 1
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+                default = {"GET", "HEAD"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
+        },
+        hide_cache_headers = {
+            type = "boolean",
+            default = false,
+        },
+        cache_strategy = {
+            type = "string",
+            default = "disk",
+            enum = {"disk", "memory"},
+            minLength = 0
+        },
+        cache_bypass = {
+            type = "string",
+            default = "1",
+            minLength = 0
+        },
+        no_cache = {
+            type = "string",
+            default = "0",
+            minLength = 0
+        },
+    },
+    required = {"cache_zone", "cache_key"},
+}
+
+local _M = {
+    version = 0.1,
+    priority = 1009,
+    name = plugin_name,
+    schema = schema,
+}
+
+function _M.check_schema(conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return false, err
+    end
+
+    if conf.cache_strategy == "memory" then
+        return false, "memory cache is not yet supported."
+    end
+
+    return true
+end
+
+-- Copy from redirect plugin, this function is useful.
+-- It can be extracted as a public function.
+local function parse_complex_value(complex_value)
+
+    local reg = [[ (\\\$[0-9a-zA-Z_]+) | ]]     -- \$host
+            .. [[ \$\{([0-9a-zA-Z_]+)\} | ]]    -- ${host}
+            .. [[ \$([0-9a-zA-Z_]+) | ]]        -- $host
+            .. [[ (\$|[^$\\]+) ]]               -- $ or others
+    local iterator, err = re_gmatch(complex_value, reg, "jiox")
+    if not iterator then
+        return nil, err
+    end
+
+    local t = {}
+    while true do
+        local m, err = iterator()
+        if err then
+            return nil, err
+        end
+
+        if not m then
+            break
+        end
+
+        tab_insert(t, m)
+    end
+
+    return t
+end
+
+
+local tmp = {}
+local function generate_complex_value(data, ctx)
+    local segs_value, err = lrucache(data, nil, parse_complex_value, data)
+    if not segs_value then
+        return nil, err
+    end
+
+    core.table.clear(tmp)
+
+    for i, value in ipairs(segs_value) do
+        core.log.info("complex value(", data, ") seg-", i, ": ", core.json.delay_encode(value))
+
+        local pat1 = value[1]    -- \$host
+        local pat2 = value[2]    -- ${host}
+        local pat3 = value[3]    -- $host
+        local pat4 = value[4]    -- $ or others
+
+        if pat2 or pat3 then
+            tab_insert(tmp, ctx.var[pat2 or pat3])
+        else
+            tab_insert(tmp, pat1 or pat4)
+        end
+    end
+
+    return tab_concat(tmp, "")
+end
+
+
+function _M.rewrite(conf, ctx)
+    core.log.info("proxy-cache plugin rewrite phase, conf: ", core.json.delay_encode(conf))
+
+    ctx.var.upstream_cache_zone = conf.cache_zone
+
+    local value, err = generate_complex_value(conf.cache_key, ctx)
+    if not value then
+        core.log.error("failed to generate the complex value by: ", conf.cache_key, " error: ", err)
+        core.response.exit(500)
+    end
+
+    ctx.var.upstream_cache_key = value
+    core.log.info("proxy-cache cache key value:", value)
+
+    local value, err = generate_complex_value(conf.cache_bypass, ctx)
+    if not value then
+        core.log.error("failed to generate the complex value by: ",
+                       conf.cache_bypass, " error: ", err)
+        core.response.exit(500)
+    end
+
+    ctx.var.upstream_cache_bypass = value
+    core.log.info("proxy-cache cache bypass value:", value)
+end
+
+
+function _M.header_filter(conf, ctx)
+    core.log.info("proxy-cache plugin header filter phase, conf: ", core.json.delay_encode(conf))
+
+    local no_cache = "1"
+
+    -- Maybe there is no need for optimization here.
+    for _, method in ipairs(conf.cache_method) do
+        if method == ctx.var.request_method then
+            no_cache = "0"
 
 Review comment:
   ```suggestion
               no_cache = "0"
   ```
   do we need add `break` after this line?

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392629937
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,258 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
 
 Review comment:
   Is it possible to set a default value?

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-593192162
 
 
   > And I have other new questions:
   > 
   > 1. need some e2e test case
   > 2. need a way to purge the old cache
   
   I'm also considering these things. For 2 if there is no better solution, [cache purge script](https://github.com/perusio/nginx-cache-purge) will be the final choice.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v edited a comment on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v edited a comment on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-591469470
 
 
   Hi @moonming 
   
   Tests cases I think the t/servroot/conf/nginx.conf file should be modified.
   
   In recent days, i have been preparing to make a presentation. So I'm gonna add tests in this weekend. 
   And are there any other problems with code implementation? 
   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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r385680075
 
 

 ##########
 File path: conf/config.yaml
 ##########
 @@ -35,6 +35,19 @@ apisix:
   #  enable_tcp_pp: true           # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
   #  enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
 
+  #proxy_cache:                     # Proxy Caching configuration
+  #  cache_ttl: 10s                 # The default caching time if the upstream does not specify the cache time
+  #   zone:                          # The parameters of a cache
+  #  - name: disk_cache_one         # The name of the cache, administrator can be specify
 
 Review comment:
   bad indentation
   
   ```yaml
     # proxy_cache:                            # Proxy Caching configuration
     #   cache_ttl: 1m                         # The default caching time if the upstream does not specify the cache time
     #   zones:                                # The parameters of a cache
     #     - name: disk_cache_one              # The name of the cache, administrator can be specify
     #                                         # which cache to use by name in the admin api
     #       memory_size: 50m                  # The size of shared memory, it's used to store the cache index
     #       disk_size: 1G                     # The size of disk, it's used to store the cache data
     #       disk_path: "/tmp/disk_cache_one"  # The path to store the cache data
     #     - name: disk_cache_two
     #       memory_size: 50m
     #       disk_size: 1G
     #       disk_path: "/tmp/disk_cache_two"
   ```

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r385680432
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
+            minLength = 1
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+                default = {"GET", "HEAD"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
+        },
+        hide_cache_headers = {
+            type = "boolean",
+            default = false,
+        },
+        cache_strategy = {
+            type = "string",
+            default = "disk",
+            enum = {"disk", "memory"},
+            minLength = 0
+        },
+        cache_bypass = {
+            type = "string",
+            default = "1",
+            minLength = 0
+        },
+        no_cache = {
+            type = "string",
+            default = "0",
+            minLength = 0
+        },
+    },
+    required = {"cache_zone", "cache_key"},
+}
+
+local _M = {
+    version = 0.1,
+    priority = 1009,
+    name = plugin_name,
+    schema = schema,
+}
+
+function _M.check_schema(conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return false, err
+    end
+
+    if conf.cache_strategy == "memory" then
 
 Review comment:
   drop it first

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r386071098
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
 
 Review comment:
   The main user of the Admin API is the dashboard, and array objects are more easily controlled by it.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392657840
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,259 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "a key for caching",
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]],
+            },
+            default = {"$host", "$request_uri"}
 
 Review comment:
   `{"$request_method", "$host", "$request_uri"}`
   
   I think we need to add `method`

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392641635
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,258 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
 
 Review comment:
   I have fixed this bug for days. you can rebase your branch.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-599180094
 
 
   > @agile6v It seems that you have to rebase your branch, this branch has conflicted with master branch.
   
   Resolved.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r385677893
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
+            minLength = 1
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+                default = {"GET", "HEAD"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
 
 Review comment:
   it is useless

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-599215439
 
 
   LGTM, we can merge it after run the test cases.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392659815
 
 

 ##########
 File path: t/plugin/proxy-cache.t
 ##########
 @@ -0,0 +1,630 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+log_level('info');
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    my $http_config = $block->http_config // <<_EOC_;
+
+    # for proxy cache
+    proxy_cache_path /tmp/disk_cache_one levels=1:2 keys_zone=disk_cache_one:50m inactive=1d max_size=1G;
+    proxy_cache_path /tmp/disk_cache_two levels=1:2 keys_zone=disk_cache_two:50m inactive=1d max_size=1G;
+
+    # for proxy cache
+    map \$upstream_cache_zone \$upstream_cache_zone_info {
+        disk_cache_one /tmp/disk_cache_one,1:2;
+        disk_cache_two /tmp/disk_cache_two,1:2;
+    }
+
+    server {
+        listen 1986;
+        server_tokens off;
+
+        location / {
+            expires 60s;
+            return 200 "hello world!";
+        }
+
+        location /hello-not-found {
+            return 404;
+        }
+    }
+_EOC_
+
+    $block->set_value("http_config", $http_config);
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity check (missing required field)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: sanity check (invalid type for cache_method)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": "GET",
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: sanity check (invalid type for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": "${uri}-cache-key",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: sanity check (invalid type for cache_bypass)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": "$arg_bypass",
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: sanity check (invalid type for no_cache)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": "$arg_no_cache"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: sanity check (illegal character for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": ["$uri-", "-cache-id"],
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: sanity check (normal case)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: hit route (cache miss)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: hit route (cache hit)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: hit route (cache bypass)
+--- request
+GET /hello?bypass=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: BYPASS
+--- no_error_log
+[error]
+
+
+
+=== TEST 11: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 12: hit route (nocache)
+--- request
+GET /hello?no_cache=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: hit route (there's no cache indeed)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: hit route (will be cached)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: hit route (not found)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: hit route (404 there's no cache indeed)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: hit route (HEAD method)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 18: hit route (HEAD method there's no cache)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 19:  hide cache headers = false
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": false,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 20: hit route (catch the cache headers)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- response_headers_like
+Cache-Control:
+--- no_error_log
+[error]
+
+
+
+=== TEST 21: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: purge cache (not found)
+--- request
+PURGE /hello-world
+--- error_code: 404
+--- no_error_log
+[error]
+
+
+
+=== TEST 23:  invalid cache zone
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "invalid_disk_cache",
 
 Review comment:
   I've also considered whether a message should be thrown.  But the check_schema function cannot get variable.  Currently return 500 is the Nginx default behavior. 

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v edited a comment on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v edited a comment on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-590082018
 
 
   Currently only supports the upstream dynamically specify the cache time. If the upstream does not specify it such as the header `Expires` or `Cache-Control`, the `cache_ttl` in the config.yaml will be used.  And the option `hide_cache_headers ` will be decide whether the upstream header `Expires` and `Cache-Control` is sent to the client.
   It's just a trick. Hopefully it can be easily understand.
   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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r385705627
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
+            minLength = 1
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+                default = {"GET", "HEAD"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
+        },
+        hide_cache_headers = {
+            type = "boolean",
+            default = false,
+        },
+        cache_strategy = {
+            type = "string",
+            default = "disk",
+            enum = {"disk", "memory"},
+            minLength = 0
+        },
+        cache_bypass = {
+            type = "string",
+            default = "1",
+            minLength = 0
+        },
+        no_cache = {
+            type = "string",
+            default = "0",
+            minLength = 0
+        },
+    },
+    required = {"cache_zone", "cache_key"},
+}
+
+local _M = {
+    version = 0.1,
+    priority = 1009,
+    name = plugin_name,
+    schema = schema,
+}
+
+function _M.check_schema(conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return false, err
+    end
+
+    if conf.cache_strategy == "memory" then
+        return false, "memory cache is not yet supported."
+    end
+
+    return true
+end
+
+-- Copy from redirect plugin, this function is useful.
+-- It can be extracted as a public function.
+local function parse_complex_value(complex_value)
+
+    local reg = [[ (\\\$[0-9a-zA-Z_]+) | ]]     -- \$host
+            .. [[ \$\{([0-9a-zA-Z_]+)\} | ]]    -- ${host}
+            .. [[ \$([0-9a-zA-Z_]+) | ]]        -- $host
+            .. [[ (\$|[^$\\]+) ]]               -- $ or others
+    local iterator, err = re_gmatch(complex_value, reg, "jiox")
+    if not iterator then
+        return nil, err
+    end
+
+    local t = {}
+    while true do
+        local m, err = iterator()
+        if err then
+            return nil, err
+        end
+
+        if not m then
+            break
+        end
+
+        tab_insert(t, m)
+    end
+
+    return t
+end
+
+
+local tmp = {}
+local function generate_complex_value(data, ctx)
 
 Review comment:
   It can parse strings like `${uri}/helloworld` 

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r387398703
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,278 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "a key for caching",
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]]
+            },
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
+        },
+        hide_cache_headers = {
+            type = "boolean",
+            default = false,
+        },
+        cache_bypass = {
+            type = "array",
+            minItems = 1,
+            items = {
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]]
+            },
+        },
+        no_cache = {
+            type = "array",
+            minItems = 1,
+            items = {
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]]
+            },
+        },
+    },
+    required = {"cache_zone", "cache_key"},
+}
+
+local _M = {
+    version = 0.1,
+    priority = 1009,
+    name = plugin_name,
+    schema = schema,
+}
+
+
+function _M.check_schema(conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return false, err
+    end
+
+    return true
+end
+
+
+local tmp = {}
+local function generate_complex_value(data, ctx)
+    core.table.clear(tmp)
+
+    core.log.info("proxy-cache complex value: ", core.json.delay_encode(data))
+    for i, value in ipairs(data) do
+        core.log.info("proxy-cache complex value index-", i, ": ", value)
+
+        if string.sub(value, 1, 1) == "$" then
+            tab_insert(tmp, ctx.var[string.sub(value, 2)])
+        else
+            tab_insert(tmp, value)
+        end
+    end
+
+    return tab_concat(tmp, "")
+end
+
+
+-- check whether the request method and response status
+-- match the user defined.
+local function match_method_and_status(conf, ctx)
+    local match_method, match_status = false, false
+
+    -- Maybe there is no need for optimization here.
+    for _, method in ipairs(conf.cache_method) do
+        if method == ctx.var.request_method then
+            match_method = true
+            break
+        end
+    end
+
+    for _, status in ipairs(conf.cache_http_status) do
+        if status == ngx.status then
+            match_status = true
+            break
+        end
+    end
+
+    if match_method and match_status then
+        return true
+    end
+
+    return false
+end
+
+-- refer to https://gist.github.com/titpetric/ed6ec548af160e82c650cf39074878fb
 
 Review comment:
   This code has no copyright statement. The author reserves all rights by default and cannot be used in Apache APISIX.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-593183186
 
 
   How about the new style? I think the new style is simpler.
   
   ```json
   # current style
   "proxy-cache": {
       "cache_key": ["$uri"]
       "cache_bypass": ["$arg_bypass"],
       ... ...
   }
   ```
   
   ```json
   # new style
   "proxy-cache": {
       "cache_key": ["uri"]
       "cache_bypass": ["arg_bypass"],
       ... ...
   }
   ```

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392673996
 
 

 ##########
 File path: t/plugin/proxy-cache.t
 ##########
 @@ -0,0 +1,630 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+log_level('info');
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    my $http_config = $block->http_config // <<_EOC_;
+
+    # for proxy cache
+    proxy_cache_path /tmp/disk_cache_one levels=1:2 keys_zone=disk_cache_one:50m inactive=1d max_size=1G;
+    proxy_cache_path /tmp/disk_cache_two levels=1:2 keys_zone=disk_cache_two:50m inactive=1d max_size=1G;
+
+    # for proxy cache
+    map \$upstream_cache_zone \$upstream_cache_zone_info {
+        disk_cache_one /tmp/disk_cache_one,1:2;
+        disk_cache_two /tmp/disk_cache_two,1:2;
+    }
+
+    server {
+        listen 1986;
+        server_tokens off;
+
+        location / {
+            expires 60s;
+            return 200 "hello world!";
+        }
+
+        location /hello-not-found {
+            return 404;
+        }
+    }
+_EOC_
+
+    $block->set_value("http_config", $http_config);
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity check (missing required field)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: sanity check (invalid type for cache_method)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": "GET",
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: sanity check (invalid type for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": "${uri}-cache-key",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: sanity check (invalid type for cache_bypass)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": "$arg_bypass",
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: sanity check (invalid type for no_cache)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": "$arg_no_cache"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: sanity check (illegal character for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": ["$uri-", "-cache-id"],
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: sanity check (normal case)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: hit route (cache miss)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: hit route (cache hit)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: hit route (cache bypass)
+--- request
+GET /hello?bypass=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: BYPASS
+--- no_error_log
+[error]
+
+
+
+=== TEST 11: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 12: hit route (nocache)
+--- request
+GET /hello?no_cache=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: hit route (there's no cache indeed)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: hit route (will be cached)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: hit route (not found)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: hit route (404 there's no cache indeed)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: hit route (HEAD method)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 18: hit route (HEAD method there's no cache)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 19:  hide cache headers = false
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": false,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 20: hit route (catch the cache headers)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- response_headers_like
+Cache-Control:
+--- no_error_log
+[error]
+
+
+
+=== TEST 21: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: purge cache (not found)
+--- request
+PURGE /hello-world
+--- error_code: 404
+--- no_error_log
+[error]
+
+
+
+=== TEST 23:  invalid cache zone
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "invalid_disk_cache",
 
 Review comment:
   We can get the local configuration this way. Here is an example:
   
   https://github.com/apache/incubator-apisix/blob/aa3c71a10ad9c37dcb86653a6a4ef325fa86097b/lua/apisix/plugins/heartbeat.lua#L84

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-593183693
 
 
   And I have other new questions:
   
   1. need some e2e test case
   2. need a way to purge the old cache 

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r383055441
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,247 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+local tostring = tostring
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
+            minLength = 1
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+                default = {"GET", "HEAD"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
+        },
+        hide_cache_headers = {
+            type = "boolean",
+            default = false,
+        },
+        cache_strategy = {
+            type = "string",
+            default = "disk",
+            enum = {"disk", "memory"},
+            minLength = 0
+        },
+        cache_bypass = {
+            type = "string",
+            default = "1",
+            minLength = 0
+        },
+        no_cache = {
+            type = "string",
+            default = "0",
+            minLength = 0
+        },
+    },
+    required = {"cache_zone", "cache_key"},
+}
+
+local _M = {
+    version = 0.1,
+    priority = 1007,
+    name = plugin_name,
+    schema = schema,
+}
+
+function _M.check_schema(conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return false, err
+    end
+
+    if conf.cache_strategy == "memory" then
+        return false, "memory cache is not yet supported."
+    end
+
+    local t = {}
+    for _, method in ipairs(conf.cache_method) do
 
 Review comment:
   That is not a good way for the dashboard. the dashboard submits `cache_method` in an array way, but it will get `cache_method` in a hash way.  
   
   It will make the dashboard developer confuse.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392666086
 
 

 ##########
 File path: t/plugin/proxy-cache.t
 ##########
 @@ -0,0 +1,630 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+log_level('info');
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    my $http_config = $block->http_config // <<_EOC_;
+
+    # for proxy cache
+    proxy_cache_path /tmp/disk_cache_one levels=1:2 keys_zone=disk_cache_one:50m inactive=1d max_size=1G;
+    proxy_cache_path /tmp/disk_cache_two levels=1:2 keys_zone=disk_cache_two:50m inactive=1d max_size=1G;
+
+    # for proxy cache
+    map \$upstream_cache_zone \$upstream_cache_zone_info {
+        disk_cache_one /tmp/disk_cache_one,1:2;
+        disk_cache_two /tmp/disk_cache_two,1:2;
+    }
+
+    server {
+        listen 1986;
+        server_tokens off;
+
+        location / {
+            expires 60s;
+            return 200 "hello world!";
+        }
+
+        location /hello-not-found {
+            return 404;
+        }
+    }
+_EOC_
+
+    $block->set_value("http_config", $http_config);
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity check (missing required field)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: sanity check (invalid type for cache_method)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": "GET",
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: sanity check (invalid type for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": "${uri}-cache-key",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: sanity check (invalid type for cache_bypass)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": "$arg_bypass",
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: sanity check (invalid type for no_cache)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": "$arg_no_cache"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: sanity check (illegal character for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": ["$uri-", "-cache-id"],
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: sanity check (normal case)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: hit route (cache miss)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: hit route (cache hit)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: hit route (cache bypass)
+--- request
+GET /hello?bypass=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: BYPASS
+--- no_error_log
+[error]
+
+
+
+=== TEST 11: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 12: hit route (nocache)
+--- request
+GET /hello?no_cache=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: hit route (there's no cache indeed)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: hit route (will be cached)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: hit route (not found)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: hit route (404 there's no cache indeed)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: hit route (HEAD method)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 18: hit route (HEAD method there's no cache)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 19:  hide cache headers = false
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": false,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 20: hit route (catch the cache headers)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- response_headers_like
+Cache-Control:
+--- no_error_log
+[error]
+
+
+
+=== TEST 21: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: purge cache (not found)
+--- request
+PURGE /hello-world
+--- error_code: 404
+--- no_error_log
+[error]
+
+
+
+=== TEST 23:  invalid cache zone
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "invalid_disk_cache",
 
 Review comment:
   We can scan the `cache_zone` data from the local config file `conf/config.yaml`.
   
   We just check if the name is valid.
   
   https://github.com/apache/incubator-apisix/pull/1153/files#diff-82e3bcd74a1f6e15d0577acef2d4fa8cR41

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392629849
 
 

 ##########
 File path: t/plugin/proxy-cache.t
 ##########
 @@ -0,0 +1,591 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
+    if ($ENV{TEST_NGINX_CHECK_LEAK}) {
+        $SkipReason = "unavailable for the hup tests";
+
+    } else {
+        $ENV{TEST_NGINX_USE_HUP} = 1;
+        undef $ENV{TEST_NGINX_USE_STAP};
+    }
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+log_level('info');
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    my $http_config = $block->http_config // <<_EOC_;
+
+    # for proxy cache
+    proxy_cache_path /tmp/disk_cache_one levels=1:2 keys_zone=disk_cache_one:50m inactive=1d max_size=1G;
+    proxy_cache_path /tmp/disk_cache_two levels=1:2 keys_zone=disk_cache_two:50m inactive=1d max_size=1G;
+
+    # for proxy cache
+    map \$upstream_cache_zone \$upstream_cache_zone_info {
+        disk_cache_one /tmp/disk_cache_one,1:2;
+        disk_cache_two /tmp/disk_cache_two,1:2;
+    }
+
+    server {
+        listen 1986;
+        server_tokens off;
+
+        location / {
+            expires 60s;
+            return 200 "hello world!";
+        }
+
+        location /hello-not-found {
+            return 404;
+        }
+    }
+_EOC_
+
+    $block->set_value("http_config", $http_config);
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity check (missing required field)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: sanity check (invalid type for cache_method)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
 
 Review comment:
   `cache_zone` maybe wrong, need test case to cover this.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-595247413
 
 
   Do you have any suggestions for the test cases?
   
   Moreover, I think I should figure out the logic of the APISIX test cases.
   
   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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r386113243
 
 

 ##########
 File path: bin/apisix
 ##########
 @@ -410,6 +410,7 @@ http {
             set $upstream_hdr_cache_control     '';
 
             proxy_cache                         $upstream_cache_zone;
+            proxy_cache_valid                   502 504 0s;
 
 Review comment:
   Yes. here will be confusing. If 502 returned from upstream, the proxy_no_cache directive will take effect. But it returned from APISIX,  the proxy_no_cache directive will not take effect. 

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r385679589
 
 

 ##########
 File path: conf/config.yaml
 ##########
 @@ -35,6 +35,19 @@ apisix:
   #  enable_tcp_pp: true           # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
   #  enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
 
+  #proxy_cache:                     # Proxy Caching configuration
+  #  cache_ttl: 10s                 # The default caching time if the upstream does not specify the cache time
+  #   zone:                          # The parameters of a cache
 
 Review comment:
   `zones` is better

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r387403420
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,278 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "a key for caching",
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]]
+            },
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
+        },
+        hide_cache_headers = {
+            type = "boolean",
+            default = false,
+        },
+        cache_bypass = {
+            type = "array",
+            minItems = 1,
+            items = {
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]]
+            },
+        },
+        no_cache = {
+            type = "array",
+            minItems = 1,
+            items = {
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]]
+            },
+        },
+    },
+    required = {"cache_zone", "cache_key"},
+}
+
+local _M = {
+    version = 0.1,
+    priority = 1009,
+    name = plugin_name,
+    schema = schema,
+}
+
+
+function _M.check_schema(conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return false, err
+    end
+
+    return true
+end
+
+
+local tmp = {}
+local function generate_complex_value(data, ctx)
+    core.table.clear(tmp)
+
+    core.log.info("proxy-cache complex value: ", core.json.delay_encode(data))
+    for i, value in ipairs(data) do
+        core.log.info("proxy-cache complex value index-", i, ": ", value)
+
+        if string.sub(value, 1, 1) == "$" then
+            tab_insert(tmp, ctx.var[string.sub(value, 2)])
+        else
+            tab_insert(tmp, value)
+        end
+    end
+
+    return tab_concat(tmp, "")
+end
+
+
+-- check whether the request method and response status
+-- match the user defined.
+local function match_method_and_status(conf, ctx)
+    local match_method, match_status = false, false
+
+    -- Maybe there is no need for optimization here.
+    for _, method in ipairs(conf.cache_method) do
+        if method == ctx.var.request_method then
+            match_method = true
+            break
+        end
+    end
+
+    for _, status in ipairs(conf.cache_http_status) do
+        if status == ngx.status then
+            match_status = true
+            break
+        end
+    end
+
+    if match_method and match_status then
+        return true
+    end
+
+    return false
+end
+
+-- refer to https://gist.github.com/titpetric/ed6ec548af160e82c650cf39074878fb
+local function file_exists(name)
+    local f = io_open(name, "r")
+    if f~=nil then io_close(f) return true else return false end
+end
+
+
+local function explode(d, p)
+    local t, ll
+    t={}
+    ll=0
+    if(#p == 1) then return {p} end
+    while true do
+        local l=string.find(p, d, ll, true) -- find the next d in the string
+        if l~=nil then -- if "not not" found then..
+                tab_insert(t, string.sub(p, ll, l-1)) -- Save it in our array.
+                ll=l+1 -- save just after where we found it for searching next time.
+        else
+                tab_insert(t, string.sub(p, ll)) -- Save what's left in our array.
+                break -- Break at end, as it should be, according to the lua manual.
+        end
+    end
+    return t
+end
+
+
+local function generate_cache_filename(cache_path, cache_levels, cache_key)
+    local md5sum = ngx.md5(cache_key)
+    local levels = explode(":", cache_levels)
+    local filename = ""
+
+    local index = string.len(md5sum)
+    for k, v in pairs(levels) do
+            local length = tonumber(v)
+            -- add trailing [length] chars to index
+            index = index - length;
 
 Review comment:
   better remove `;` in the end of line

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-598047369
 
 
   Please review @moonming @membphis 

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-598163059
 
 
   OK. I'll add it to README. Basically test cases are covered. 

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r386937456
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -178,6 +158,91 @@ local function match_method_and_status(conf, ctx)
     return false
 end
 
+-- refer to https://gist.github.com/titpetric/ed6ec548af160e82c650cf39074878fb
+local function file_exists(name)
+    local f = io_open(name, "r")
+    if f~=nil then io_close(f) return true else return false end
+end
+
+
+local function explode(d, p)
+    local t, ll
+    t={}
+    ll=0
+    if(#p == 1) then return {p} end
+    while true do
+        local l=string.find(p, d, ll, true) -- find the next d in the string
+        if l~=nil then -- if "not not" found then..
+                tab_insert(t, string.sub(p, ll, l-1)) -- Save it in our array.
+                ll=l+1 -- save just after where we found it for searching next time.
+        else
+                tab_insert(t, string.sub(p, ll)) -- Save what's left in our array.
+                break -- Break at end, as it should be, according to the lua manual.
+        end
+    end
+    return t
+end
+
+
+local function generate_cache_filename(cache_path, cache_levels, cache_key)
+    local md5sum = ngx.md5(cache_key)
+    local levels = explode(":", cache_levels)
+    local filename = ""
+
+    local index = string.len(md5sum)
+    for k, v in pairs(levels) do
+            local length = tonumber(v)
+            -- add trailing [length] chars to index
+            index = index - length;
+            filename = filename .. md5sum:sub(index+1, index+length) .. "/";
+    end
+    if cache_path:sub(-1) ~= "/" then
+            cache_path = cache_path .. "/";
+    end
+    filename = cache_path .. filename .. md5sum
+    return filename
+end
+
+
+local function cache_purge(conf, ctx)
+    local cache_zone_info = ngx_re.split(ctx.var.upstream_cache_zone_info, ",")
+
+    local filename = generate_cache_filename(cache_zone_info[1], cache_zone_info[2],
+                                             ctx.var.upstream_cache_key)
+    if file_exists(filename) then
+        os.remove(filename)
 
 Review comment:
   Don't worry. If the worker has permission to create it, then it has permission to delete it.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-593466068
 
 
   > And I have other new questions:
   > 
   > 1. need some e2e test case
   > 2. need a way to purge the old cache
   
   1. e2e test cases I haven't figured it out yet. Do you have any suggestions?
   2. Added.  Users only need to specify the method=PURGE to delete cache file. Please review it.
   
   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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] moonming commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
moonming commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-598144119
 
 
   LGTM. Need add this plugin to README.md. 
   @membphis please take a look about how to add test cases.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r386070744
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -186,22 +186,27 @@ function _M.header_filter(conf, ctx)
     core.log.info("proxy-cache plugin header filter phase, conf: ", core.json.delay_encode(conf))
 
     local no_cache = "1"
+    local match_method, match_status = false, false
 
 Review comment:
   We can move this check logic into a single function, which will make the code simpler.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r385714849
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
 
 Review comment:
   YES. I agree with you. 
   But I wonder if this style can be unified in all plugins. Here we can extract the common functions for other plugins to use.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392666263
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,259 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "a key for caching",
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]],
+            },
+            default = {"$host", "$request_uri"}
 
 Review comment:
   The response data of the HEAD and GET methods are different.
   
   So I think we better add `method` to the cache key.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r385677585
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
 
 Review comment:
   I think we can use an array object instead of a string object which is easier.
   
   `{"host", "uri", "args"}` can be the default value.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392673846
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,259 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "a key for caching",
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]],
+            },
+            default = {"$host", "$request_uri"}
 
 Review comment:
   ok, you are right

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392658133
 
 

 ##########
 File path: t/plugin/proxy-cache.t
 ##########
 @@ -0,0 +1,630 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+log_level('info');
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    my $http_config = $block->http_config // <<_EOC_;
+
+    # for proxy cache
+    proxy_cache_path /tmp/disk_cache_one levels=1:2 keys_zone=disk_cache_one:50m inactive=1d max_size=1G;
+    proxy_cache_path /tmp/disk_cache_two levels=1:2 keys_zone=disk_cache_two:50m inactive=1d max_size=1G;
+
+    # for proxy cache
+    map \$upstream_cache_zone \$upstream_cache_zone_info {
+        disk_cache_one /tmp/disk_cache_one,1:2;
+        disk_cache_two /tmp/disk_cache_two,1:2;
+    }
+
+    server {
+        listen 1986;
+        server_tokens off;
+
+        location / {
+            expires 60s;
+            return 200 "hello world!";
+        }
+
+        location /hello-not-found {
+            return 404;
+        }
+    }
+_EOC_
+
+    $block->set_value("http_config", $http_config);
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity check (missing required field)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: sanity check (invalid type for cache_method)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": "GET",
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: sanity check (invalid type for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": "${uri}-cache-key",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: sanity check (invalid type for cache_bypass)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": "$arg_bypass",
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: sanity check (invalid type for no_cache)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": "$arg_no_cache"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: sanity check (illegal character for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": ["$uri-", "-cache-id"],
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: sanity check (normal case)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: hit route (cache miss)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: hit route (cache hit)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: hit route (cache bypass)
+--- request
+GET /hello?bypass=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: BYPASS
+--- no_error_log
+[error]
+
+
+
+=== TEST 11: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 12: hit route (nocache)
+--- request
+GET /hello?no_cache=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: hit route (there's no cache indeed)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: hit route (will be cached)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: hit route (not found)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: hit route (404 there's no cache indeed)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: hit route (HEAD method)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 18: hit route (HEAD method there's no cache)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 19:  hide cache headers = false
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": false,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 20: hit route (catch the cache headers)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- response_headers_like
+Cache-Control:
+--- no_error_log
+[error]
+
+
+
+=== TEST 21: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: purge cache (not found)
+--- request
+PURGE /hello-world
+--- error_code: 404
+--- no_error_log
+[error]
+
+
+
+=== TEST 23:  invalid cache zone
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "invalid_disk_cache",
 
 Review comment:
   We should throw an error message directly when the user tries to set an invalid `cache zone`.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-593967942
 
 
   > > And I have other new questions:
   > > 
   > > 1. need some e2e test case
   > > 2. need a way to purge the old cache
   > 
   > 1. e2e test cases I haven't figured it out yet. Do you have any suggestions?
   > 2. Added.  Users only need to specify the method=PURGE to delete cache file. Please review it.
   > 
   > Thanks.
   
   @moonming @membphis 

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392629668
 
 

 ##########
 File path: t/plugin/proxy-cache.t
 ##########
 @@ -0,0 +1,591 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
 
 Review comment:
   https://github.com/apache/incubator-apisix/pull/1153/files#diff-c7cd747d5292a96708759980b770f3f0R17-R25
   
   I think we can remove this block, it is useless.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] moonming commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
moonming commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-591413643
 
 
   how to add test cases for this plugin?

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392657923
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -178,6 +158,91 @@ local function match_method_and_status(conf, ctx)
     return false
 end
 
+-- refer to https://gist.github.com/titpetric/ed6ec548af160e82c650cf39074878fb
+local function file_exists(name)
+    local f = io_open(name, "r")
+    if f~=nil then io_close(f) return true else return false end
+end
+
+
+local function explode(d, p)
+    local t, ll
+    t={}
+    ll=0
+    if(#p == 1) then return {p} end
+    while true do
+        local l=string.find(p, d, ll, true) -- find the next d in the string
+        if l~=nil then -- if "not not" found then..
+                tab_insert(t, string.sub(p, ll, l-1)) -- Save it in our array.
+                ll=l+1 -- save just after where we found it for searching next time.
+        else
+                tab_insert(t, string.sub(p, ll)) -- Save what's left in our array.
+                break -- Break at end, as it should be, according to the lua manual.
+        end
+    end
+    return t
+end
+
+
+local function generate_cache_filename(cache_path, cache_levels, cache_key)
+    local md5sum = ngx.md5(cache_key)
+    local levels = explode(":", cache_levels)
+    local filename = ""
+
+    local index = string.len(md5sum)
+    for k, v in pairs(levels) do
+            local length = tonumber(v)
+            -- add trailing [length] chars to index
+            index = index - length;
+            filename = filename .. md5sum:sub(index+1, index+length) .. "/";
+    end
+    if cache_path:sub(-1) ~= "/" then
+            cache_path = cache_path .. "/";
+    end
+    filename = cache_path .. filename .. md5sum
+    return filename
+end
+
+
+local function cache_purge(conf, ctx)
+    local cache_zone_info = ngx_re.split(ctx.var.upstream_cache_zone_info, ",")
+
+    local filename = generate_cache_filename(cache_zone_info[1], cache_zone_info[2],
+                                             ctx.var.upstream_cache_key)
+    if file_exists(filename) then
+        os.remove(filename)
 
 Review comment:
   that 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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-598996405
 
 
   @membphis  ping

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r385541656
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
+            minLength = 1
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+                default = {"GET", "HEAD"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
+        },
+        hide_cache_headers = {
+            type = "boolean",
+            default = false,
+        },
+        cache_strategy = {
+            type = "string",
+            default = "disk",
+            enum = {"disk", "memory"},
+            minLength = 0
+        },
+        cache_bypass = {
+            type = "string",
+            default = "1",
+            minLength = 0
+        },
+        no_cache = {
+            type = "string",
+            default = "0",
+            minLength = 0
+        },
+    },
+    required = {"cache_zone", "cache_key"},
+}
+
+local _M = {
+    version = 0.1,
+    priority = 1009,
+    name = plugin_name,
+    schema = schema,
+}
+
+function _M.check_schema(conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return false, err
+    end
+
+    if conf.cache_strategy == "memory" then
+        return false, "memory cache is not yet supported."
+    end
+
+    return true
+end
+
+-- Copy from redirect plugin, this function is useful.
+-- It can be extracted as a public function.
+local function parse_complex_value(complex_value)
+
+    local reg = [[ (\\\$[0-9a-zA-Z_]+) | ]]     -- \$host
+            .. [[ \$\{([0-9a-zA-Z_]+)\} | ]]    -- ${host}
+            .. [[ \$([0-9a-zA-Z_]+) | ]]        -- $host
+            .. [[ (\$|[^$\\]+) ]]               -- $ or others
+    local iterator, err = re_gmatch(complex_value, reg, "jiox")
+    if not iterator then
+        return nil, err
+    end
+
+    local t = {}
+    while true do
+        local m, err = iterator()
+        if err then
+            return nil, err
+        end
+
+        if not m then
+            break
+        end
+
+        tab_insert(t, m)
+    end
+
+    return t
+end
+
+
+local tmp = {}
+local function generate_complex_value(data, ctx)
 
 Review comment:
   why we need this fuction?

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis merged pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis merged pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153
 
 
   

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392672970
 
 

 ##########
 File path: t/plugin/proxy-cache.t
 ##########
 @@ -0,0 +1,630 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+log_level('info');
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    my $http_config = $block->http_config // <<_EOC_;
+
+    # for proxy cache
+    proxy_cache_path /tmp/disk_cache_one levels=1:2 keys_zone=disk_cache_one:50m inactive=1d max_size=1G;
+    proxy_cache_path /tmp/disk_cache_two levels=1:2 keys_zone=disk_cache_two:50m inactive=1d max_size=1G;
+
+    # for proxy cache
+    map \$upstream_cache_zone \$upstream_cache_zone_info {
+        disk_cache_one /tmp/disk_cache_one,1:2;
+        disk_cache_two /tmp/disk_cache_two,1:2;
+    }
+
+    server {
+        listen 1986;
+        server_tokens off;
+
+        location / {
+            expires 60s;
+            return 200 "hello world!";
+        }
+
+        location /hello-not-found {
+            return 404;
+        }
+    }
+_EOC_
+
+    $block->set_value("http_config", $http_config);
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity check (missing required field)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: sanity check (invalid type for cache_method)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": "GET",
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: sanity check (invalid type for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": "${uri}-cache-key",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: sanity check (invalid type for cache_bypass)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": "$arg_bypass",
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: sanity check (invalid type for no_cache)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": "$arg_no_cache"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: sanity check (illegal character for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": ["$uri-", "-cache-id"],
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: sanity check (normal case)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: hit route (cache miss)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: hit route (cache hit)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: hit route (cache bypass)
+--- request
+GET /hello?bypass=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: BYPASS
+--- no_error_log
+[error]
+
+
+
+=== TEST 11: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 12: hit route (nocache)
+--- request
+GET /hello?no_cache=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: hit route (there's no cache indeed)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: hit route (will be cached)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: hit route (not found)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: hit route (404 there's no cache indeed)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: hit route (HEAD method)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 18: hit route (HEAD method there's no cache)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 19:  hide cache headers = false
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": false,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 20: hit route (catch the cache headers)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- response_headers_like
+Cache-Control:
+--- no_error_log
+[error]
+
+
+
+=== TEST 21: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: purge cache (not found)
+--- request
+PURGE /hello-world
+--- error_code: 404
+--- no_error_log
+[error]
+
+
+
+=== TEST 23:  invalid cache zone
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "invalid_disk_cache",
 
 Review comment:
   @membphis  OK. Does the parsing way refer to the bin/apisix file? 
   
   

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392669384
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,259 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "a key for caching",
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]],
+            },
+            default = {"$host", "$request_uri"}
 
 Review comment:
   Currently the default behavior is to convert HEAD to GET request.  See also: http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cache_convert_head
   
   The conversation is the default behavior before introducing the proxy_cache_convert_header directive.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r386070858
 
 

 ##########
 File path: bin/apisix
 ##########
 @@ -410,6 +410,7 @@ http {
             set $upstream_hdr_cache_control     '';
 
             proxy_cache                         $upstream_cache_zone;
+            proxy_cache_valid                   502 504 0s;
 
 Review comment:
   We already have the "cache_http_status" property, so I think we can delete this line.

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392659145
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,259 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local ngx_re = require("ngx.re")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local string = string
+local io_open = io.open
+local io_close = io.close
+local ngx = ngx
+local os = os
+local ipairs = ipairs
+local pairs = pairs
+local tonumber = tonumber
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "a key for caching",
+                type = "string",
+                pattern = [[(^[^\$].+$|^\$[0-9a-zA-Z_]+$)]],
+            },
+            default = {"$host", "$request_uri"}
 
 Review comment:
   Method can be specified by the cache_method. How about the user control this?

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r392660022
 
 

 ##########
 File path: t/plugin/proxy-cache.t
 ##########
 @@ -0,0 +1,630 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+log_level('info');
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    my $http_config = $block->http_config // <<_EOC_;
+
+    # for proxy cache
+    proxy_cache_path /tmp/disk_cache_one levels=1:2 keys_zone=disk_cache_one:50m inactive=1d max_size=1G;
+    proxy_cache_path /tmp/disk_cache_two levels=1:2 keys_zone=disk_cache_two:50m inactive=1d max_size=1G;
+
+    # for proxy cache
+    map \$upstream_cache_zone \$upstream_cache_zone_info {
+        disk_cache_one /tmp/disk_cache_one,1:2;
+        disk_cache_two /tmp/disk_cache_two,1:2;
+    }
+
+    server {
+        listen 1986;
+        server_tokens off;
+
+        location / {
+            expires 60s;
+            return 200 "hello world!";
+        }
+
+        location /hello-not-found {
+            return 404;
+        }
+    }
+_EOC_
+
+    $block->set_value("http_config", $http_config);
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity check (missing required field)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: sanity check (invalid type for cache_method)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": "GET",
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: sanity check (invalid type for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": "${uri}-cache-key",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: sanity check (invalid type for cache_bypass)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": "$arg_bypass",
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: sanity check (invalid type for no_cache)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": "$arg_no_cache"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: sanity check (illegal character for cache_key)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_key": ["$uri-", "-cache-id"],
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1985": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 400
+--- response_body eval
+qr/failed to check the configuration of plugin proxy-cache/
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: sanity check (normal case)
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": true,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: hit route (cache miss)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: hit route (cache hit)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: hit route (cache bypass)
+--- request
+GET /hello?bypass=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: BYPASS
+--- no_error_log
+[error]
+
+
+
+=== TEST 11: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 12: hit route (nocache)
+--- request
+GET /hello?no_cache=1
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 13: hit route (there's no cache indeed)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: MISS
+--- raw_response_headers_unlike
+Expires:
+--- no_error_log
+[error]
+
+
+
+=== TEST 14: hit route (will be cached)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: hit route (not found)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: hit route (404 there's no cache indeed)
+--- request
+GET /hello-not-found
+--- error_code: 404
+--- response_body eval
+qr/404 Not Found/
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: hit route (HEAD method)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 18: hit route (HEAD method there's no cache)
+--- request
+HEAD /hello-world
+--- error_code: 200
+--- response_headers
+Apisix-Cache-Status: MISS
+--- no_error_log
+[error]
+
+
+
+=== TEST 19:  hide cache headers = false
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "disk_cache_one",
+                               "cache_bypass": ["$arg_bypass"],
+                               "cache_method": ["GET"],
+                               "cache_http_status": [200],
+                               "hide_cache_headers": false,
+                               "no_cache": ["$arg_no_cache"]
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1986": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/hello*"
+                   }]]
+                   )
+
+               if code >= 300 then
+                   ngx.status = code
+               end
+               ngx.say(body)
+           }
+       }
+--- request
+GET /t
+--- error_code: 200
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 20: hit route (catch the cache headers)
+--- request
+GET /hello
+--- response_body chop
+hello world!
+--- response_headers
+Apisix-Cache-Status: HIT
+--- response_headers_like
+Cache-Control:
+--- no_error_log
+[error]
+
+
+
+=== TEST 21: purge cache
+--- request
+PURGE /hello
+--- error_code: 200
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: purge cache (not found)
+--- request
+PURGE /hello-world
+--- error_code: 404
+--- no_error_log
+[error]
+
+
+
+=== TEST 23:  invalid cache zone
+--- 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": {
+                            "proxy-cache": {
+                               "cache_zone": "invalid_disk_cache",
 
 Review comment:
   Any suggestions?

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-590082018
 
 
   Currently only supports the upstream dynamically specify the cache time. If the upstream does not specify it such as the header `Expires` or `Cache-Control`, the `cache_ttl` in the config.yaml will be used.  And the option `hide_cache_headers ` will be decide whether the upstream header `Expires` and `Cache-Control` is sent to the client.
   It's just a trick. Hopefully you can understand it.
   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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r384856556
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
+            minLength = 1
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+                default = {"GET", "HEAD"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
+        },
+        hide_cache_headers = {
+            type = "boolean",
+            default = false,
+        },
+        cache_strategy = {
+            type = "string",
+            default = "disk",
+            enum = {"disk", "memory"},
+            minLength = 0
+        },
+        cache_bypass = {
+            type = "string",
+            default = "1",
+            minLength = 0
+        },
+        no_cache = {
+            type = "string",
+            default = "0",
+            minLength = 0
+        },
+    },
+    required = {"cache_zone", "cache_key"},
+}
+
+local _M = {
+    version = 0.1,
+    priority = 1009,
+    name = plugin_name,
+    schema = schema,
+}
+
+function _M.check_schema(conf)
+    local ok, err = core.schema.check(schema, conf)
+    if not ok then
+        return false, err
+    end
+
+    if conf.cache_strategy == "memory" then
+        return false, "memory cache is not yet supported."
+    end
+
+    return true
+end
+
+-- Copy from redirect plugin, this function is useful.
+-- It can be extracted as a public function.
+local function parse_complex_value(complex_value)
+
+    local reg = [[ (\\\$[0-9a-zA-Z_]+) | ]]     -- \$host
+            .. [[ \$\{([0-9a-zA-Z_]+)\} | ]]    -- ${host}
+            .. [[ \$([0-9a-zA-Z_]+) | ]]        -- $host
+            .. [[ (\$|[^$\\]+) ]]               -- $ or others
+    local iterator, err = re_gmatch(complex_value, reg, "jiox")
+    if not iterator then
+        return nil, err
+    end
+
+    local t = {}
+    while true do
+        local m, err = iterator()
+        if err then
+            return nil, err
+        end
+
+        if not m then
+            break
+        end
+
+        tab_insert(t, m)
+    end
+
+    return t
+end
+
+
+local tmp = {}
+local function generate_complex_value(data, ctx)
+    local segs_value, err = lrucache(data, nil, parse_complex_value, data)
+    if not segs_value then
+        return nil, err
+    end
+
+    core.table.clear(tmp)
+
+    for i, value in ipairs(segs_value) do
+        core.log.info("complex value(", data, ") seg-", i, ": ", core.json.delay_encode(value))
+
+        local pat1 = value[1]    -- \$host
+        local pat2 = value[2]    -- ${host}
+        local pat3 = value[3]    -- $host
+        local pat4 = value[4]    -- $ or others
+
+        if pat2 or pat3 then
+            tab_insert(tmp, ctx.var[pat2 or pat3])
+        else
+            tab_insert(tmp, pat1 or pat4)
+        end
+    end
+
+    return tab_concat(tmp, "")
+end
+
+
+function _M.rewrite(conf, ctx)
+    core.log.info("proxy-cache plugin rewrite phase, conf: ", core.json.delay_encode(conf))
+
+    ctx.var.upstream_cache_zone = conf.cache_zone
+
+    local value, err = generate_complex_value(conf.cache_key, ctx)
+    if not value then
+        core.log.error("failed to generate the complex value by: ", conf.cache_key, " error: ", err)
+        core.response.exit(500)
+    end
+
+    ctx.var.upstream_cache_key = value
+    core.log.info("proxy-cache cache key value:", value)
+
+    local value, err = generate_complex_value(conf.cache_bypass, ctx)
+    if not value then
+        core.log.error("failed to generate the complex value by: ",
+                       conf.cache_bypass, " error: ", err)
+        core.response.exit(500)
+    end
+
+    ctx.var.upstream_cache_bypass = value
+    core.log.info("proxy-cache cache bypass value:", value)
+end
+
+
+function _M.header_filter(conf, ctx)
+    core.log.info("proxy-cache plugin header filter phase, conf: ", core.json.delay_encode(conf))
+
+    local no_cache = "1"
+
+    -- Maybe there is no need for optimization here.
+    for _, method in ipairs(conf.cache_method) do
+        if method == ctx.var.request_method then
+            no_cache = "0"
+        end
+    end
+
+    for _, status in ipairs(conf.cache_http_status) do
+        if status == ngx.status then
+            no_cache = "0"
 
 Review comment:
   ditto

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r385678375
 
 

 ##########
 File path: lua/apisix/plugins/proxy-cache.lua
 ##########
 @@ -0,0 +1,238 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local tab_insert = table.insert
+local tab_concat = table.concat
+local re_gmatch = ngx.re.gmatch
+local ngx = ngx
+local ipairs = ipairs
+
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 100
+})
+
+local plugin_name = "proxy-cache"
+
+local schema = {
+    type = "object",
+    properties = {
+        cache_zone = {
+            type = "string",
+            minLength = 1
+        },
+        cache_key = {
+            type = "string",
+            minLength = 1
+        },
+        cache_http_status = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http response status",
+                type = "integer",
+                minimum = 200,
+                maximum = 599,
+            },
+            uniqueItems = true,
+            default = {200, 301, 404},
+        },
+        cache_method = {
+            type = "array",
+            minItems = 1,
+            items = {
+                description = "http method",
+                type = "string",
+                enum = {"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD",
+                    "OPTIONS", "CONNECT", "TRACE"},
+                default = {"GET", "HEAD"},
+            },
+            uniqueItems = true,
+            default = {"GET", "HEAD"},
+        },
+        hide_cache_headers = {
+            type = "boolean",
+            default = false,
+        },
+        cache_strategy = {
 
 Review comment:
   we can drop it first, add this feature later

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on issue #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#issuecomment-591469470
 
 
   Hi 
   
   Tests cases I think the t/servroot/conf/nginx.conf file should be modified.
   
   In recent days, i have been preparing to make a presentation. So I'm gonna add tests in this weekend. 
   And are there any other problems with code implementation? 
   
   

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


With regards,
Apache Git Services

[GitHub] [incubator-apisix] agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.

Posted by GitBox <gi...@apache.org>.
agile6v commented on a change in pull request #1153: feature: support for proxy caching plugin based on disk.
URL: https://github.com/apache/incubator-apisix/pull/1153#discussion_r386113395
 
 

 ##########
 File path: bin/apisix
 ##########
 @@ -410,6 +410,7 @@ http {
             set $upstream_hdr_cache_control     '';
 
             proxy_cache                         $upstream_cache_zone;
+            proxy_cache_valid                   502 504 0s;
 
 Review comment:
   I will remove it and explain why in the document.

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


With regards,
Apache Git Services