-
Notifications
You must be signed in to change notification settings - Fork 38
[SDK-568] Add stable cross-SDK identifiers to IterableDataRegion #1081
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
c3652f2
d4c9288
fb4efa7
0de07ac
34475b2
01cf132
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -362,10 +362,21 @@ public Builder setAllowedProtocols(@NonNull String[] allowedProtocols) { | |
|
|
||
| /** | ||
| * Set the data region used by the SDK | ||
| * <p> | ||
| * To resolve a region from a string or numeric identifier (for example when bridging from a | ||
| * cross-platform wrapper), use {@link IterableDataRegion#from(String)} or | ||
| * {@link IterableDataRegion#from(int)}, which fall back to | ||
| * {@link IterableDataRegion#US} and log on unrecognised values. | ||
| * | ||
| * @param dataRegion enum value that determines which endpoint to use, defaults to IterableDataRegion.US | ||
| */ | ||
| @NonNull | ||
| public Builder setDataRegion(@NonNull IterableDataRegion dataRegion) { | ||
| if (dataRegion == null) { | ||
| IterableLogger.w("IterableConfig", "setDataRegion received null, defaulting to " + IterableDataRegion.US.getRegionCode()); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use the static final string TAG here instead of the "magic string"! |
||
| this.dataRegion = IterableDataRegion.US; | ||
| return this; | ||
| } | ||
| this.dataRegion = dataRegion; | ||
| return this; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,109 @@ | ||
| package com.iterable.iterableapi; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
|
|
||
| /** | ||
| * Data region determining which data center and endpoints the SDK sends data to. | ||
| * Defaults to {@link #US}; EU-hosted projects must select {@link #EU}. | ||
| */ | ||
| public enum IterableDataRegion { | ||
| US("https://api.iterable.com/api/"), | ||
| EU("https://api.eu.iterable.com/api/"); | ||
| US(0, "US", "https://api.iterable.com/api/"), | ||
| EU(1, "EU", "https://api.eu.iterable.com/api/"); | ||
|
|
||
| private static final String TAG = "IterableDataRegion"; | ||
|
|
||
| private final int code; | ||
| private final String regionCode; | ||
| private final String endpoint; | ||
|
|
||
| IterableDataRegion(String endpoint) { | ||
| IterableDataRegion(int code, String regionCode, String endpoint) { | ||
| this.code = code; | ||
| this.regionCode = regionCode; | ||
| this.endpoint = endpoint; | ||
| } | ||
|
|
||
| public String getEndpoint() { | ||
| return this.endpoint; | ||
| } | ||
|
|
||
| /** | ||
| * Stable numeric identifier for this region, matching the values used by the React Native and | ||
| * Flutter SDKs. Unlike {@link #ordinal()} this is guaranteed not to shift if regions are added. | ||
| */ | ||
| public int getCode() { | ||
| return this.code; | ||
| } | ||
|
|
||
| /** | ||
| * Stable short identifier for this region (for example {@code "EU"}), matching the values used | ||
| * by Iterable's other SDKs. | ||
| */ | ||
| @NonNull | ||
| public String getRegionCode() { | ||
| return this.regionCode; | ||
| } | ||
|
rtlsilva marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * Resolves a region from its short identifier (e.g. {@code "EU"}), accepting either case. Also | ||
| * accepts a full API endpoint URL, so values from the iOS SDK's string-based data region can be | ||
| * passed through unchanged. | ||
| * <p> | ||
| * Unrecognised or null values fall back to {@link #US} and are logged, matching the behaviour of | ||
| * Iterable's other SDKs. | ||
| * | ||
| * @param value region identifier, or {@code null} | ||
| * @return the matching region, or {@link #US} if the value is not recognised | ||
| */ | ||
| @NonNull | ||
| public static IterableDataRegion from(@Nullable String value) { | ||
| if (value == null || value.trim().isEmpty()) { | ||
| IterableLogger.w(TAG, "No data region specified, defaulting to " + US.regionCode); | ||
| return US; | ||
| } | ||
|
|
||
| String normalized = value.trim(); | ||
| for (IterableDataRegion region : values()) { | ||
| if (normalized.equalsIgnoreCase(region.regionCode) || normalized.equalsIgnoreCase(region.endpoint)) { | ||
| return region; | ||
| } | ||
| } | ||
|
|
||
| IterableLogger.w(TAG, "Unsupported data region \"" + value + "\", defaulting to " + US.regionCode | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These warnings never reach logcat. That makes the check Note this is not fixable by the integrator either: I confirmed this with a probe test using Cursor,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for the catch, this was a good finding and is now fixed. One exception, from(null) / from("") stays at warning. That's the documented default rather than a misconfiguration, and ERROR there would fire for every wrapper app that never sets a region. |
||
| + ". Supported values: " + supportedRegionCodes()); | ||
| return US; | ||
| } | ||
|
|
||
| /** | ||
| * Resolves a region from its numeric identifier, as used by the React Native and Flutter SDKs. | ||
| * <p> | ||
| * Unrecognised values fall back to {@link #US} and are logged, matching the behaviour of | ||
| * Iterable's other SDKs. | ||
| * | ||
| * @param code region identifier, see {@link #getCode()} | ||
| * @return the matching region, or {@link #US} if the code is not recognised | ||
| */ | ||
| @NonNull | ||
| public static IterableDataRegion from(int code) { | ||
| for (IterableDataRegion region : values()) { | ||
| if (region.code == code) { | ||
| return region; | ||
| } | ||
| } | ||
|
|
||
| IterableLogger.w(TAG, "Unsupported data region code " + code + ", defaulting to " + US.regionCode | ||
| + ". Supported values: " + supportedRegionCodes()); | ||
| return US; | ||
| } | ||
|
|
||
| private static String supportedRegionCodes() { | ||
| StringBuilder builder = new StringBuilder(); | ||
| for (IterableDataRegion region : values()) { | ||
| if (builder.length() > 0) { | ||
| builder.append(", "); | ||
| } | ||
| builder.append(region.regionCode).append(" (").append(region.code).append(")"); | ||
| } | ||
| return builder.toString(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| package com.iterable.iterableapi | ||
|
|
||
| import org.hamcrest.Matchers.`is` | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertThat | ||
| import org.junit.Test | ||
|
|
||
| class IterableDataRegionTest { | ||
|
|
||
| @Test | ||
| fun endpointsMatchDataCenters() { | ||
| assertEquals("https://api.iterable.com/api/", IterableDataRegion.US.endpoint) | ||
| assertEquals("https://api.eu.iterable.com/api/", IterableDataRegion.EU.endpoint) | ||
| } | ||
|
|
||
| /** These identifiers are part of the cross-SDK contract; changing them breaks the wrappers. */ | ||
| @Test | ||
| fun stableIdentifiersMatchOtherSdks() { | ||
| assertEquals(0, IterableDataRegion.US.code) | ||
| assertEquals(1, IterableDataRegion.EU.code) | ||
| assertEquals("US", IterableDataRegion.US.regionCode) | ||
| assertEquals("EU", IterableDataRegion.EU.regionCode) | ||
| } | ||
|
|
||
| @Test | ||
| fun fromRegionCode() { | ||
| assertThat(IterableDataRegion.from("US"), `is`(IterableDataRegion.US)) | ||
| assertThat(IterableDataRegion.from("EU"), `is`(IterableDataRegion.EU)) | ||
| } | ||
|
|
||
| @Test | ||
| fun fromRegionCodeIsCaseAndWhitespaceInsensitive() { | ||
| assertThat(IterableDataRegion.from("eu"), `is`(IterableDataRegion.EU)) | ||
| assertThat(IterableDataRegion.from("Eu"), `is`(IterableDataRegion.EU)) | ||
| assertThat(IterableDataRegion.from(" EU "), `is`(IterableDataRegion.EU)) | ||
| } | ||
|
|
||
| /** iOS expresses the data region as the endpoint URL itself, so those values must resolve. */ | ||
| @Test | ||
| fun fromIosStyleEndpointUrl() { | ||
| assertThat( | ||
| IterableDataRegion.from("https://api.eu.iterable.com/api/"), | ||
| `is`(IterableDataRegion.EU) | ||
| ) | ||
| assertThat( | ||
| IterableDataRegion.from("https://api.iterable.com/api/"), | ||
| `is`(IterableDataRegion.US) | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun fromUnsupportedStringFallsBackToUs() { | ||
| assertThat(IterableDataRegion.from("APAC"), `is`(IterableDataRegion.US)) | ||
| assertThat(IterableDataRegion.from(""), `is`(IterableDataRegion.US)) | ||
| assertThat(IterableDataRegion.from(" "), `is`(IterableDataRegion.US)) | ||
| assertThat(IterableDataRegion.from(null as String?), `is`(IterableDataRegion.US)) | ||
| } | ||
|
|
||
| @Test | ||
| fun fromCode() { | ||
| assertThat(IterableDataRegion.from(0), `is`(IterableDataRegion.US)) | ||
| assertThat(IterableDataRegion.from(1), `is`(IterableDataRegion.EU)) | ||
| } | ||
|
|
||
| @Test | ||
| fun fromUnsupportedCodeFallsBackToUs() { | ||
| assertThat(IterableDataRegion.from(2), `is`(IterableDataRegion.US)) | ||
| assertThat(IterableDataRegion.from(-1), `is`(IterableDataRegion.US)) | ||
| assertThat(IterableDataRegion.from(Int.MAX_VALUE), `is`(IterableDataRegion.US)) | ||
| } | ||
|
|
||
| /** Round-trips guard the wrappers, which serialize the region and resolve it back. */ | ||
| @Test | ||
| fun identifiersRoundTrip() { | ||
| for (region in IterableDataRegion.values()) { | ||
| assertThat(IterableDataRegion.from(region.code), `is`(region)) | ||
| assertThat(IterableDataRegion.from(region.regionCode), `is`(region)) | ||
| assertThat(IterableDataRegion.from(region.endpoint), `is`(region)) | ||
| } | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.