Spaces + rate limits #11
Conversation
WalkthroughThe recent changes significantly enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfluenceClient
participant API
User->>ConfluenceClient: Request Space List
ConfluenceClient->>API: GET /spaces
API-->>ConfluenceClient: Return Space Data
ConfluenceClient-->>User: List of Spaces
sequenceDiagram
participant User
participant ConfluenceClient
participant API
User->>ConfluenceClient: Manage Space Permissions
ConfluenceClient->>API: POST /spaces/{spaceId}/permissions
API-->>ConfluenceClient: Confirm Permission Change
ConfluenceClient-->>User: Permission Updated
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
README.mdis excluded by none and included by none
Files selected for processing (8)
- pkg/connector/client/confluence.go (6 hunks)
- pkg/connector/client/models.go (1 hunks)
- pkg/connector/client/path.go (2 hunks)
- pkg/connector/client/ratelimit.go (1 hunks)
- pkg/connector/client/request.go (1 hunks)
- pkg/connector/connector.go (2 hunks)
- pkg/connector/spaces.go (1 hunks)
- pkg/connector/spaces_test.go (1 hunks)
Additional comments not posted (51)
pkg/connector/client/ratelimit.go (1)
27-45: LGTM!The function correctly checks for rate limit status and status codes.
pkg/connector/spaces_test.go (2)
14-50: LGTM!The test case correctly sets up the context, client, and server, and includes appropriate assertions for listing spaces.
52-63: LGTM!The sub-test correctly sets up the space and includes appropriate assertions for listing grants.
pkg/connector/client/request.go (4)
15-21: LGTM!The function correctly makes a GET request using the
makeRequesthelper function.
23-30: LGTM!The function correctly makes a POST request using the
makeRequesthelper function.
32-38: LGTM!The function correctly makes a DELETE request using the
makeRequesthelper function.
40-86: LGTM!The function correctly handles the details of making an HTTP request, including rate limit handling and error processing.
pkg/connector/connector.go (2)
33-37: LGTM!The definition of
spaceResourceTypeis consistent with other resource types.
102-102: LGTM! But verify the implementation ofnewSpaceBuilder.The addition of
newSpaceBuilder(c.client)is correctly placed.However, ensure that the implementation of
newSpaceBuilderis correct and aligns with the expected functionality.Verification successful
Implementation of
newSpaceBuilderandspaceBuilderlooks correct.
- The
newSpaceBuilderfunction initializes aspaceBuilderstruct with the provided client.- The
spaceBuilderstruct and its methods (ResourceTypeandList) are correctly defined and align with the expected functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `newSpaceBuilder`. # Test: Search for the function definition. Expect: Correct implementation of `newSpaceBuilder`. rg --type go 'func newSpaceBuilder' -A 10Length of output: 614
Script:
#!/bin/bash # Description: Verify the `spaceBuilder` struct and its methods. # Search for the definition of `spaceBuilder`. rg --type go 'type spaceBuilder struct' -A 10 # Search for methods associated with `spaceBuilder`. rg --type go 'func (s \*spaceBuilder)' -A 10Length of output: 646
pkg/connector/client/path.go (2)
14-14: LGTM!The removal of the duplicate declaration of
SearchUrlPatheliminates redundancy.
Line range hint
59-66: LGTM! But verify the usage ofwithPaginationCursor.The change in visibility of
WithPaginationCursortowithPaginationCursoris correct for internal use.However, ensure that the function is used correctly within the package.
Verification successful
The function
withPaginationCursoris used correctly within the package.The change in visibility to make it internal does not negatively impact its functionality, and it is consistently used to pass pagination parameters to the
c.parsemethod.
pkg/connector/client/confluence.go: Verified multiple usages ofwithPaginationCursorwithin thec.parsemethod calls.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `withPaginationCursor`. # Test: Search for the function usage. Expect: Correct usage of `withPaginationCursor`. rg --type go 'withPaginationCursor'Length of output: 536
Script:
#!/bin/bash # Description: Extract the surrounding lines of the usages of `withPaginationCursor` in `pkg/connector/client/confluence.go`. # Test: Extract context lines around the usage of `withPaginationCursor`. rg --type go --context 5 'withPaginationCursor' pkg/connector/client/confluence.goLength of output: 1020
pkg/connector/client/models.go (14)
58-61: LGTM!The definition of
ConfluenceSpaceDescriptionValueis consistent with other types.
63-65: LGTM!The definition of
ConfluenceSpaceDescriptionis consistent with other types.
67-70: LGTM!The definition of
ConfluenceMetais consistent with other types.
72-76: LGTM!The definition of
ConfluenceSpaceOperationsResponseis consistent with other types.
78-81: LGTM!The definition of
ConfluenceSpacePermissionResponseis consistent with other types.
83-87: LGTM!The definition of
ConfluenceSpacePermissionis consistent with other types.
89-92: LGTM!The definition of
ConfluenceSpacePermissionPrincipalis consistent with other types.
94-97: LGTM!The definition of
ConfluenceSpaceOperationis consistent with other types.
99-102: LGTM!The definition of
ConfluenceSpacePermissionOperationis consistent with other types.
104-116: LGTM!The definition of
ConfluenceSpaceis consistent with other types.
118-121: LGTM!The definition of
confluenceSpaceListis consistent with other types.
123-126: LGTM!The definition of
SpacePermissionSubjectis consistent with other types.
128-131: LGTM!The definition of
SpacePermissionOperationis consistent with other types.
133-136: LGTM!The definition of
CreateSpacePermissionRequestBodyis consistent with other types.pkg/connector/spaces.go (10)
21-28: LGTM!The function correctly generates an entitlement name by concatenating operation details with a separator.
30-34: LGTM!The function correctly splits an entitlement name into its components.
40-42: LGTM!The function correctly returns the resource type for spaces.
44-75: LGTM!The function correctly lists all spaces as resource objects, handles pagination, and converts spaces to resources. Error handling and rate limit annotations are correctly implemented.
78-131: LGTM!The function correctly retrieves entitlements for a given resource, handles pagination, and converts permissions to entitlements. Error handling, rate limit annotations, and logging are appropriately implemented.
134-187: LGTM!The function correctly retrieves grants for a given resource, handles pagination, and converts permissions to grants. Error handling and rate limit annotations are correctly implemented.
190-207: LGTM!The function correctly grants a permission to a principal. Error handling and rate limit annotations are correctly implemented.
209-225: LGTM!The function correctly revokes a permission from a principal. Error handling and rate limit annotations are correctly implemented.
227-231: LGTM!The function correctly creates a new
spaceBuilderinstance.
233-244: LGTM!The function correctly converts a Confluence space to a resource. Error handling is correctly implemented.
pkg/connector/client/confluence.go (16)
Line range hint
76-85:
LGTM!The function correctly verifies the current user by retrieving user information and checking for validity. Error handling is correctly implemented.
Line range hint
109-122:
LGTM!The function correctly retrieves groups with pagination, handles pagination, and converts the response to a list of groups. Error handling and rate limit annotations are correctly implemented.
Line range hint
178-191:
LGTM!The function correctly adds a user to a group by constructing the URL, preparing the request body, and sending a POST request. Error handling and rate limit annotations are correctly implemented.
Line range hint
208-217:
LGTM!The function correctly removes a user from a group by constructing the URL and sending a DELETE request. Error handling and rate limit annotations are correctly implemented.
226-234: LGTM!The function correctly increments a pagination token by converting it to an integer, incrementing it by the count, and returning the new token as a string. Error handling is correctly implemented.
237-242: LGTM!The function correctly converts a string to an integer and returns 0 if there's an error. Error handling is correctly implemented.
245-273: LGTM!The function correctly retrieves spaces with pagination, handles pagination, and converts the response to a list of spaces. Error handling and rate limit annotations are correctly implemented.
276-314: LGTM!The function correctly retrieves operations for a specific space with pagination, handles pagination, and converts the response to a list of operations. Error handling, rate limit annotations, and logging are correctly implemented.
317-349: LGTM!The function correctly retrieves permissions for a specific space with pagination, handles pagination, and converts the response to a list of permissions. Error handling and rate limit annotations are correctly implemented.
352-363: LGTM!The function correctly maps principal types to subject types and returns an error for unsupported types.
366-418: LGTM!The function correctly adds a permission to a space by constructing the URL, preparing the request body, and sending a POST request. Error handling and rate limit annotations are correctly implemented.
421-472: LGTM!The function correctly finds a space permission by listing all permissions. Error handling and rate limit annotations are correctly implemented.
475-504: LGTM!The function correctly retrieves a space by its ID. Error handling and rate limit annotations are correctly implemented.
507-557: LGTM!The function correctly removes a permission from a space by finding the permission and the space, constructing the URL, and sending a DELETE request. Error handling and rate limit annotations are correctly implemented.
560-567: LGTM!The function correctly parses the URL and extracts the pagination cursor.
Line range hint
570-606:
LGTM!The function correctly retrieves users using a search query with pagination, handles pagination, and converts the response to a list of users. Error handling and rate limit annotations are correctly implemented.
pkg/connector/client/ratelimit.go
Outdated
| func WithConfluenceRatelimitData(resource *v2.RateLimitDescription) uhttp.DoOption { | ||
| return func(response *uhttp.WrapperResponse) error { | ||
| rateLimitData, err := helpers.ExtractRateLimitData(response.StatusCode, &response.Header) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| resource = rateLimitData | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Pass the resource parameter by reference.
The resource parameter should be passed by reference to ensure the extracted rate limit data is correctly assigned.
- func WithConfluenceRatelimitData(resource *v2.RateLimitDescription) uhttp.DoOption {
+ func WithConfluenceRatelimitData(resource **v2.RateLimitDescription) uhttp.DoOption {
return func(response *uhttp.WrapperResponse) error {
rateLimitData, err := helpers.ExtractRateLimitData(response.StatusCode, &response.Header)
if err != nil {
return err
}
- resource = rateLimitData
+ *resource = rateLimitData
return nil
}
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func WithConfluenceRatelimitData(resource *v2.RateLimitDescription) uhttp.DoOption { | |
| return func(response *uhttp.WrapperResponse) error { | |
| rateLimitData, err := helpers.ExtractRateLimitData(response.StatusCode, &response.Header) | |
| if err != nil { | |
| return err | |
| } | |
| resource = rateLimitData | |
| return nil | |
| } | |
| func WithConfluenceRatelimitData(resource **v2.RateLimitDescription) uhttp.DoOption { | |
| return func(response *uhttp.WrapperResponse) error { | |
| rateLimitData, err := helpers.ExtractRateLimitData(response.StatusCode, &response.Header) | |
| if err != nil { | |
| return err | |
| } | |
| *resource = rateLimitData | |
| return nil | |
| } | |
| } |
There was a problem hiding this comment.
We can't change the function definition without breaking other stuff. I think it's fine to set the attributes on resource directly. What would that code look like?
There was a problem hiding this comment.
@ggreer, here's how you can set the attributes on the resource directly without changing the function definition:
func WithConfluenceRatelimitData(resource *v2.RateLimitDescription) uhttp.DoOption {
return func(response *uhttp.WrapperResponse) error {
rateLimitData, err := helpers.ExtractRateLimitData(response.StatusCode, &response.Header)
if err != nil {
return err
}
*resource = *rateLimitData
return nil
}
}This code sets the attributes of resource directly by dereferencing both resource and rateLimitData and assigning the values.
pkg/connector/client/ratelimit.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| resource = rateLimitData |
There was a problem hiding this comment.
Instead of making response a double pointer, it's probably easier to set the attributes on the struct one by one.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/connector/client/ratelimit.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- pkg/connector/client/ratelimit.go
Summary by CodeRabbit
New Features
spaceBuilderstruct, allowing listing and entitlement management of spaces.Bug Fixes
Tests