Skip to content
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

(OraklNode) update id types from node admin #1521

Merged
merged 1 commit into from
May 24, 2024

Conversation

nick-bisonai
Copy link
Collaborator

Description

ID's data type in PGSQL is int4 which could be represented as int32 type in golang.
This PR updates the types and unifies with other packages (fetcher, aggregator, and reporter)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.

Deployment

  • Should publish npm package
  • Should publish Docker image

@nick-bisonai nick-bisonai self-assigned this May 24, 2024
@nick-bisonai nick-bisonai requested a review from a team as a code owner May 24, 2024 06:42
Copy link
Contributor

coderabbitai bot commented May 24, 2024

Walkthrough

Walkthrough

The changes across multiple files primarily involve updating the data types of various ID fields from int64 to int32. This includes modifying struct fields and updating integer-to-string conversions in test cases to accommodate the new data types.

Changes

Files Change Summary
node/pkg/admin/config/controller.go Updated ConfigId, Id fields from int64 to int32 in several structs, and changed configNameIdMap type.
node/pkg/admin/feed/controller.go Changed Id and ConfigId fields in FeedModel struct from int64 to int32.
node/pkg/admin/providerUrl/controller.go Modified Id field in ProviderUrlModel struct from *int64 to *int32.
node/pkg/admin/proxy/controller.go Updated Id field in ProxyModel struct from *int64 to *int32.
node/pkg/admin/wallet/controller.go Changed Id field in WalletModel struct from *int64 to *int32.
node/pkg/admin/tests/aggregator_test.go Updated integer-to-string conversions using strconv.Itoa instead of strconv.FormatInt.
node/pkg/admin/tests/config_test.go Modified integer-to-string conversion in TestConfigSync to use strconv.Itoa.
node/pkg/admin/tests/feed_test.go Changed integer-to-string conversions in TestFeedGetByConfigId and TestFeedGetById to strconv.Itoa.
node/pkg/admin/tests/fetcher_test.go Updated integer-to-string conversions in TestFetcherDeactivate and TestAdapterActivate to strconv.Itoa.
node/pkg/admin/tests/providerUrl_test.go Modified integer-to-string conversion in TestProviderUrlGetById to use strconv.Itoa.
node/pkg/admin/tests/proxy_test.go Changed integer-to-string conversions in TestProxyGetById, TestProxyUpdateById, and TestProxyDeleteById to strconv.Itoa.
node/pkg/admin/tests/wallet_test.go Updated integer-to-string conversions in various test functions to use strconv.Itoa.

🐇
In the land of code, IDs were tall,
But now they're shorter, not so small.
From sixty-four to thirty-two,
A change that makes our tests anew.
With strconv.Itoa, strings align,
Our code refined, it’s simply divine!
🛠️✨

Tip

New Features and Improvements

Review Settings

Introduced new personality profiles for code reviews. Users can now select between "Chill" and "Assertive" review tones to tailor feedback styles according to their preferences. The "Assertive" profile posts more comments and nitpicks the code more aggressively, while the "Chill" profile is more relaxed and posts fewer comments.

AST-based Instructions

CodeRabbit offers customizing reviews based on the Abstract Syntax Tree (AST) pattern matching. Read more about AST-based instructions in the documentation.

Community-driven AST-based Rules

We are kicking off a community-driven initiative to create and share AST-based rules. Users can now contribute their AST-based rules to detect security vulnerabilities, code smells, and anti-patterns. Please see the ast-grep-essentials repository for more information.

New Static Analysis Tools

We are continually expanding our support for static analysis tools. We have added support for biome, hadolint, and ast-grep. Update the settings in your .coderabbit.yaml file or head over to the settings page to enable or disable the tools you want to use.

Tone Settings

