Skip to content

Commit

Permalink
[TT-12454] Extract ApplyPolicies into internal/policy scope (#6367)
Browse files Browse the repository at this point in the history
### **User description**
This extracts a large problematic `ApplyPolicies` function into it's own
package scope. It does this by:

- defining an interface for policy access and deleting inline mutex use
- updated *Gateway to implement storage interface
- implement storage interface in policy.Store as well (used inline)

On top of that:

- Fixes a bug in the Duration/Less() functions on user.APILimit, fixes
tests
- Adds a test particular to ApplyRateLimits (decrease coginitive
complexity)

The duration was calculated as rate/per, however, the correct way was
per/rate;
This fixes it so duration is calculated correctly, fixing the Less
function comparison.

___

### **PR Type**
Enhancement, Bug fix


___

### **Description**
- Refactored `handleGetPolicy` to use the new `PolicyByID` method.
- Introduced a `Repository` interface and added methods `PolicyIDs`,
`PolicyByID`, and `PolicyCount` to the `Gateway` struct.
- Refactored `ApplyPolicies` in `BaseMiddleware` to use the new `policy`
package.
- Updated `buildNodeInfo` to use `PolicyCount` instead of
`policiesByIDLen`.
- Removed redundant methods `getPolicy` and `policiesByIDLen` from
`Gateway`.


___



### **Changes walkthrough** 📝
<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Enhancement
</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>api.go</strong><dd><code>Refactor policy retrieval in
`handleGetPolicy`.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

gateway/api.go
- Replaced `getPolicy` with `PolicyByID` in `handleGetPolicy`.



</details>
    

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6367/files#diff-644cda3aeb4ac7f325359e85fcddb810f100dd5e6fa480b0d9f9363a743c4e05">+1/-1</a>&nbsp;
&nbsp; &nbsp; </td>
</tr>                    

<tr>
  <td>
    <details>
<summary><strong>gateway.go</strong><dd><code>Add policy-related methods
and interface to Gateway.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
</dd></summary>
<hr>

gateway/gateway.go
<li>Introduced <code>Repository</code> interface.<br> <li> Added methods
<code>PolicyIDs</code>, <code>PolicyByID</code>, and
<code>PolicyCount</code>.<br>


</details>
    

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6367/files#diff-17cb8b37eda9018fe1c6cdb5f96b3fc948fc8ba49bc516987b8269576db9fcd4">+38/-0</a>&nbsp;
&nbsp; </td>
</tr>                    

<tr>
  <td>
    <details>
<summary><strong>middleware.go</strong><dd><code>Refactor ApplyPolicies
to use new policy store.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

gateway/middleware.go
<li>Removed <code>clearSession</code> method.<br> <li> Refactored
<code>ApplyPolicies</code> to use <code>policy.New</code> and
<code>store.Apply</code>.<br>


</details>
    

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6367/files#diff-703054910891a4db633eca0f42ed779d6b4fa75cd9b3aa4c503e681364201c1b">+3/-411</a>&nbsp;
</td>
</tr>                    

<tr>
  <td>
    <details>
<summary><strong>rpc_storage_handler.go</strong><dd><code>Update policy
count retrieval in buildNodeInfo.</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

gateway/rpc_storage_handler.go
- Replaced `policiesByIDLen` with `PolicyCount` in `buildNodeInfo`.



</details>
    

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6367/files#diff-8875f75b602664c44b62b67a4da41d748124ad270573a44db4ec977ee5d68021">+1/-1</a>&nbsp;
&nbsp; &nbsp; </td>
</tr>                    

<tr>
  <td>
    <details>
<summary><strong>server.go</strong><dd><code>Remove redundant policy
methods from Gateway.</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

gateway/server.go
- Removed `getPolicy` and `policiesByIDLen` methods.



</details>
    

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6367/files#diff-4652d1bf175a0be8f5e61ef7177c9666f23e077d8626b73ac9d13358fa8b525b">+0/-13</a>&nbsp;
&nbsp; </td>
</tr>                    
</table></td></tr></tr></tbody></table>

___

> 💡 **PR-Agent usage**:
>Comment `/help` on the PR to get a list of all available PR-Agent tools
and their descriptions

---------

Co-authored-by: Tit Petric <tit@tyk.io>
  • Loading branch information
titpetric and Tit Petric committed Jun 26, 2024
1 parent 656095e commit 2dfcc3b
Show file tree
Hide file tree
Showing 17 changed files with 875 additions and 479 deletions.
2 changes: 1 addition & 1 deletion gateway/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ func (gw *Gateway) handleRemoveSortedSetRange(keyName, scoreFrom, scoreTo string
}

func (gw *Gateway) handleGetPolicy(polID string) (interface{}, int) {
if pol := gw.getPolicy(polID); pol.ID != "" {
if pol, ok := gw.PolicyByID(polID); ok && pol.ID != "" {
return pol, http.StatusOK
}

Expand Down
38 changes: 38 additions & 0 deletions gateway/gateway.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package gateway

import (
"github.com/TykTechnologies/tyk/internal/policy"
"github.com/TykTechnologies/tyk/user"
)

type Repository interface {
policy.Repository
}

var _ Repository = &Gateway{}

func (gw *Gateway) PolicyIDs() []string {
gw.policiesMu.RLock()
defer gw.policiesMu.RUnlock()

result := make([]string, 0, len(gw.policiesByID))
for id := range gw.policiesByID {
result = append(result, id)
}
return result
}

func (gw *Gateway) PolicyByID(polID string) (user.Policy, bool) {
gw.policiesMu.RLock()
defer gw.policiesMu.RUnlock()

pol, ok := gw.policiesByID[polID]
return pol, ok
}

func (gw *Gateway) PolicyCount() int {
gw.policiesMu.RLock()
defer gw.policiesMu.RUnlock()

return len(gw.policiesByID)
}
Loading

0 comments on commit 2dfcc3b

Please sign in to comment.