Skip to content

Update Numeric Types: Add int64 and string support for AttributeTypeInt64#93

Merged
Saurabhdarekar merged 1 commit intomainfrom
Update-Numeric-Types
Sep 4, 2025
Merged

Update Numeric Types: Add int64 and string support for AttributeTypeInt64#93
Saurabhdarekar merged 1 commit intomainfrom
Update-Numeric-Types

Conversation

@Saurabhdarekar
Copy link
Copy Markdown
Contributor

@Saurabhdarekar Saurabhdarekar commented Sep 3, 2025

This PR is a copy of original #90. I have migrated to a new PR as for old PR codeQL workflow was not working.

The framework.AttributeTypeInt64 handler in web/json_value.go only accepted float64 values, causing failures when developers provided int64 or string values directly. This forced adapter developers to manually cast values to float64 before passing them to the framework - a poor developer experience that also introduced precision loss for large integers.

Example of the issue:
// This would fail with the old implementation
obj.WithAttribute("userId", int64(123456789012345)) // Error!

// Developers had to do this workaround:
obj.WithAttribute("userId", float64(myInt64Value)) // Ugly + precision loss

Solution

Enhanced the AttributeTypeInt64 case to handle three input types:

  1. int64 - Direct return (no conversion needed)
  2. float64 - Convert with precision validation
  3. string - Parse using strconv.ParseInt

Key Improvements

🛡️ Precision Safety: Added validation for float64 → int64 conversions

  • Rejects values outside the safe integer range (±2^53-1)
  • Prevents fractional values from being silently truncated
  • Provides clear error messages when precision would be lost

🚀 Expanded Range: String parsing supports the full int64 range without precision loss

  • Handles values from -9223372036854775808 to 9223372036854775807
  • No more artificial limitations due to float64 precision boundaries

✨ Better Developer Experience:

  • Developers can provide int64 values directly without manual casting
  • Clear, actionable error messages guide developers when issues occur
  • Backward compatible with existing float64 inputs

Testing

  • ✅ All existing tests continue to pass
  • ✅ Added comprehensive test coverage for new int64 handling scenarios
  • ✅ Added tests for precision validation and error cases
  • ✅ Added integration tests in JSON object processing context

Breaking Changes

⚠️ Intentional safety improvements that may surface previously silent issues:

  • Values with fractional parts now error instead of truncating silently
  • Large integers beyond safe precision now error instead of corrupting data
  • These are good breaks that improve data integrity

@Saurabhdarekar Saurabhdarekar changed the title Add int64 and string support for AttributeTypeInt64 Update Numeric Types: Add int64 and string support for AttributeTypeInt64 Sep 3, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.16%. Comparing base (6a6fbdd) to head (08266f6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #93      +/-   ##
==========================================
+ Coverage   48.68%   50.16%   +1.48%     
==========================================
  Files          24       24              
  Lines        2654     2675      +21     
==========================================
+ Hits         1292     1342      +50     
+ Misses       1306     1287      -19     
+ Partials       56       46      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Saurabhdarekar Saurabhdarekar marked this pull request as ready for review September 3, 2025 21:02
Copilot AI review requested due to automatic review settings September 3, 2025 21:02
@Saurabhdarekar Saurabhdarekar requested a review from a team as a code owner September 3, 2025 21:02
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances numeric type handling in the JSON value conversion framework by adding comprehensive support for int64 and string inputs for AttributeTypeInt64 and AttributeTypeDouble. The changes improve developer experience by allowing direct int64 values and string parsing while adding precision validation to prevent data corruption.

Key Changes

  • Extended AttributeTypeInt64 to handle int64, float64 (with validation), and string inputs
  • Enhanced AttributeTypeDouble to support string parsing in addition to float64
  • Added comprehensive test coverage for all new input type combinations and edge cases

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
web/json_value.go Core conversion logic updates for AttributeTypeInt64 and AttributeTypeDouble with new type support and validation
web/json_value_test.go Unit tests for individual attribute conversion scenarios including edge cases and error conditions
web/json_object_test.go Integration tests for JSON object processing with comprehensive coverage of mixed types and error handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread web/json_value.go
Comment thread web/json_value.go
Copy link
Copy Markdown
Contributor

@nholbrook nholbrook left a comment

Choose a reason for hiding this comment

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

LGTM - code matches that from my previous review on #90

@Saurabhdarekar Saurabhdarekar merged commit 30a11c0 into main Sep 4, 2025
7 checks passed
@Saurabhdarekar Saurabhdarekar deleted the Update-Numeric-Types branch September 4, 2025 14:59
Saurabhdarekar added a commit that referenced this pull request Sep 17, 2025
…uteTypeInt64" (#97)

Reverts #93

The reason to revert this change is as it implies changes to be done is
adapter and there needs a deep investigation on how it will affect the
customers. After this investigation is completed we can merge this
change again.
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.

3 participants