Skip to content
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

Ensure assignment date is in ISO8601 format (FF-1933) #46

Merged
merged 1 commit into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eppo/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ plugins {
}

group = "cloud.eppo"
version = "1.0.7"
version = "1.0.8"

android {
compileSdk 33
Expand Down
16 changes: 11 additions & 5 deletions eppo/src/main/java/cloud/eppo/android/EppoClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ public class EppoClient {
// Useful for testing in situations where we want to mock the http client
private static EppoHttpClient httpClientOverride = null;

private EppoClient(Application application, String apiKey, String host, AssignmentLogger assignmentLogger, boolean isGracefulMode) {
private EppoClient(Application application, String apiKey, String host, AssignmentLogger assignmentLogger,
boolean isGracefulMode) {
EppoHttpClient httpClient = httpClientOverride == null ? new EppoHttpClient(host, apiKey) : httpClientOverride;
ConfigurationStore configStore = new ConfigurationStore(application);
requestor = new ConfigurationRequestor(configStore, httpClient);
Expand Down Expand Up @@ -133,7 +134,7 @@ protected EppoValue getTypedAssignment(String subjectKey, String flagKey, Subjec
String allocationKey = rule.getAllocationKey();
Allocation allocation = flag.getAllocations().get(allocationKey);
if (allocation == null) {
Log.w(TAG, "unexpected unknown allocation key \""+allocationKey+"\"");
Log.w(TAG, "unexpected unknown allocation key \"" + allocationKey + "\"");
return null;
}

Expand All @@ -157,9 +158,14 @@ protected EppoValue getTypedAssignment(String subjectKey, String flagKey, Subjec
variationToLog = typedValue.stringValue();
}

Assignment assignment = new Assignment(experimentKey,
flagKey, allocationKey, variationToLog,
subjectKey, Utils.getISODate(new Date()), subjectAttributes);
Assignment assignment = Assignment.createWithCurrentDate(
experimentKey,
flagKey,
allocationKey,
variationToLog,
subjectKey,
subjectAttributes
);
assignmentLogger.logAssignment(assignment);
}

Expand Down
17 changes: 15 additions & 2 deletions eppo/src/main/java/cloud/eppo/android/logging/Assignment.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package cloud.eppo.android.logging;

import cloud.eppo.android.dto.SubjectAttributes;
import cloud.eppo.android.util.Utils;

import java.util.Date;

public class Assignment {
private String experiment;
Expand All @@ -18,8 +21,7 @@ public Assignment(
String variation,
String subject,
String timestamp,
SubjectAttributes subjectAttributes
) {
SubjectAttributes subjectAttributes) {
this.experiment = experiment;
this.featureFlag = featureFlag;
this.allocation = allocation;
Expand All @@ -29,6 +31,17 @@ public Assignment(
this.subjectAttributes = subjectAttributes;
}

public static Assignment createWithCurrentDate(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

String experiment,
String featureFlag,
String allocation,
String variation,
String subject,
SubjectAttributes subjectAttributes) {
return new Assignment(experiment, featureFlag, allocation, variation, subject,
Utils.getISODate(new Date()), subjectAttributes);
}

public String getExperiment() {
return experiment;
}
Expand Down
10 changes: 4 additions & 6 deletions eppo/src/main/java/cloud/eppo/android/util/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
import java.math.BigInteger;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.TimeZone;
import java.util.Locale;

import cloud.eppo.android.dto.ShardRange;

Expand Down Expand Up @@ -50,10 +49,9 @@ public static void validateNotEmptyOrNull(String input, String errorMessage) {
}

public static String getISODate(Date date) {
TimeZone tz = TimeZone.getTimeZone("UTC");
DateFormat df = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'");
df.setTimeZone(tz);
return df.format(date);
SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'", Locale.US);
Copy link
Member Author

Choose a reason for hiding this comment

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

Add locale argument to SimpleDateFormat constructor

dateFormat.setTimeZone(java.util.TimeZone.getTimeZone("UTC"));
return dateFormat.format(date);
}

public static SharedPreferences getSharedPrefs(Context context) {
Expand Down
62 changes: 62 additions & 0 deletions eppo/src/test/java/cloud/eppo/android/UtilsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package cloud.eppo.android;

import org.junit.Test;
import static org.junit.Assert.*;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Locale;
import java.util.TimeZone;

import cloud.eppo.android.util.Utils;

public class UtilsTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

🙌


@Test
public void testGetISODate() {
String isoDate = Utils.getISODate(new Date());
assertNotNull("ISO date should not be null", isoDate);

// Verify the format
SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'", Locale.US);
dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
try {
Date date = dateFormat.parse(isoDate);
assertNotNull("Parsed date should not be null", date);

// Optionally, verify the date is not too far from the current time
long currentTime = System.currentTimeMillis();
long parsedTime = date.getTime();
assertTrue("The parsed date should be within a reasonable range of the current time",
Math.abs(currentTime - parsedTime) < 10000); // for example, within 10 seconds
} catch (ParseException e) {
fail("Parsing the ISO date failed: " + e.getMessage());
}
}

@Test
public void testGetCurrentDateISOInDifferentLocale() {
// Arrange
Locale defaultLocale = Locale.getDefault();
try {
// Set locale to Arabic
Locale.setDefault(new Locale("ar"));
String isoDate = Utils.getISODate(new Date());
Comment on lines +43 to +44
Copy link
Member Author

Choose a reason for hiding this comment

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

Ran this test without the fix:

٢٠٢٤-٠٤-١٥T٢٣:٤١:٣٥Z

java.lang.AssertionError: Date should be in ISO 8601 format
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at cloud.eppo.android.UtilsTest.testGetCurrentDateISOInDifferentLocale(UtilsTest.java:54)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.base/java.lang.reflect.Method.invoke(Unknown Source)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:110)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:38)
	at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:62)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.base/java.lang.reflect.Method.invoke(Unknown Source)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at jdk.proxy1/jdk.proxy1.$Proxy2.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:176)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)


cloud.eppo.android.UtilsTest > testGetCurrentDateISOInDifferentLocale FAILED
    java.lang.AssertionError at UtilsTest.java:54


// Act
// Check if the date is in the correct ISO 8601 format
// This is a simple regex check to see if the string follows the
// YYYY-MM-DDTHH:MM:SSZ pattern
boolean isISO8601 = isoDate.matches("\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}Z");

// Assert
assertTrue("Date should be in ISO 8601 format", isISO8601);

} catch (Exception e) {
fail("Test failed with exception: " + e.getMessage());
} finally {
// Reset locale back to original
Locale.setDefault(defaultLocale);
}
}
}
Loading