Skip to content

Regenerate with decimal_string enabled for v2 APIs#17

Closed
shivampkumar wants to merge 4 commits into
mirror/pr-2187-basefrom
mirror/pr-2187
Closed

Regenerate with decimal_string enabled for v2 APIs#17
shivampkumar wants to merge 4 commits into
mirror/pr-2187-basefrom
mirror/pr-2187

Conversation

@shivampkumar

@shivampkumar shivampkumar commented Jun 11, 2026

Copy link
Copy Markdown

Why?

V2 API decimal fields (e.g. amount on billing meter events) were previously generated as plain String types, even though the same fields in v1 are generated as BigDecimal. A codegen workaround suppressed decimal_string type mapping for all v2 APIs. That workaround has been removed.

What?

  • Regenerated code with decimal_string enabled for v2 APIs
  • V2 decimal fields now use BigDecimal instead of String, matching v1 behavior
  • SDK fix: Added a custom Gson TypeAdapter<BigDecimal> to JsonEncoder.BODY_GSON that serializes BigDecimal as a JSON string (e.g., "75.25" not 75.25). The V2 API expects decimal fields as strings on the wire. V1 doesn't hit this because FormEncoder calls .toString() naturally.

Testing

Integration tested against a live Stripe sandbox (6/6 tests passed):

  1. BigDecimal deserialization from Gson JSON string ✓
  2. BigDecimal null when field absent ✓
  3. BigDecimal high precision (10 decimal places) ✓
  4. Amount consolidation (shared v2.Amount class) ✓
  5. Live V2 Account create with BigDecimal percentOwnership ✓
  6. Live V2 Account retrieve — round-trip BigDecimal percentOwnership ✓

See Also

Changelog

  • ⚠️ Breaking change: V2 API decimal fields changed type from String to BigDecimal. Code that reads or writes these fields as String will need to use BigDecimal instead. Affected fields:
    • V2.Core.Account / V2.Core.AccountPerson: percentOwnership
    • PaymentEvaluation.Signals.FraudulentPayment: score
    • Params: AccountCreateParams, PersonCreateParams, AccountTokenCreateParams, PersonTokenCreateParams (all: percentOwnership)
    • Params: InvoiceItemCreateParams, InvoiceAddLinesParams, InvoiceUpdateLinesParams, InvoiceCreatePreviewParams (all: quantityDecimal)

Mirrored from stripe/stripe-java#2187 by jar-stripe for the VibeOps live demo. Real PR, real diff, reviewed by the same pipeline our customers run.

jar-stripe and others added 4 commits March 18, 2026 21:22
V2 API fields with `format: decimal` now generate as `BigDecimal` instead
of `String`. Driven by sdk-codegen#3369.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude
The V2 API expects decimal fields as JSON strings (e.g., "25.5") not
JSON numbers. Add a custom Gson TypeAdapter for BigDecimal to BODY_GSON
that serializes via toPlainString().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude

@vibeops-ai vibeops-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ VibeOps Review — The PR has a critical bug in the BigDecimal TypeAdapter that will crash when the API returns numeric values instead of strings.

🟡 Human review required · verdict=request_changes

Blockers (1)

BigDecimal TypeAdapter crashes on numeric JSON tokens (confirmed by logic)
src/main/java/com/stripe/net/JsonEncoder.java:32

The read method calls in.nextString() unconditionally, but Gson's JsonReader.nextString() only works when the current token is a JSON string. If the Stripe API returns percent_ownership as a JSON number (e.g., 25.5) instead of a string (e.g., "25.5"), this will throw IllegalStateException because the current token is NUMBER, not STRING. The adapter assumes all BigDecimal values come as strings, but APIs can return them as either strings or numbers.

Check token type first:
```java
@Override
public BigDecimal read(JsonReader in) throws IOException {
  if (in.peek() == JsonToken.STRING) {
    return new BigDecimal(in.nextString());
  } else {
    return BigDecimal.valueOf(in.nextDouble());
  }
}

### What's good
- Good alignment between v1 and v2 APIs by standardizing on BigDecimal for decimal fields, improving type safety and consistency across the SDK.

### Context
- This PR removes a codegen workaround that was suppressing decimal_string mapping for v2 APIs, bringing them in line with v1 behavior.

---
*Reviewed by [VibeOps](https://vibeops.tech)*

}

@Override
public BigDecimal read(JsonReader in) throws IOException {

@vibeops-ai vibeops-ai Bot Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Blocker · Bug · The read method calls in.nextString() unconditionally, but Gson's JsonReader.nextString() only works when the current token is a JSON string. If the Stripe API returns percent_ownership as a JSON number (e.g., 25.5) instead of a string (e.g., "25.5"), this will throw IllegalStateException because the current token is NUMBER, not STRING. The adapter assumes all BigDecimal values come as strings, but APIs can return them as either strings or numbers.

🔗 Backed by: agent reasoning

Suggested change
public BigDecimal read(JsonReader in) throws IOException {
Check token type first:
```java
`Override`
public BigDecimal read(JsonReader in) throws IOException {
if (in.peek() == JsonToken.STRING) {
return new BigDecimal(in.nextString());
} else {
return BigDecimal.valueOf(in.nextDouble());
}
}


*Was this helpful? React with :+1: or :-1: to help VibeOps learn.*

}

@Override
public BigDecimal read(JsonReader in) throws IOException {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Question — In JsonEncoder.java: The read method of BIG_DECIMAL_STRING_ADAPTER calls in.nextString() unconditionally. Could you clarify the intent here?

If this was intentional, I'll note it and won't flag it as a bug. If not, I can flag the related usages as blockers.

Reply here and I'll learn from your answer.

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