Skip to content

Commit

Permalink
Merge c580894 into ec8c0b9
Browse files Browse the repository at this point in the history
  • Loading branch information
guperrot committed Nov 17, 2018
2 parents ec8c0b9 + c580894 commit b1f64c9
Show file tree
Hide file tree
Showing 13 changed files with 249 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ private void queuePage(String name, Map<String, String> properties) {
* @param flags optional flags.
*/
private synchronized void trackEventAsync(final String name, final List<TypedProperty> properties, final AnalyticsTransmissionTarget transmissionTarget, final int flags) {
final String userId = AppCenter.getInstance().getUserId();
post(new Runnable() {

@Override
Expand All @@ -759,6 +760,9 @@ public void run() {
if (aTransmissionTarget.isEnabled()) {
eventLog.addTransmissionTarget(aTransmissionTarget.getTransmissionTargetToken());
eventLog.setTag(aTransmissionTarget);
if (aTransmissionTarget == mDefaultTransmissionTarget) {
eventLog.setUserId(userId);
}
} else {
AppCenterLog.error(LOG_TAG, "This transmission target is disabled.");
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ abstract class AbstractAnalyticsTest {
@Mock
AppCenterHandler mAppCenterHandler;

@Mock
AppCenter mAppCenter;

@Mock
private AppCenterFuture<Boolean> mCoreEnabledFuture;

Expand All @@ -46,6 +49,7 @@ public void setUp() {
mockStatic(AppCenterLog.class);
mockStatic(AppCenter.class);
when(AppCenter.isConfigured()).thenReturn(true);
when(AppCenter.getInstance()).thenReturn(mAppCenter);
when(AppCenter.isEnabled()).thenReturn(mCoreEnabledFuture);
when(mCoreEnabledFuture.get()).thenReturn(true);
Answer<Void> runNow = new Answer<Void>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,45 @@ public void trackEventWithInvalidFlags() {
AppCenterLog.warn(eq(AppCenter.LOG_TAG), anyString());
}

@Test
public void trackEventWithUserIdWhenConfiguredForTarget() {
when(mAppCenter.getUserId()).thenReturn("c:alice");
Analytics analytics = Analytics.getInstance();
Channel channel = mock(Channel.class);
analytics.onStarting(mAppCenterHandler);
analytics.onStarted(mock(Context.class), channel, null, "target", true);
Analytics.trackEvent("eventName1");
ArgumentCaptor<EventLog> eventLogArgumentCaptor = ArgumentCaptor.forClass(EventLog.class);
verify(channel).enqueue(eventLogArgumentCaptor.capture(), anyString(), eq(DEFAULTS));
assertEquals("c:alice", eventLogArgumentCaptor.getValue().getUserId());
}

@Test
public void trackEventWithoutUserIdWhenConfiguredForAppSecretOnly() {
when(mAppCenter.getUserId()).thenReturn("alice");
Analytics analytics = Analytics.getInstance();
Channel channel = mock(Channel.class);
analytics.onStarting(mAppCenterHandler);
analytics.onStarted(mock(Context.class), channel, "appSecret", null, true);
Analytics.trackEvent("eventName1");
ArgumentCaptor<EventLog> eventLogArgumentCaptor = ArgumentCaptor.forClass(EventLog.class);
verify(channel).enqueue(eventLogArgumentCaptor.capture(), anyString(), eq(DEFAULTS));
assertNull(eventLogArgumentCaptor.getValue().getUserId());
}

@Test
public void trackEventWithoutUserIdWhenConfiguredForBothSecrets() {
when(mAppCenter.getUserId()).thenReturn("c:alice");
Analytics analytics = Analytics.getInstance();
Channel channel = mock(Channel.class);
analytics.onStarting(mAppCenterHandler);
analytics.onStarted(mock(Context.class), channel, "appSecret", "target", true);
Analytics.trackEvent("eventName1");
ArgumentCaptor<EventLog> eventLogArgumentCaptor = ArgumentCaptor.forClass(EventLog.class);
verify(channel).enqueue(eventLogArgumentCaptor.capture(), anyString(), eq(DEFAULTS));
assertEquals("c:alice", eventLogArgumentCaptor.getValue().getUserId());
}

@Test
public void trackEventFromLibrary() {
Analytics analytics = Analytics.getInstance();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ public void serializeAndDeserialize() throws JSONException {
/* User extension. */
log.getExt().setUser(new UserExtension());
checkSerialization(serializer, log);
log.getExt().getUser().setLocalId("d:1234");
checkSerialization(serializer, log);
log.getExt().getUser().setLocale("en-US");
checkSerialization(serializer, log);

Expand All @@ -97,6 +99,8 @@ public void serializeAndDeserialize() throws JSONException {
checkSerialization(serializer, log);
log.getExt().getApp().setLocale("fr-FR");
checkSerialization(serializer, log);
log.getExt().getApp().setUserId("c:charlie");
checkSerialization(serializer, log);

/* Net extension. */
log.getExt().setNet(new NetExtension());
Expand Down
24 changes: 18 additions & 6 deletions sdk/appcenter/src/main/java/com/microsoft/appcenter/AppCenter.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@
import static com.microsoft.appcenter.Constants.DEFAULT_TRIGGER_COUNT;
import static com.microsoft.appcenter.Constants.DEFAULT_TRIGGER_INTERVAL;
import static com.microsoft.appcenter.Constants.DEFAULT_TRIGGER_MAX_PARALLEL_REQUESTS;
import static com.microsoft.appcenter.Constants.USER_ID_MAX_LENGTH;
import static com.microsoft.appcenter.Constants.USER_ID_APP_CENTER_MAX_LENGTH;
import static com.microsoft.appcenter.Constants.USER_ID_ONE_COLLECTOR_PATTERN;
import static com.microsoft.appcenter.utils.AppCenterLog.NONE;

public class AppCenter {
Expand Down Expand Up @@ -418,14 +419,25 @@ public static AppCenterFuture<Boolean> setMaxStorageSize(long storageSizeInBytes
* {@link #setUserId(String)} implementation at instance level.
*/
private synchronized void setInstanceUserId(String userId) {
if (!checkPrecondition()) {
if (!mConfiguredFromApp) {
AppCenterLog.error(LOG_TAG, "AppCenter must be configured from application, libraries cannot use call setUserId.");
return;
}
if (mAppSecret != null && userId != null && userId.length() > USER_ID_MAX_LENGTH) {
AppCenterLog.error(LOG_TAG, "userId is limited to " + USER_ID_MAX_LENGTH + " characters.");
} else {
mUserId = userId;
if (mAppSecret == null && mTransmissionTargetToken == null) {
AppCenterLog.error(LOG_TAG, "AppCenter must be configured with a secret from application to call setUserId.");
return;
}
if (userId != null) {
if (mAppSecret != null && userId.length() > USER_ID_APP_CENTER_MAX_LENGTH) {
AppCenterLog.error(LOG_TAG, "userId is limited to " + USER_ID_APP_CENTER_MAX_LENGTH + " characters.");
return;
}
if (mTransmissionTargetToken != null && !USER_ID_ONE_COLLECTOR_PATTERN.matcher(userId).matches()) {
AppCenterLog.error(LOG_TAG, "userId must match the " + USER_ID_ONE_COLLECTOR_PATTERN + " regular expression.");
return;
}
}
mUserId = userId;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.microsoft.appcenter.utils.AppCenterLog;

import java.io.File;
import java.util.regex.Pattern;

/**
* Various constants and meta information loaded from the context.
Expand All @@ -30,7 +31,12 @@ public class Constants {
/**
* Maximum allowed length for user identifier for App Center server.
*/
static final int USER_ID_MAX_LENGTH = 256;
static final int USER_ID_APP_CENTER_MAX_LENGTH = 256;

/**
* Valid application userId for One Collector.
*/
static final Pattern USER_ID_ONE_COLLECTOR_PATTERN = Pattern.compile("[cidw]:.*");

/**
* Path where crash logs and temporary files are stored.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ public class AppExtension implements Model {
*/
private static final String LOCALE = "locale";

/**
* User ID.
*/
private static final String USER_ID = "userId";

/**
* Application identifier.
*/
Expand All @@ -52,6 +57,11 @@ public class AppExtension implements Model {
*/
private String locale;

/**
* User ID.
*/
private String userId;

/**
* Get application id.
*
Expand Down Expand Up @@ -125,12 +135,31 @@ public void setLocale(String locale) {
this.locale = locale;
}

/**
* Get user ID.
*
* @return user ID.
*/
public String getUserId() {
return userId;
}

/**
* Set user ID.
*
* @param userId user ID.
*/
public void setUserId(String userId) {
this.userId = userId;
}

@Override
public void read(JSONObject object) {
setId(object.optString(ID, null));
setVer(object.optString(VER, null));
setName(object.optString(NAME, null));
setLocale(object.optString(LOCALE, null));
setUserId(object.optString(USER_ID, null));
}

@Override
Expand All @@ -139,9 +168,9 @@ public void write(JSONStringer writer) throws JSONException {
JSONUtils.write(writer, VER, getVer());
JSONUtils.write(writer, NAME, getName());
JSONUtils.write(writer, LOCALE, getLocale());
JSONUtils.write(writer, USER_ID, getUserId());
}

@SuppressWarnings("SimplifiableIfStatement")
@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand All @@ -152,7 +181,8 @@ public boolean equals(Object o) {
if (id != null ? !id.equals(that.id) : that.id != null) return false;
if (ver != null ? !ver.equals(that.ver) : that.ver != null) return false;
if (name != null ? !name.equals(that.name) : that.name != null) return false;
return locale != null ? locale.equals(that.locale) : that.locale == null;
if (locale != null ? !locale.equals(that.locale) : that.locale != null) return false;
return userId != null ? userId.equals(that.userId) : that.userId == null;
}

@Override
Expand All @@ -161,6 +191,7 @@ public int hashCode() {
result = 31 * result + (ver != null ? ver.hashCode() : 0);
result = 31 * result + (name != null ? name.hashCode() : 0);
result = 31 * result + (locale != null ? locale.hashCode() : 0);
result = 31 * result + (userId != null ? userId.hashCode() : 0);
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public static void setName(CommonSchemaLog log, String name) throws IllegalArgum
/**
* Adds part A extension to common schema log from device object in Log.
*
* @param src source log.
* @param dest destination common schema log.
* @param src source log.
* @param dest destination common schema log.
* @param transmissionTarget transmission target to use.
*/
public static void addPartAFromLog(Log src, CommonSchemaLog dest, String transmissionTarget) {
Expand All @@ -58,6 +58,7 @@ public static void addPartAFromLog(Log src, CommonSchemaLog dest, String transmi
/* Add top level part A fields. */
dest.setVer("3.0");
dest.setTimestamp(src.getTimestamp());

/* TODO: We should cache the ikey for transmission target */
dest.setIKey("o:" + getTargetKey(transmissionTarget));

Expand Down Expand Up @@ -89,6 +90,9 @@ public static void addPartAFromLog(Log src, CommonSchemaLog dest, String transmi
dest.getExt().getApp().setVer(device.getAppVersion());
dest.getExt().getApp().setId("a:" + device.getAppNamespace());

/* We use userId from AppCenter.setUserId into app.userId and leave user.localId null for now. */
dest.getExt().getApp().setUserId(src.getUserId());

/* TODO: Add network type. */
/* Add net extension. */
dest.getExt().setNet(new NetExtension());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,46 @@
*/
public class UserExtension implements Model {

/**
* LocalId property.
*/
private static final String LOCAL_ID = "localId";

/**
* Locale property.
*/
private static final String LOCALE = "locale";

/**
* Local Id.
*/
private String localId;

/**
* User locale.
*/
private String locale;

/**
* Get localId.
*
* @return localId.
*/
@SuppressWarnings("WeakerAccess")
public String getLocalId() {
return localId;
}

/**
* set localId.
*
* @param localId localId.
*/
@SuppressWarnings("WeakerAccess")
public void setLocalId(String localId) {
this.localId = localId;
}

/**
* Get user locale.
*
Expand All @@ -42,11 +72,13 @@ public void setLocale(String locale) {

@Override
public void read(JSONObject object) {
setLocalId(object.optString(LOCAL_ID, null));
setLocale(object.optString(LOCALE, null));
}

@Override
public void write(JSONStringer writer) throws JSONException {
JSONUtils.write(writer, LOCAL_ID, getLocalId());
JSONUtils.write(writer, LOCALE, getLocale());
}

Expand All @@ -57,11 +89,14 @@ public boolean equals(Object o) {

UserExtension that = (UserExtension) o;

if (localId != null ? !localId.equals(that.localId) : that.localId != null) return false;
return locale != null ? locale.equals(that.locale) : that.locale == null;
}

@Override
public int hashCode() {
return locale != null ? locale.hashCode() : 0;
int result = localId != null ? localId.hashCode() : 0;
result = 31 * result + (locale != null ? locale.hashCode() : 0);
return result;
}
}
Loading

0 comments on commit b1f64c9

Please sign in to comment.