Users can now customize CodeRabbit to review code in the style of their favorite characters or personalities. Here are some of our favorite examples:

  • Mr. T: "You must talk like Mr. T in all your code reviews. I pity the fool who doesn't!"
  • Pirate: "Arr, matey! Ye must talk like a pirate in all yer code reviews. Yarrr!"
  • Snarky: "You must be snarky in all your code reviews. Snark, snark, snark!"

Revamped Settings Page

We have redesigned the settings page for a more intuitive layout, enabling users to find and adjust settings quickly. This change was long overdue; it not only improves the user experience but also allows our development team to add more settings in the future with ease. Going forward, the changes to .coderabbit.yaml will be reflected in the settings page, and vice versa.

Miscellaneous

  • Turn off free summarization: You can switch off free summarization of PRs opened by users not on a paid plan using the enable_free_tier setting.
  • Knowledge-base scope: You can now set the scope of the knowledge base to either the repository (local) or the organization (global) level using the knowledge_base setting. In addition, you can specify Jira project keys and Linear team keys to limit the knowledge base scope for those integrations.
  • High-level summary placement: You can now customize the location of the high-level summary in the PR description using the high_level_summary_placeholder setting (default @coderabbitai summary).
  • Revamped request changes workflow: You can now configure CodeRabbit to auto-approve or request changes on PRs based on the review feedback using the request_changes_workflow setting.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 988d92d and c5711b5.
Files selected for processing (12)
  • node/pkg/admin/config/controller.go (4 hunks)
  • node/pkg/admin/feed/controller.go (1 hunks)
  • node/pkg/admin/providerUrl/controller.go (1 hunks)
  • node/pkg/admin/proxy/controller.go (1 hunks)
  • node/pkg/admin/tests/aggregator_test.go (2 hunks)
  • node/pkg/admin/tests/config_test.go (1 hunks)
  • node/pkg/admin/tests/feed_test.go (2 hunks)
  • node/pkg/admin/tests/fetcher_test.go (2 hunks)
  • node/pkg/admin/tests/providerUrl_test.go (1 hunks)
  • node/pkg/admin/tests/proxy_test.go (3 hunks)
  • node/pkg/admin/tests/wallet_test.go (5 hunks)
  • node/pkg/admin/wallet/controller.go (1 hunks)
Files skipped from review due to trivial changes (2)
  • node/pkg/admin/tests/aggregator_test.go
  • node/pkg/admin/tests/fetcher_test.go
Additional comments not posted (16)
node/pkg/admin/feed/controller.go (1)

11-11: Updated data types for Id and ConfigId from int64 to int32 align with PR objectives to standardize ID types across the system.

Also applies to: 14-14

node/pkg/admin/tests/feed_test.go (1)

36-36: Updated test cases to use strconv.Itoa for ID conversion align with the changes in data types from int64 to int32.

Also applies to: 52-52

node/pkg/admin/providerUrl/controller.go (1)

10-10: Updated data type for Id from int64 to int32 in ProviderUrlModel aligns with PR objectives to standardize ID types across the system.

node/pkg/admin/tests/providerUrl_test.go (1)

77-77: Updated test case to use strconv.Itoa for ID conversion aligns with the changes in data types from int64 to int32.

node/pkg/admin/proxy/controller.go (1)

10-10: Updated data type for Id from int64 to int32 in ProxyModel aligns with PR objectives to standardize ID types across the system.

node/pkg/admin/tests/config_test.go (1)

45-45: Updated test case to use strconv.Itoa for ID conversion aligns with the changes in data types from int64 to int32.

node/pkg/admin/tests/proxy_test.go (4)

79-79: Ensure the ID conversion from int64 to int32 is handled correctly in all test cases.


152-152: Confirm that the retrieval of a proxy by ID after deletion correctly returns a nil result, reflecting the updated ID type.


147-147: Check the delete functionality with the new ID type to ensure it's functioning as expected.

Verification successful

To verify the delete functionality with the new ID type, we need to inspect the DeleteRequest function definition and the ID type used in the proxy_test.go file. This will help us ensure that the ID conversion is handled correctly.


