Skip to content

Revert "Crashlytics Add Internal Keys feature for Unity Metadata" #2697

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

Merged
merged 1 commit into from
May 26, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -222,33 +222,6 @@ public void testWriteKeys_readDifferentSession() {
assertEquals(0, readKeys.size());
}

// Ensures the Internal Keys and User Custom Keys are stored separately
public void testWriteKeys_readSeparateFromUser() {
final Map<String, String> keys =
new HashMap<String, String>() {
{
put(KEY_1, VALUE_1);
}
};

final Map<String, String> internalKeys =
new HashMap<String, String>() {
{
put(KEY_2, VALUE_2);
put(KEY_3, VALUE_3);
}
};

storeUnderTest.writeKeyData(SESSION_ID_1, keys);
storeUnderTest.writeKeyData(SESSION_ID_1, internalKeys, /*isInternal=*/ true);

final Map<String, String> readKeys = storeUnderTest.readKeyData(SESSION_ID_1);
final Map<String, String> readInternalKeys = storeUnderTest.readKeyData(SESSION_ID_1, true);

assertEqualMaps(keys, readKeys);
assertEqualMaps(internalKeys, readInternalKeys);
}

public void testReadKeys_noStoredData() {
final Map<String, String> readKeys = storeUnderTest.readKeyData(SESSION_ID_1);
assertEquals(0, readKeys.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,11 @@ public void testNonFatalEvent_addsSortedKeysToEvent() {
ImmutableList.from(customAttribute1, customAttribute2);

when(reportMetadata.getCustomKeys()).thenReturn(attributes);
when(reportMetadata.getInternalKeys()).thenReturn(attributes);

reportingCoordinator.onBeginSession(sessionId, timestamp);
reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);

verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes);
verify(mockEventAppBuilder).setInternalKeys(expectedCustomAttributes);
verify(mockEventAppBuilder).build();
verify(mockEventBuilder).setApp(mockEventApp);
verify(mockEventBuilder).build();
Expand Down Expand Up @@ -290,13 +288,11 @@ public void testFatalEvent_addsSortedKeysToEvent() {
ImmutableList.from(customAttribute1, customAttribute2);

when(reportMetadata.getCustomKeys()).thenReturn(attributes);
when(reportMetadata.getInternalKeys()).thenReturn(attributes);

reportingCoordinator.onBeginSession(sessionId, timestamp);
reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp);

verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes);
verify(mockEventAppBuilder).setInternalKeys(expectedCustomAttributes);
verify(mockEventAppBuilder).build();
verify(mockEventBuilder).setApp(mockEventApp);
verify(mockEventBuilder).build();
Expand Down Expand Up @@ -465,7 +461,6 @@ private void mockEventInteractions() {
when(mockEvent.getApp()).thenReturn(mockEventApp);
when(mockEventApp.toBuilder()).thenReturn(mockEventAppBuilder);
when(mockEventAppBuilder.setCustomAttributes(any())).thenReturn(mockEventAppBuilder);
when(mockEventAppBuilder.setInternalKeys(any())).thenReturn(mockEventAppBuilder);
when(mockEventAppBuilder.build()).thenReturn(mockEventApp);
when(dataCapture.captureEventData(
any(Throwable.class),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,28 +434,14 @@ void setCustomKey(String key, String value) {
return;
}
}
cacheKeyData(userMetadata.getCustomKeys(), false);
cacheKeyData(userMetadata.getCustomKeys());
}

void setCustomKeys(Map<String, String> keysAndValues) {
// Write all the key/value pairs before doing anything computationally expensive.
userMetadata.setCustomKeys(keysAndValues);
// Once all the key/value pairs are added, update the cache.
cacheKeyData(userMetadata.getCustomKeys(), false);
}

void setInternalKey(String key, String value) {
try {
userMetadata.setInternalKey(key, value);
} catch (IllegalArgumentException ex) {
if (context != null && CommonUtils.isAppDebuggable(context)) {
throw ex;
} else {
Logger.getLogger().e("Attempting to set custom attribute with null key, ignoring.");
return;
}
}
cacheKeyData(userMetadata.getInternalKeys(), true);
cacheKeyData(userMetadata.getCustomKeys());
}

/**
Expand Down Expand Up @@ -489,13 +475,13 @@ public Void call() throws Exception {
* crash happens immediately after setting a value. If this becomes a problem, we can investigate
* writing synchronously, or potentially add an explicit user-facing API for synchronous writes.
*/
private void cacheKeyData(final Map<String, String> keyData, boolean isInternal) {
private void cacheKeyData(final Map<String, String> keyData) {
backgroundWorker.submit(
new Callable<Void>() {
@Override
public Void call() throws Exception {
final String currentSessionId = getCurrentSessionId();
new MetaDataStore(getFilesDir()).writeKeyData(currentSessionId, keyData, isInternal);
new MetaDataStore(getFilesDir()).writeKeyData(currentSessionId, keyData);
return null;
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public boolean onPreExecute(AppData appData, SettingsDataProvider settingsProvid
initializationMarker = new CrashlyticsFileMarker(INITIALIZATION_MARKER_FILE_NAME, fileStore);

final UserMetadata userMetadata = new UserMetadata();

final LogFileDirectoryProvider logFileDirectoryProvider =
new LogFileDirectoryProvider(fileStore);
final LogFileManager logFileManager = new LogFileManager(context, logFileDirectoryProvider);
Expand Down Expand Up @@ -346,22 +347,6 @@ public void setCustomKeys(Map<String, String> keysAndValues) {
controller.setCustomKeys(keysAndValues);
}

/**
* Set a value to be associated with a given key for your crash data. The key/value pairs will be
* reported with any crash that occurs in this session. A maximum of 64 key/value pairs can be
* stored for any type. New keys added over that limit will be ignored. Keys and values are
* trimmed ({@link String#trim()}), and keys or values that exceed 1024 characters will be
* truncated.
*
* <p>IMPORTANT: This method is accessed via reflection and JNI. Do not change the type without
* updating the SDKs that depend on it.
*
* @throws NullPointerException if key is null.
*/
public void setInternalKey(String key, String value) {
controller.setInternalKey(key, value);
}

// endregion

// region Package-protected getters
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class MetaDataStore {

private static final String USERDATA_SUFFIX = "user";
private static final String KEYDATA_SUFFIX = "keys";
private static final String INTERNAL_KEYDATA_SUFFIX = "internal-keys";
private static final String METADATA_EXT = ".meta";

private static final String KEY_USER_ID = "userId";
Expand Down Expand Up @@ -86,12 +85,7 @@ public UserMetadata readUserData(String sessionId) {
}

public void writeKeyData(String sessionId, Map<String, String> keyData) {
writeKeyData(sessionId, keyData, false);
}

void writeKeyData(String sessionId, Map<String, String> keyData, boolean isInternal) {
final File f =
isInternal ? getInternalKeysFileForSession(sessionId) : getKeysFileForSession(sessionId);
final File f = getKeysFileForSession(sessionId);
Writer writer = null;
try {
final String keyDataString = keysDataToJson(keyData);
Expand All @@ -106,12 +100,7 @@ void writeKeyData(String sessionId, Map<String, String> keyData, boolean isInter
}

public Map<String, String> readKeyData(String sessionId) {
return readKeyData(sessionId, false);
}

Map<String, String> readKeyData(String sessionId, boolean isInternal) {
final File f =
isInternal ? getInternalKeysFileForSession(sessionId) : getKeysFileForSession(sessionId);
final File f = getKeysFileForSession(sessionId);
if (!f.exists()) {
return Collections.emptyMap();
}
Expand All @@ -138,11 +127,6 @@ public File getKeysFileForSession(String sessionId) {
return new File(filesDir, sessionId + KEYDATA_SUFFIX + METADATA_EXT);
}

@NonNull
public File getInternalKeysFileForSession(String sessionId) {
return new File(filesDir, sessionId + INTERNAL_KEYDATA_SUFFIX + METADATA_EXT);
}

private static UserMetadata jsonToUserData(String json) throws JSONException {
final JSONObject dataObj = new JSONObject(json);
UserMetadata metadata = new UserMetadata();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,16 +220,13 @@ private void persistEvent(

final List<CustomAttribute> sortedCustomAttributes =
getSortedCustomAttributes(reportMetadata.getCustomKeys());
final List<CustomAttribute> sortedInternalKeys =
getSortedCustomAttributes(reportMetadata.getInternalKeys());

if (!sortedCustomAttributes.isEmpty()) {
eventBuilder.setApp(
capturedEvent
.getApp()
.toBuilder()
.setCustomAttributes(ImmutableList.from(sortedCustomAttributes))
.setInternalKeys(ImmutableList.from(sortedInternalKeys))
.build());
}

Expand Down
Loading