From f673243af0190905b753d41d4aa34787e280624d Mon Sep 17 00:00:00 2001 From: jar-stripe Date: Fri, 25 Oct 2024 11:25:16 -0700 Subject: [PATCH] Do not allow setting Stripe.stripeVersion (#1909) updated Stripe.java to make apiVersion final, and add a stripeVersionWithBetaHeaders variable and public getter for use by e.g. RequestOptions updated tests, added clearBetaVersion method to Stripe with a comment explaining its test only --- src/main/java/com/stripe/Stripe.java | 25 ++++++--- .../model/EventDataObjectDeserializer.java | 18 +++--- .../java/com/stripe/net/RequestOptions.java | 4 +- src/test/java/com/stripe/BaseStripeTest.java | 5 +- .../com/stripe/functional/StripeTest.java | 6 +- .../EventDataObjectDeserializerTest.java | 56 +++++++++++-------- .../com/stripe/net/RequestOptionsTest.java | 18 ------ .../com/stripe/net/StripeRequestTest.java | 3 +- 8 files changed, 66 insertions(+), 69 deletions(-) diff --git a/src/main/java/com/stripe/Stripe.java b/src/main/java/com/stripe/Stripe.java index 6b280d2a27b..a55e406e4f3 100644 --- a/src/main/java/com/stripe/Stripe.java +++ b/src/main/java/com/stripe/Stripe.java @@ -4,6 +4,7 @@ import java.net.Proxy; import java.util.HashMap; import java.util.Map; +import lombok.Getter; public abstract class Stripe { public static final int DEFAULT_CONNECT_TIMEOUT = 30 * 1000; @@ -21,24 +22,30 @@ public abstract class Stripe { public static volatile boolean enableTelemetry = true; /** - * Stripe API version which is sent by default on requests. This can be updated to include beta - * headers. - * - *

Pointing to different API versions than {@code API_VERSION} can lead to deserialziation - * errors and should be avoided. + * Stripe API version which is sent by default on requests. This is set to the API version that is + * linked to this SDK version. Call {@link Stripe#addBetaVersion(String,String)} to add beta + * version information. */ - public static volatile String stripeVersion = API_VERSION; + public static final String stripeVersion = API_VERSION; + + // Used to set the default version in RequestOptions + @Getter private static String stripeVersionWithBetaHeaders = stripeVersion; /** Add a specified beta to the global Stripe API Version. Only call this method once per beta. */ public static void addBetaVersion(String betaName, String betaVersion) { - if (stripeVersion.indexOf("; " + betaName + "=") >= 0) { + if (stripeVersionWithBetaHeaders.indexOf("; " + betaName + "=") >= 0) { throw new RuntimeException( String.format( "Stripe version header %s already contains entry for beta %s", - stripeVersion, betaName)); + stripeVersionWithBetaHeaders, betaName)); } - stripeVersion = String.format("%s; %s=%s", stripeVersion, betaName, betaVersion); + stripeVersionWithBetaHeaders = String.format("%s; %s=%s", stripeVersion, betaName, betaVersion); + } + + // For testing only. This is not part of a stable API and could change in non-major versions. + public static void clearBetaVersion() { + stripeVersionWithBetaHeaders = stripeVersion; } // Note that URLConnection reserves the value of 0 to mean "infinite diff --git a/src/main/java/com/stripe/model/EventDataObjectDeserializer.java b/src/main/java/com/stripe/model/EventDataObjectDeserializer.java index 49d710998b6..d82921276b7 100644 --- a/src/main/java/com/stripe/model/EventDataObjectDeserializer.java +++ b/src/main/java/com/stripe/model/EventDataObjectDeserializer.java @@ -165,7 +165,10 @@ public StripeObject deserializeUnsafe() throws EventDataObjectDeserializationExc + "consider transforming the raw JSON data object to be compatible with this " + "current model class schemas using `deserializeUnsafeWith`. " + "Original error message: %s", - Stripe.stripeVersion, this.apiVersion, Stripe.stripeVersion, e.getMessage()); + Stripe.getStripeVersionWithBetaHeaders(), + this.apiVersion, + Stripe.getStripeVersionWithBetaHeaders(), + e.getMessage()); } else { errorMessage = String.format( @@ -195,15 +198,10 @@ public StripeObject deserializeUnsafeWith(CompatibilityTransformer transformer) } private boolean apiVersionMatch() { - // Trim the locally configured API version to not include beta headers, since the payload won't - // have any. - String localApiVersion = StringUtils.trimApiVersion(Stripe.stripeVersion); - String eventApiVersion = StringUtils.trimApiVersion(this.apiVersion); - // Preserved for testing; we have tests that hook getIntegrationApiVersion - // to test with other api versions. - if (!localApiVersion.contains(".")) { - return eventApiVersion.equals(localApiVersion); + String eventApiVersion = StringUtils.trimApiVersion(this.apiVersion); + if (!Stripe.API_VERSION.contains(".")) { + return eventApiVersion.equals(Stripe.API_VERSION); } // If the event api version is from before we started adding @@ -215,7 +213,7 @@ private boolean apiVersionMatch() { // versions are yyyy-MM-dd.releaseIdentifier String eventReleaseTrain = eventApiVersion.split("\\.", 2)[1]; - String currentReleaseTrain = localApiVersion.split("\\.", 2)[1]; + String currentReleaseTrain = Stripe.API_VERSION.split("\\.", 2)[1]; return eventReleaseTrain.equals(currentReleaseTrain); } diff --git a/src/main/java/com/stripe/net/RequestOptions.java b/src/main/java/com/stripe/net/RequestOptions.java index 940a774966e..68eb8bb9474 100644 --- a/src/main/java/com/stripe/net/RequestOptions.java +++ b/src/main/java/com/stripe/net/RequestOptions.java @@ -17,8 +17,8 @@ public class RequestOptions { private final String stripeAccount; private final String baseUrl; - /** Uses the globally set version by defaeult, unless an override is provided. */ - private final String stripeVersion = Stripe.stripeVersion; + /** Uses the globally set version by default, unless an override is provided. */ + private final String stripeVersion = Stripe.getStripeVersionWithBetaHeaders(); /** * Stripe version override when made on behalf of others. This can be used when the returned diff --git a/src/test/java/com/stripe/BaseStripeTest.java b/src/test/java/com/stripe/BaseStripeTest.java index f86848f378a..3aeb3221c13 100644 --- a/src/test/java/com/stripe/BaseStripeTest.java +++ b/src/test/java/com/stripe/BaseStripeTest.java @@ -46,7 +46,6 @@ public class BaseStripeTest { private String origApiKey; private String origClientId; private String origUploadBase; - private String origStripeVersion; protected static final String TEST_API_KEY = "sk_test_123"; static { @@ -117,7 +116,8 @@ public void setUpStripeMockUsage() { this.origUploadBase = Stripe.getUploadBase(); this.origApiKey = Stripe.apiKey; this.origClientId = Stripe.clientId; - this.origStripeVersion = Stripe.stripeVersion; + + Stripe.clearBetaVersion(); Stripe.overrideApiBase("http://localhost:" + port); Stripe.overrideUploadBase("http://localhost:" + port); @@ -145,7 +145,6 @@ public void tearDownStripeMockUsage() { Stripe.overrideUploadBase(this.origUploadBase); Stripe.apiKey = this.origApiKey; Stripe.clientId = this.origClientId; - Stripe.stripeVersion = this.origStripeVersion; } /** diff --git a/src/test/java/com/stripe/functional/StripeTest.java b/src/test/java/com/stripe/functional/StripeTest.java index 8edf4618a1d..451797fbdd2 100644 --- a/src/test/java/com/stripe/functional/StripeTest.java +++ b/src/test/java/com/stripe/functional/StripeTest.java @@ -1,6 +1,7 @@ package com.stripe.functional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import com.stripe.BaseStripeTest; @@ -11,9 +12,10 @@ public class StripeTest extends BaseStripeTest { @Test public void testAddBetaVersion() { - Stripe.stripeVersion = "2024-02-23"; Stripe.addBetaVersion("super_cool_beta", "v1"); - assertEquals(Stripe.stripeVersion, "2024-02-23; super_cool_beta=v1"); + assertNotEquals(Stripe.stripeVersion, Stripe.API_VERSION + "; super_cool_beta=v1"); + assertEquals( + Stripe.getStripeVersionWithBetaHeaders(), Stripe.API_VERSION + "; super_cool_beta=v1"); assertThrows( RuntimeException.class, () -> { diff --git a/src/test/java/com/stripe/model/EventDataObjectDeserializerTest.java b/src/test/java/com/stripe/model/EventDataObjectDeserializerTest.java index a828387a7be..1a5306afa04 100644 --- a/src/test/java/com/stripe/model/EventDataObjectDeserializerTest.java +++ b/src/test/java/com/stripe/model/EventDataObjectDeserializerTest.java @@ -16,7 +16,6 @@ public class EventDataObjectDeserializerTest extends BaseStripeTest { private static final String OLD_EVENT_VERSION = "2013-08-15"; - private static final String CURRENT_EVENT_VERSION = "2017-08-15"; private static final String NO_MATCH_VERSION = "2000-08-15"; private void verifyDeserializedStripeObject(StripeObject stripeObject) { @@ -29,7 +28,15 @@ private void verifyDeserializedStripeObject(StripeObject stripeObject) { } private String getCurrentEventStringFixture() throws IOException { - return getResourceAsString("/api_fixtures/customer_created.json"); + String eventJson = getResourceAsString("/api_fixtures/customer_created.json"); + return eventJson.replace( + "\"api_version\": \"2017-08-15\",", "\"api_version\": \"" + Stripe.API_VERSION + "\","); + } + + private String getNoMatchEventStringFixture() throws IOException { + String eventJson = getResourceAsString("/api_fixtures/customer_created.json"); + return eventJson.replace( + "\"api_version\": \"2017-08-15\",", "\"api_version\": \"" + NO_MATCH_VERSION + "\","); } private String getOldEventStringFixture() throws IOException { @@ -44,9 +51,8 @@ public void testDeserializeOnApiVersionMatch() throws Exception { assertNotNull(event); assertNotNull(event.getId()); - assertEquals(CURRENT_EVENT_VERSION, event.getApiVersion()); + assertEquals(Stripe.API_VERSION, event.getApiVersion()); - Stripe.stripeVersion = CURRENT_EVENT_VERSION; EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertTrue(deserializer.getObject().isPresent()); @@ -55,15 +61,14 @@ public void testDeserializeOnApiVersionMatch() throws Exception { @Test public void testDeserializeUnsafeOnApiVersionMismatch() throws Exception { - final String data = getCurrentEventStringFixture(); + final String data = getNoMatchEventStringFixture(); final Event event = ApiResource.GSON.fromJson(data, Event.class); assertNotNull(event); assertNotNull(event.getId()); - assertNotEquals(NO_MATCH_VERSION, event.getApiVersion()); + assertNotEquals(Stripe.API_VERSION, event.getApiVersion()); - Stripe.stripeVersion = NO_MATCH_VERSION; EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertFalse(deserializer.getObject().isPresent()); @@ -78,10 +83,9 @@ public void testDeserializeUnsafeOnApiVersionMismatch() throws Exception { public void testDeserializeUnsafeDoesNotMutateState() throws IOException, EventDataObjectDeserializationException { - final String data = getCurrentEventStringFixture(); + final String data = getNoMatchEventStringFixture(); final Event event = ApiResource.GSON.fromJson(data, Event.class); - Stripe.stripeVersion = NO_MATCH_VERSION; EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertFalse(deserializer.getObject().isPresent()); @@ -94,12 +98,20 @@ public void testDeserializeUnsafeDoesNotMutateState() @Test public void testFailureOnApiVersionMatch() throws Exception { - final String data = getOldEventStringFixture(); + // This is a little funky: basically, it is used to test what happens if + // the versions match but we encounter an error parsing the event which + // will occur if we use the data format of the old event fixture. + // This was more of a risk when Stripe.stripeVersion was overrideable + // but even still, the case is worth testing. + final String data = + getOldEventStringFixture() + .replace( + "\"api_version\": \"" + OLD_EVENT_VERSION + "\",", + "\"api_version\": \"" + Stripe.API_VERSION + "\","); final Event event = ApiResource.GSON.fromJson(data, Event.class); - assertEquals(OLD_EVENT_VERSION, event.getApiVersion()); + assertEquals(Stripe.API_VERSION, event.getApiVersion()); - Stripe.stripeVersion = OLD_EVENT_VERSION; EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertFalse(deserializer.getObject().isPresent()); @@ -128,7 +140,6 @@ public void testFailureOnApiVersionMisMatch() throws Exception { assertEquals(OLD_EVENT_VERSION, event.getApiVersion()); - Stripe.stripeVersion = NO_MATCH_VERSION; EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertFalse(deserializer.getObject().isPresent()); @@ -137,7 +148,7 @@ public void testFailureOnApiVersionMisMatch() throws Exception { deserializer.deserializeUnsafe(); fail("Expect event data deserialization failure."); } catch (EventDataObjectDeserializationException e) { - assertTrue(e.getMessage().contains("Stripe API version " + NO_MATCH_VERSION)); + assertTrue(e.getMessage().contains("Stripe API version " + Stripe.API_VERSION)); assertTrue(e.getMessage().contains("event data object has " + OLD_EVENT_VERSION)); } } @@ -147,9 +158,9 @@ public void testIgnoresBetaHeadersInLibraryVersionWhenDeterminingMatch() throws final String data = getCurrentEventStringFixture(); final Event event = ApiResource.GSON.fromJson(data, Event.class); - assertEquals(CURRENT_EVENT_VERSION, event.getApiVersion()); + assertEquals(Stripe.API_VERSION, event.getApiVersion()); - Stripe.stripeVersion = CURRENT_EVENT_VERSION + "; some_beta=v1; some_beta=v2"; + Stripe.addBetaVersion("some_beta", "v1"); EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertTrue(deserializer.getObject().isPresent()); @@ -160,9 +171,8 @@ public void testIgnoresBetaHeadersInLibraryVersionWhenDeterminingMatch() throws public void testIgnoresBetaHeadersInEventVersionWhenDeterminingMatch() throws Exception { final String data = getCurrentEventStringFixture(); final Event event = ApiResource.GSON.fromJson(data, Event.class); - event.apiVersion = CURRENT_EVENT_VERSION + "; some_beta=v1; some_beta=v2"; + event.apiVersion = Stripe.API_VERSION + "; some_beta=v1"; - Stripe.stripeVersion = CURRENT_EVENT_VERSION; EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertTrue(deserializer.getObject().isPresent()); @@ -171,12 +181,12 @@ public void testIgnoresBetaHeadersInEventVersionWhenDeterminingMatch() throws Ex @Test public void testApiVersionMismatchReportedEvenWhenBetaHeadersPresent() throws Exception { - final String data = getCurrentEventStringFixture(); + final String data = getOldEventStringFixture(); final Event event = ApiResource.GSON.fromJson(data, Event.class); - assertEquals(CURRENT_EVENT_VERSION, event.getApiVersion()); + assertEquals(OLD_EVENT_VERSION, event.getApiVersion()); - Stripe.stripeVersion = OLD_EVENT_VERSION + "; some_beta=v1; some_beta=v2"; + Stripe.addBetaVersion("some_beta", "v1"); EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertFalse(deserializer.getObject().isPresent()); @@ -187,7 +197,6 @@ public void testDeserializeUnsafeWith() throws Exception { final String data = getOldEventStringFixture(); final Event event = ApiResource.GSON.fromJson(data, Event.class); - Stripe.stripeVersion = NO_MATCH_VERSION; EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertFalse(deserializer.getObject().isPresent()); @@ -229,8 +238,7 @@ public void testDeserializeSetsResponseGetter() throws Exception { final String data = getCurrentEventStringFixture(); final Event event = ApiResource.GSON.fromJson(data, Event.class); - Stripe.stripeVersion = CURRENT_EVENT_VERSION; - assertEquals(CURRENT_EVENT_VERSION, event.getApiVersion()); + assertEquals(Stripe.API_VERSION, event.getApiVersion()); EventDataObjectDeserializer deserializer = event.getDataObjectDeserializer(); assertTrue(deserializer.getObject().isPresent()); diff --git a/src/test/java/com/stripe/net/RequestOptionsTest.java b/src/test/java/com/stripe/net/RequestOptionsTest.java index de4fdd3569a..15970137fff 100644 --- a/src/test/java/com/stripe/net/RequestOptionsTest.java +++ b/src/test/java/com/stripe/net/RequestOptionsTest.java @@ -50,24 +50,6 @@ public void testPersistentValuesInToBuilder() { assertNull(optsRebuilt.getBaseUrl()); } - @Test - public void testUsesGlobalStripeVersionWithDefault() { - String originalVersion = Stripe.stripeVersion; - assertEquals(originalVersion, RequestOptions.getDefault().getStripeVersion()); - - Stripe.stripeVersion = "2022-08-19"; - assertEquals("2022-08-19", RequestOptions.getDefault().getStripeVersion()); - } - - @Test - public void testUsesGlobalStripeVersionWithBuilder() { - String originalVersion = Stripe.stripeVersion; - assertEquals(originalVersion, RequestOptions.builder().build().getStripeVersion()); - - Stripe.stripeVersion = "2022-08-19"; - assertEquals("2022-08-19", RequestOptions.builder().build().getStripeVersion()); - } - @Test public void testToBuilderFullCopy() { RequestOptions opts = diff --git a/src/test/java/com/stripe/net/StripeRequestTest.java b/src/test/java/com/stripe/net/StripeRequestTest.java index cac8f036b6b..564d3b438df 100644 --- a/src/test/java/com/stripe/net/StripeRequestTest.java +++ b/src/test/java/com/stripe/net/StripeRequestTest.java @@ -1,3 +1,5 @@ +package com.stripe.net; + import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -8,7 +10,6 @@ import com.stripe.Stripe; import com.stripe.exception.AuthenticationException; import com.stripe.exception.StripeException; -import com.stripe.net.*; import com.stripe.net.RequestOptions.RequestOptionsBuilder; import com.stripe.param.common.EmptyParam; import java.nio.charset.StandardCharsets;