The DeleteRequest function is defined in multiple places, and it appears to be a generic function that handles DELETE requests. In the proxy_test.go file, the ID is being converted to a string using strconv.Itoa(int(*insertResult.Id)), which suggests that the ID is of a pointer type that can be dereferenced to an integer.

Given this context, it seems that the ID type is being handled correctly in the DeleteRequest function. The conversion to a string using strconv.Itoa is appropriate for an integer ID type.

  • DeleteRequest function definition:

    • node/pkg/admin/tests/test_helper.go:87
    • delegator/utils/test_helper.go:111
    • api/utils/test_helper.go:72
  • ID usage in proxy_test.go:

    • strconv.Itoa(int(*insertResult.Id))

The delete functionality with the new ID type appears to be functioning as expected.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the delete request handles the new ID type correctly.
# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type go $'DeleteRequest'

Length of output: 6062



Script:

#!/bin/bash
# Description: Verify the definition of the DeleteRequest function and the ID type used in proxy_test.go.
# Step 1: Get the definition of the DeleteRequest function.
ast-grep --lang go --pattern 'func DeleteRequest[T any](app *fiber.App, endpoint string, requestBody interface{}) (T, error)' 

# Step 2: Check the ID type used in proxy_test.go.
rg --type go 'Id' node/pkg/admin/tests/proxy_test.go

Length of output: 1904


114-114: Verify that the updated ID type does not affect the patch request functionality.

Verification successful

The PatchRequest function relies on the req function, which handles the HTTP request and response processing. The req function appears to correctly handle the request body and endpoint, ensuring that the ID type conversion should not cause issues. Additionally, the test cases that use PatchRequest are comprehensive and cover various scenarios.

  • PatchRequest function definitions:

    • node/pkg/admin/tests/test_helper.go
    • delegator/utils/test_helper.go
    • api/utils/test_helper.go
  • req function definitions:

    • node/pkg/admin/tests/test_helper.go
    • delegator/utils/test_helper.go
    • api/utils/test_helper.go
  • Test cases using PatchRequest:

    • TestProxyInsert
    • TestProxyGet
    • TestProxyGetById
    • TestProxyUpdateById
    • TestProxyDeleteById
    • TestWalletInsert
    • TestWalletGet
    • TestWalletGetAddress
    • TestWalletGetById
    • TestWalletUpdateById
    • TestWalletDeleteById
    • TestFunctionRead
    • TestFunctionReadSingle
    • TestFunctionInsert
    • TestFunctionUpdate
    • TestFunctionDelete
    • TestReporterRead
    • TestReporterReadSingle
    • TestReporterInsert
    • TestReporterUpdate
    • TestReporterDelete
    • TestOrganizationRead
    • TestOrganizationReadSingle
    • TestOrganizationInsert
    • TestOrganizationUpdate
    • TestOrganizationDelete
    • TestContractRead
    • TestContractInsert
    • TestContractReadSingle
    • TestContractUpdate
    • TestContractDelete
    • TestConnectReporter
    • TestVrf
    • TestService
    • TestReporter
    • TestProxy
    • TestListener
    • TestChain
    • TestAggregator
    • TestAggregate

Given the comprehensive test coverage and the correct handling of the request in the req function, the updated ID type should not affect the patch request functionality.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the patch request handles the new ID type correctly.
# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type go $'PatchRequest'

Length of output: 2845



Script:

#!/bin/bash
# Retrieve the definitions of the PatchRequest function
ast-grep --lang go --pattern $'func PatchRequest[T any](app *fiber.App, endpoint string, requestBody interface{}) (T, error) {
  $$$
}' 

# List the test cases that use PatchRequest to verify their coverage
rg --type go 'PatchRequest' --files-with-matches | xargs -I {} rg --type go 'func Test' {}

Length of output: 2566



Script:

