Skip to content

Support schema-on-read for PrimitiveWeaver.intWeaver#106

Merged
xerial merged 4 commits intomainfrom
copilot/fix-105
May 30, 2025
Merged

Support schema-on-read for PrimitiveWeaver.intWeaver#106
xerial merged 4 commits intomainfrom
copilot/fix-105

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 30, 2025

This PR implements schema-on-read capability for PrimitiveWeaver.intWeaver, enabling flexible type conversions from various MessagePack types to Int, similar to airframe-codec functionality.

Changes

Enhanced intWeaver.unpack method to support:

  • INTEGER types: All MessagePack integer formats (INT8, INT16, INT32, INT64, UINT8, UINT16, UINT32, UINT64, FixInt) with proper overflow handling
  • FLOAT types: Safe conversion of whole number doubles/floats within Int range
  • STRING types: Parsing of numeric strings to Int with comprehensive error handling
  • BOOLEAN types: Standard conversion (true→1, false→0)
  • NIL type: Converts to 0 following common convention
  • Error handling: Appropriate error messages for unsupported types and invalid conversions

Before:

override def unpack(u: Unpacker, context: WeaverContext): Unit =
  // TODO Schema-on-read support
  context.setInt(u.unpackInt)

After:

override def unpack(u: Unpacker, context: WeaverContext): Unit =
  u.getNextValueType match
    case ValueType.INTEGER => /* handle all integer types */
    case ValueType.FLOAT => /* convert whole numbers safely */
    case ValueType.STRING => /* parse numeric strings */
    case ValueType.BOOLEAN => /* true=1, false=0 */
    case ValueType.NIL => /* nil=0 */
    case other => /* appropriate error */

Testing

Added comprehensive test suite (PrimitiveWeaverTest) with 9 test cases covering:

  • ✅ Valid conversions for all supported types
  • ✅ Error cases for invalid conversions (fractional floats, non-numeric strings, etc.)
  • ✅ Edge cases (Int.MaxValue/MinValue boundaries, overflow scenarios)
  • ✅ Cross-platform compatibility (JVM, JS, Native)

All existing tests continue to pass, ensuring backward compatibility.

Impact

  • No breaking changes - maintains full backward compatibility
  • Enhanced functionality - enables flexible data transformation patterns
  • Robust error handling - prevents runtime failures with clear error messages
  • Minimal code footprint - surgical changes with maximum impact

Fixes #105.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: xerial <57538+xerial@users.noreply.github.com>
Copilot AI changed the title [WIP] Support schema-on-read for PrimitiveWeaver.intWeaver Support schema-on-read for PrimitiveWeaver.intWeaver May 30, 2025
Copilot AI requested a review from xerial May 30, 2025 22:09
Copilot finished work on behalf of xerial May 30, 2025 22:09
case ValueType.NIL =>
try
u.unpackNil
context.setInt(0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

context.setNull should be used

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to use context.setNull instead of context.setInt(0) for NIL values. This is more semantically correct as NIL represents absence of value rather than explicit zero. The functional behavior remains the same (NIL → 0) since null.asInstanceOf[Int] returns 0 in Scala. (578b981)

case e: Exception =>
context.setError(e)
case other =>
context.setError(new IllegalArgumentException(s"Cannot convert ${other} to Int"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For unknown valuse, we also need to call skipValue of the unpacker to proceed the cursor

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added u.skipValue call before setting error for unknown value types to properly advance the unpacker cursor. (eeea193)

…xerial

Co-authored-by: xerial <57538+xerial@users.noreply.github.com>
…acker cursor

Co-authored-by: xerial <57538+xerial@users.noreply.github.com>
Copilot finished work on behalf of xerial May 30, 2025 22:30
@xerial xerial marked this pull request as ready for review May 30, 2025 22:55
@xerial xerial merged commit 68a13ed into main May 30, 2025
10 checks passed
@xerial xerial deleted the copilot/fix-105 branch May 30, 2025 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support schema-on-read for PrimitiveWeaver.intWeaver

2 participants