Skip to content

Commit

Permalink
Merge pull request #850 from Microsoft/fix/part-a-was-retroactive
Browse files Browse the repository at this point in the history
Fix part A properties to not be applied retro-actively
  • Loading branch information
guperrot committed Oct 22, 2018
2 parents 5ceb7c8 + 7b25a6e commit 81aed43
Show file tree
Hide file tree
Showing 9 changed files with 262 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ public class Analytics extends AbstractAppCenterService {
/**
* The default transmission target.
*/
private AnalyticsTransmissionTarget mDefaultTransmissionTarget;
@VisibleForTesting
AnalyticsTransmissionTarget mDefaultTransmissionTarget;

/**
* Current activity to replay onResume when enabled in foreground.
Expand Down Expand Up @@ -430,21 +431,31 @@ private synchronized AnalyticsTransmissionTarget getInstanceTransmissionTarget(S
AppCenterLog.debug(LOG_TAG, "Returning transmission target found with token " + transmissionTargetToken);
return transmissionTarget;
}
transmissionTarget = new AnalyticsTransmissionTarget(transmissionTargetToken, null);
AppCenterLog.debug(LOG_TAG, "Created transmission target with token " + transmissionTargetToken);
transmissionTarget = createAnalyticsTransmissionTarget(transmissionTargetToken);
mTransmissionTargets.put(transmissionTargetToken, transmissionTarget);
final AnalyticsTransmissionTarget finalTransmissionTarget = transmissionTarget;
postCommandEvenIfDisabled(new Runnable() {

@Override
public void run() {
finalTransmissionTarget.initInBackground(mContext, mChannel);
}
});
return transmissionTarget;
}
}

/**
* Unconditionally create a new transmission target at root level, even if one exists with the given token.
*
* @param transmissionTargetToken the token.
* @return the created target.
*/
private AnalyticsTransmissionTarget createAnalyticsTransmissionTarget(String transmissionTargetToken) {
final AnalyticsTransmissionTarget transmissionTarget = new AnalyticsTransmissionTarget(transmissionTargetToken, null);
AppCenterLog.debug(LOG_TAG, "Created transmission target with token " + transmissionTargetToken);
postCommandEvenIfDisabled(new Runnable() {

@Override
public void run() {
transmissionTarget.initInBackground(mContext, mChannel);
}
});
return transmissionTarget;
}

@Override
public boolean isAppSecretRequired() {
return false;
Expand Down Expand Up @@ -671,6 +682,7 @@ public void run() {
if (aTransmissionTarget != null) {
if (aTransmissionTarget.isEnabled()) {
eventLog.addTransmissionTarget(aTransmissionTarget.getTransmissionTargetToken());
eventLog.setTag(aTransmissionTarget);
} else {
AppCenterLog.error(LOG_TAG, "This transmission target is disabled.");
return;
Expand Down Expand Up @@ -760,7 +772,7 @@ public void onConfigurationUpdated(String appSecret, String transmissionTargetTo
@WorkerThread
private void setDefaultTransmissionTarget(String transmissionTargetToken) {
if (transmissionTargetToken != null) {
mDefaultTransmissionTarget = getInstanceTransmissionTarget(transmissionTargetToken);
mDefaultTransmissionTarget = createAnalyticsTransmissionTarget(transmissionTargetToken);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,14 @@ public void onPreparingLog(@NonNull Log log, @NonNull String groupName) {
* @return true if log should be overridden, false otherwise.
*/
private boolean shouldOverridePartAProperties(@NonNull Log log) {
String targetToken = mTransmissionTarget.getTransmissionTargetToken();
return log instanceof CommonSchemaLog && mTransmissionTarget.isEnabled()
&& log.getTransmissionTargetTokens().contains(targetToken);
return log instanceof CommonSchemaLog && log.getTag() == mTransmissionTarget &&
mTransmissionTarget.isEnabled();
}

/**
* Get app name. Used for checking parents for property inheritance.
*
* @return App name
* @return App name.
*/
private String getAppName() {
return mAppName;
Expand All @@ -149,16 +148,22 @@ private String getAppName() {
/**
* Override common schema Part A property App.Name.
*
* @param appName App name
* @param appName App name.
*/
public void setAppName(String appName) {
mAppName = appName;
public void setAppName(final String appName) {
Analytics.getInstance().postCommandEvenIfDisabled(new Runnable() {

@Override
public void run() {
mAppName = appName;
}
});
}

/**
* Get app version. Used for checking parents for property inheritance.
*
* @return App version
* @return App version.
*/
private String getAppVersion() {
return mAppVersion;
Expand All @@ -169,8 +174,14 @@ private String getAppVersion() {
*
* @param appVersion App version
*/
public void setAppVersion(String appVersion) {
mAppVersion = appVersion;
public void setAppVersion(final String appVersion) {
Analytics.getInstance().postCommandEvenIfDisabled(new Runnable() {

@Override
public void run() {
mAppVersion = appVersion;
}
});
}

/**
Expand All @@ -187,8 +198,14 @@ private String getAppLocale() {
*
* @param appLocale App Locale
*/
public void setAppLocale(String appLocale) {
mAppLocale = appLocale;
public void setAppLocale(final String appLocale) {
Analytics.getInstance().postCommandEvenIfDisabled(new Runnable() {

@Override
public void run() {
mAppLocale = appLocale;
}
});
}

/**
Expand Down Expand Up @@ -260,7 +277,13 @@ public synchronized void removeEventProperty(String key) {
* This does not have any effect on child transmission targets.
*/
public void collectDeviceId() {
mDeviceIdEnabled = true;
Analytics.getInstance().postCommandEvenIfDisabled(new Runnable() {

@Override
public void run() {
mDeviceIdEnabled = true;
}
});
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ public Collection<CommonSchemaLog> toCommonSchemaLogs(Log log) {
/* Properties go to Part C. */
PartCUtils.addPartCFromLog(eventLog.getTypedProperties(), commonSchemaEventLog);
commonSchemaLogs.add(commonSchemaEventLog);

/* Copy tag. */
commonSchemaEventLog.setTag(log.getTag());
}
return commonSchemaLogs;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ private void testTrackEventWithTransmissionTarget(final String defaultToken, boo
mChannel = mock(Channel.class);
analytics.onStarting(mAppCenterHandler);
analytics.onStarted(mock(Context.class), mChannel, null, defaultToken, startFromApp);
AnalyticsTransmissionTarget target = Analytics.getTransmissionTarget("token");
final AnalyticsTransmissionTarget target = Analytics.getTransmissionTarget("token");
assertNotNull(target);

/* Getting a reference to the same target a second time actually returns the same. */
Expand All @@ -128,12 +128,15 @@ public boolean matches(Object item) {
EventLog eventLog = (EventLog) item;
boolean nameAndPropertiesMatch = eventLog.getName().equals("name") && eventLog.getTypedProperties() == null;
boolean tokenMatch;
boolean tagMatch;
if (defaultToken != null) {
tokenMatch = eventLog.getTransmissionTargetTokens().size() == 1 && eventLog.getTransmissionTargetTokens().contains(defaultToken);
tagMatch = Analytics.getInstance().mDefaultTransmissionTarget.equals(eventLog.getTag());
} else {
tokenMatch = eventLog.getTransmissionTargetTokens().isEmpty();
tagMatch = eventLog.getTag() == null;
}
return nameAndPropertiesMatch && tokenMatch;
return nameAndPropertiesMatch && tokenMatch && tagMatch;
}
return false;
}
Expand All @@ -153,7 +156,8 @@ public boolean matches(Object item) {
EventLog eventLog = (EventLog) item;
boolean nameAndPropertiesMatch = eventLog.getName().equals("name") && eventLog.getTypedProperties() == null;
boolean tokenMatch = eventLog.getTransmissionTargetTokens().size() == 1 && eventLog.getTransmissionTargetTokens().contains("token");
return nameAndPropertiesMatch && tokenMatch;
boolean tagMatch = target.equals(eventLog.getTag());
return nameAndPropertiesMatch && tokenMatch && tagMatch;
}
return false;
}
Expand All @@ -177,15 +181,16 @@ public boolean matches(Object item) {
stringTypedProperty.setValue("valid");
typedProperties.add(stringTypedProperty);
boolean tokenMatch = eventLog.getTransmissionTargetTokens().size() == 1 && eventLog.getTransmissionTargetTokens().contains("token2");
return nameMatches && tokenMatch && typedProperties.equals(eventLog.getTypedProperties());
boolean tagMatch = Analytics.getTransmissionTarget("token2").equals(eventLog.getTag());
return nameMatches && tokenMatch && tagMatch && typedProperties.equals(eventLog.getTypedProperties());
}
return false;
}
}), anyString());
reset(mChannel);

/* Create a child transmission target and track event. */
AnalyticsTransmissionTarget childTarget = target.getTransmissionTarget("token3");
final AnalyticsTransmissionTarget childTarget = target.getTransmissionTarget("token3");
childTarget.trackEvent("name");
verify(mChannel).enqueue(argThat(new ArgumentMatcher<Log>() {

Expand All @@ -195,7 +200,8 @@ public boolean matches(Object item) {
EventLog eventLog = (EventLog) item;
boolean nameAndPropertiesMatch = eventLog.getName().equals("name") && eventLog.getTypedProperties() == null;
boolean tokenMatch = eventLog.getTransmissionTargetTokens().size() == 1 && eventLog.getTransmissionTargetTokens().contains("token3");
return nameAndPropertiesMatch && tokenMatch;
boolean tagMatch = childTarget.equals(eventLog.getTag());
return nameAndPropertiesMatch && tokenMatch && tagMatch;
}
return false;
}
Expand Down
Loading

0 comments on commit 81aed43

Please sign in to comment.