#!/bin/bash
# Retrieve the definition of the req function
ast-grep --lang go --pattern $'func req[T any](app *fiber.App, method string, endpoint string, requestBody interface{}) (T, error) {
  $$$
}' 

# List the test cases that use PatchRequest to verify their coverage
rg --type go 'PatchRequest' --files-with-matches | xargs -I '{}' rg --type go 'func Test' '{}'

Length of output: 7933

node/pkg/admin/wallet/controller.go (1)

13-13: The update from int64 to int32 in the WalletModel struct is consistent with the PR objectives to standardize ID types across the system.

node/pkg/admin/tests/wallet_test.go (4)

45-45: Ensure the retrieval of a wallet by ID reflects the updated ID type without affecting functionality.


99-99: Confirm that the get by ID functionality correctly handles the new ID type, especially in edge cases like invalid IDs.


152-152: Verify the delete functionality and subsequent retrieval attempt with the new ID type to ensure correct behavior.

Also applies to: 164-164


125-125: Check the update functionality with the new ID type to ensure it's functioning as expected.

Also applies to: 130-130

Verification successful

The PatchRequest function is used in multiple test files, including wallet_test.go. To ensure the update functionality with the new ID type is functioning as expected, we need to verify the PatchRequest function definition and its usage in wallet_test.go.

Let's inspect the PatchRequest function definition and its usage in wallet_test.go to ensure it handles the new ID type correctly.


The PatchRequest function is defined in multiple files and is used in wallet_test.go to update a wallet. The function signature and its usage in wallet_test.go appear to be consistent with the new ID type.

  • The PatchRequest function is defined as:

    func PatchRequest[T any](app *fiber.App, endpoint string, requestBody interface{}) (T, error) {
        return req[T](app, "PATCH", endpoint, requestBody)
    }
  • In wallet_test.go, the function is used as follows:

    updateResult, err := PatchRequest[wallet.WalletModel](testItems.app, "/api/v1/wallet/"+strconv.Itoa(int(*testItems.tmpData.wallet.Id)), map[string]any{"pk": mockWallet.Pk})
    if err != nil {
        t.Fatalf("error updating wallet: %v", err)
    }

The usage of PatchRequest in wallet_test.go correctly handles the new ID type by converting the ID to a string using strconv.Itoa(int(*testItems.tmpData.wallet.Id)).

No issues were found with the update functionality regarding the new ID type.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the update request handles the new ID type correctly.
# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type go $'PatchRequest'

Length of output: 2845



Script:

#!/bin/bash
# Description: Verify the definition of the PatchRequest function and its usage in wallet_test.go.
# 1. Get the definition of the PatchRequest function.
# 2. Get the relevant lines from wallet_test.go where PatchRequest is used.

# Get the definition of the PatchRequest function
rg --type go --context 5 'func PatchRequest' 

# Get the relevant lines from wallet_test.go where PatchRequest is used
rg --type go --context 5 'PatchRequest' node/pkg/admin/tests/wallet_test.go

Length of output: 3007

node/pkg/admin/config/controller.go (1)

22-22: The updates to the ID types in various models and the map in the Sync function are consistent with the PR objectives and should help standardize ID handling across the system.

Also applies to: 34-34, 43-43, 106-106

@Intizar-T
Copy link
Contributor

seems like there is int8 in go as well. I'm wondering why not scale it down to int8 if the id type in psql is int4

Copy link
Contributor

@Intizar-T Intizar-T left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@nick-bisonai
Copy link
Collaborator Author

seems like there is int8 in go as well. I'm wondering why not scale it down to int8 if the id type in psql is int4

the possible number range for pgsql int4 is equal to golang int32
both in between -2147483648 to 2147483647.

but golang int8 can represent numbers only in range between -128 to 127, so it's not compatible

@nick-bisonai nick-bisonai merged commit f3be033 into master May 24, 2024
1 check passed
@nick-bisonai nick-bisonai deleted the feat/update-id-data-type branch May 24, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants