-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add rest table #75
add rest table #75
Conversation
WalkthroughThis pull request introduces comprehensive documentation for a REST API in the newly created file Changes
Suggested labels
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- integration/rest.md (1 hunks)
Additional context used
LanguageTool
integration/rest.md
[uncategorized] ~17-~17: The abbreviation “e.g.” (= for example) requires two periods.
Context: ... | 1. Key: role - value: (e.g Admin, Read-Only, Read-Write)
2. us...(E_G)
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...econds"
],
"data": [ { queryData… } ]
} ...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
integration/rest.md
10-10: null
Bare URL used(MD034, no-bare-urls)
10-10: null
Bare URL used(MD034, no-bare-urls)
11-11: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (1)
integration/rest.md (1)
8-8
: Table headers are correctly formatted.The headers are well-structured and appropriately labeled for clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- integration/rest.md (1 hunks)
Additional context used
LanguageTool
integration/rest.md
[uncategorized] ~17-~17: The abbreviation “e.g.” (= for example) requires two periods.
Context: ... | 1. Key: role - value: (e.g Admin, Read-Only, Read-Write)
2. us...(E_G)
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...econds"
],
"data": [ { queryData… } ]
} ...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
integration/rest.md
10-10: null
Bare URL used(MD034, no-bare-urls)
10-10: null
Bare URL used(MD034, no-bare-urls)
11-11: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (3)
integration/rest.md (3)
2-2
: Correct the typo in the YAML front matter.The
tile
field should be corrected totitle
to ensure proper rendering and categorization of the document.Proposed fix:
- tile: "Rest API" + title: "Rest API"
17-17
: Correct abbreviation usage.The abbreviation "e.g." should be written with two periods as "e.g.," to adhere to standard writing conventions.
Proposed fix:
- (e.g Admin, Read-Only, Read-Write) + (e.g., Admin, Read-Only, Read-Write)Tools
LanguageTool
[uncategorized] ~17-~17: The abbreviation “e.g.” (= for example) requires two periods.
Context: ... | 1. Key: role - value: (e.g Admin, Read-Only, Read-Write)
2. us...(E_G)
21-21
: Address punctuation issues in JSON data.To enhance readability and ensure proper JSON formatting, consider adjusting the spacing around curly braces.
Proposed fix:
- [ { queryData… } ] + [{queryData…}]Likely invalid or redundant comment.
Tools
LanguageTool
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...econds"
],
"data": [ { queryData… } ]
} ...(UNLIKELY_OPENING_PUNCTUATION)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- integration/rest.md (1 hunks)
Additional context used
LanguageTool
integration/rest.md
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...econds"
],
"data": [ { queryData… } ]
} | This request runs a query on t...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
integration/rest.md
10-10: null
Bare URL used(MD034, no-bare-urls)
10-10: null
Bare URL used(MD034, no-bare-urls)
11-11: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (3)
integration/rest.md (3)
2-6
: Front matter looks good.The YAML front matter is correctly formatted with appropriate fields for
title
,description
, andparent
. This ensures proper rendering and categorization of the document.
8-24
: Review of API endpoint documentation.The documentation of API endpoints is comprehensive and well-structured, providing clear details about request methods, endpoints, headers, parameters, and responses. This structure helps in easy navigation and understanding of the API functionalities.
Tools
LanguageTool
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...econds"
],
"data": [ { queryData… } ]
} | This request runs a query on t...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
10-10: null
Bare URL used(MD034, no-bare-urls)
10-10: null
Bare URL used(MD034, no-bare-urls)
11-11: null
Bare URL used(MD034, no-bare-urls)
21-21
: Address punctuation issues.There is a loose punctuation mark in the JSON data representation. Ensuring correct punctuation will improve the readability and correctness of the data format.
Apply this fix to address the punctuation issue:
- [ { queryData… } ] + [{queryData…}]Likely invalid or redundant comment.
Tools
LanguageTool
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...econds"
],
"data": [ { queryData… } ]
} | This request runs a query on t...(UNLIKELY_OPENING_PUNCTUATION)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
.wordlist.txt (2)
278-283
: Approved, but note potential inconsistency in capitalization.The new entries (graphName, http, json, queryData, schemaName, signin) are relevant and correctly spelled. However, there's a potential inconsistency between "GraphName" (line 271) and "graphName" (line 278). Consider standardizing the capitalization for consistency.
Consider using either "GraphName" or "graphName" consistently throughout the wordlist and the codebase.
290-292
: Approved, with a suggestion for placeholder terms.The entries "www", "yourQuery", and "yourSourceName" are correctly formatted and likely relevant to documentation examples.
Consider prefixing placeholder terms like "yourQuery" and "yourSourceName" with "example" (e.g., "exampleQuery", "exampleSourceName") to make their purpose clearer in the documentation.
integration/rest.md (3)
86-86
: Clarify the 'value' parameter description.The current description states that the 'value' parameter should be an integer, but it's not clear if this is specific to the
MAX_QUEUED_QUERIES
configuration or if it applies to all possible configurations.Consider updating the description to be more generic or specific:
- The integer value to set. + The value to set (integer for MAX_QUEUED_QUERIES).
124-151
: Consider adding more details to the Create New User endpoint documentation.While the current documentation is clear, it could be improved by adding information about:
- Possible user roles (e.g., Admin, Read-Only, Read-Write).
- Password requirements or constraints.
- Any additional optional parameters that can be included in the request body.
This additional information would help API users understand the expected input and potential variations.
302-372
: LGTM for Duplicate A Graph and Delete A Schema endpoints. Consider separating schema creation and query execution.The documentation for the Duplicate A Graph and Delete A Schema endpoints is clear and well-structured. However, the Create New Schema & Run A Query endpoint combines two operations, which is inconsistent with RESTful API design principles.
Consider separating the schema creation and query execution into distinct endpoints:
Create Schema endpoint:
POST /api/schema/{schemaName}
Run Query endpoint:
POST /api/schema/{schemaName}/query
This separation would make the API more consistent and easier to understand and use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .wordlist.txt (1 hunks)
- integration/rest.md (1 hunks)
🔇 Additional comments (9)
.wordlist.txt (3)
271-277
: LGTM: New entries are relevant and consistent.The new entries (GraphName, api, auth, callbackUrl, csrf, csrfToken, destinationGraphName) are relevant to web development and API contexts, which aligns with the FalkorDB documentation. The spelling and capitalization are correct and consistent with common usage.
284-289
: LGTM: Authentication and URL-related terms are appropriate.The new entries (signinUrl, signout, sourceName, url, urlencoded, userName) are correctly spelled and relevant to authentication and URL handling in web development. The capitalization is consistent with common usage.
271-292
: Summary: Wordlist additions enhance documentation support.The new entries added to the
.wordlist.txt
file are relevant to web development, APIs, and graph databases, which aligns well with the FalkorDB documentation project. These additions will likely improve spell-checking accuracy and maintain consistency in the documentation.A few minor suggestions have been made:
- Standardize capitalization between "GraphName" and "graphName".
- Consider clarifying placeholder terms like "yourQuery" and "yourSourceName".
Overall, these changes are beneficial and ready to be merged.
integration/rest.md (6)
1-6
: LGTM: YAML front matter is correct and informative.The YAML front matter contains the necessary information for the REST API documentation, including the title, description, and parent category.
8-23
: LGTM: Table of contents is comprehensive and well-structured.The table of contents provides a clear overview of all documented API endpoints, making it easy for users to navigate the documentation.
27-47
: LGTM: Login endpoint documentation is clear and comprehensive.The documentation for the Login endpoint provides all necessary information, including the HTTP method, path, purpose, and expected response format with an example JSON payload.
49-76
: LGTM: Logout endpoint documentation is thorough and well-structured.The documentation for the Logout endpoint clearly describes the request method, path, required parameters, and expected response format with example JSON payloads for both request and response.
100-122
: LGTM: Get Configuration Value endpoint documentation is clear and complete.The documentation for the Get Configuration Value endpoint provides all necessary information, including the HTTP method, path, required parameters, and expected response format with an example JSON payload.
263-282
: LGTM: Delete A Graph endpoint documentation is clear and follows RESTful conventions.The documentation for the Delete A Graph endpoint is well-structured and provides all necessary information. The use of the DELETE method is appropriate for this operation, adhering to RESTful API conventions.
### **Get All Users** - `GET /api/user` | ||
|
||
This endpoint retrieves a list of all users. | ||
|
||
#### Responses | ||
|
||
- **200**: List of users retrieved successfully | ||
- Content-Type: `application/json` | ||
- Example response: | ||
|
||
```json | ||
{ | ||
"result": [ | ||
{ | ||
"username": "default", | ||
"role": "Admin", | ||
"checked": false | ||
} | ||
] | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding pagination and filtering options to the Get All Users endpoint.
The current documentation for the Get All Users endpoint is clear, but it could be improved by adding information about:
- Pagination options (e.g., limit, offset) to handle large numbers of users efficiently.
- Filtering options (e.g., by role, username) to allow more specific queries.
Example query parameters:
?limit=10&offset=0
for pagination?role=Admin
for filtering by role
Adding these options would make the API more flexible and efficient for clients dealing with large numbers of users.
### **Modify A User** - `PATCH /api/user/{userName}` | ||
|
||
This endpoint updates the role of a specific user. | ||
|
||
#### Parameters | ||
|
||
- `cookie` (header, required): Cookie header with session and auth tokens. | ||
- `userName` (path, required): The username of the user to modify. | ||
- `role` (query, required): The new role to assign to the user (`Admin`, `Read-Only`, `Read-Write`). | ||
|
||
#### Responses | ||
|
||
- **200**: User updated successfully | ||
- Content-Type: `application/json` | ||
- Example response: | ||
|
||
```json | ||
{ | ||
"message": "User role updated" | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider expanding the Modify A User endpoint functionality.
The current documentation for the Modify A User endpoint only allows changing the user's role. Consider expanding this endpoint to allow modification of other user attributes, such as:
- Username
- Password
- Email (if applicable)
- Other relevant user information
This would make the API more flexible and reduce the need for multiple endpoints for user modifications.
Also, consider using a request body instead of query parameters for the new role, especially if more attributes are added. This would be more in line with RESTful API practices for PATCH requests.
Example request body:
{
"role": "Read-Write",
"email": "[email protected]",
"password": "newPassword123"
}
### **Create A Graph & Run A Query** - `GET /api/graph/{graphName}` | ||
|
||
This endpoint creates a graph and runs a query. | ||
|
||
#### Parameters | ||
|
||
- `cookie` (header, required): Cookie header with session and auth tokens. | ||
- `graphName` (path, required): The name of the graph to be created. | ||
- `query` (query, required): The query to run, such as `RETURN 1`. | ||
|
||
#### Responses | ||
|
||
- **200**: Graph created and query executed | ||
- Content-Type: `application/json` | ||
- Example response: | ||
|
||
```json | ||
{ | ||
"result": { | ||
"metadata": [ | ||
"Nodes created: 40", | ||
"Relationships created: 20", | ||
"Cached execution: 1", | ||
"Query internal execution time: 0.201420 milliseconds" | ||
], | ||
"data": [ | ||
{ | ||
"queryData": "exampleData" | ||
} | ||
] | ||
} | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider separating graph creation and query execution into distinct endpoints.
The current endpoint combines two operations (creating a graph and running a query) into a single GET request, which is unconventional and may lead to confusion. Consider the following improvements:
- Separate graph creation and query execution into two distinct endpoints.
- Use appropriate HTTP methods for each operation (POST for creation, GET or POST for query execution).
Suggested changes:
-
Create Graph endpoint:
POST /api/graph/{graphName}
-
Run Query endpoint:
POST /api/graph/{graphName}/query
This separation would make the API more RESTful and easier to understand and use.
### **Get All Graphs** - `GET /api/graph` | ||
|
||
This endpoint retrieves a list of all graphs. | ||
|
||
#### Responses | ||
|
||
- **200**: List of graphs retrieved successfully | ||
- Content-Type: `application/json` | ||
- Example response: | ||
|
||
```json | ||
{ | ||
"result": [ | ||
"graphName" | ||
] | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding pagination and filtering options to the Get All Graphs endpoint.
The current documentation for the Get All Graphs endpoint is clear, but it could be improved by adding information about:
- Pagination options (e.g., limit, offset) to handle large numbers of graphs efficiently.
- Filtering options (e.g., by creation date, size) to allow more specific queries.
Example query parameters:
?limit=10&offset=0
for pagination?created_after=2023-01-01
for filtering by creation date
Adding these options would make the API more flexible and efficient for clients dealing with large numbers of graphs.
### **Delete User** - `POST /api/user` | ||
|
||
This endpoint deletes a user based on their username and role. | ||
|
||
#### Request Body | ||
|
||
- Content-Type: `application/json` | ||
- Example request: | ||
|
||
```json | ||
{ | ||
"users": [ | ||
{ | ||
"username": "userName", | ||
"role": "Read-Write" | ||
} | ||
] | ||
} | ||
``` | ||
|
||
#### Responses | ||
|
||
- **200**: User deleted successfully | ||
- Content-Type: `application/json` | ||
- Example response: | ||
|
||
```json | ||
{ | ||
"message": "User deleted" | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using DELETE method for user deletion.
The current implementation uses a POST request for user deletion, which is unconventional. RESTful API best practices suggest using the DELETE method for resource deletion operations.
Consider changing the endpoint to use the DELETE method:
- ### **Delete User** - `POST /api/user`
+ ### **Delete User** - `DELETE /api/user`
If multiple users need to be deleted in a single request, you might want to create a separate endpoint for batch deletion, such as DELETE /api/users
that accepts a list of user IDs in the request body.
📝 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.
### **Delete User** - `POST /api/user` | |
This endpoint deletes a user based on their username and role. | |
#### Request Body | |
- Content-Type: `application/json` | |
- Example request: | |
```json | |
{ | |
"users": [ | |
{ | |
"username": "userName", | |
"role": "Read-Write" | |
} | |
] | |
} | |
``` | |
#### Responses | |
- **200**: User deleted successfully | |
- Content-Type: `application/json` | |
- Example response: | |
```json | |
{ | |
"message": "User deleted" | |
} | |
``` | |
### **Delete User** - `DELETE /api/user` | |
This endpoint deletes a user based on their username and role. | |
#### Request Body | |
- Content-Type: `application/json` | |
- Example request: | |
```json | |
{ | |
"users": [ | |
{ | |
"username": "userName", | |
"role": "Read-Write" | |
} | |
] | |
} | |
``` | |
#### Responses | |
- **200**: User deleted successfully | |
- Content-Type: `application/json` | |
- Example response: | |
```json | |
{ | |
"message": "User deleted" | |
} | |
``` |
|
||
#### Responses | ||
|
||
- **201**: User created successfully |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is returned if the user can not be created? for example it is already exists?
|
||
#### Responses | ||
|
||
- **200**: User deleted successfully |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is returned if the user does not exists, or the current user can not delete this user ?
} | ||
``` | ||
|
||
### **Delete User** - `POST /api/user` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a delete request and mark the post request as deprecated ?
} | ||
``` | ||
|
||
### **Get All Users** - `GET /api/user` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any user can call that ?
} | ||
``` | ||
|
||
### **Modify A User** - `PATCH /api/user/{userName}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
who can call modify user ?
|
||
#### Responses | ||
|
||
- **200**: Graph created and query executed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is return in case of the query fail ?
|
||
#### Responses | ||
|
||
- **200**: Graph duplicated successfully |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is returned if the source graph does not exists or if the operation fail (parsing etc)?
|
||
#### Responses | ||
|
||
- **200**: Schema created and query executed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again error case
} | ||
``` | ||
|
||
### **Delete A Schema** - `DELETE /api/graph/{schemaName}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again error case
} | ||
``` | ||
|
||
### **Create New Schema & Run A Query** - `GET /api/graph/{schemaName}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider alias to post and depreciate
Summary by CodeRabbit
New Features
Documentation