Skip to content

Commit

Permalink
Merge pull request #3231 from lognaturel/issue-3126
Browse files Browse the repository at this point in the history 
In audit log, only include position predicates for repeats
  • Loading branch information
@yanokwa
yanokwa authored Jul 24, 2019
2 parents 04bbc9a + 9aadaf8 commit d94df2c
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 45 deletions.
70 changes: 49 additions & 21 deletions collect_app/src/androidTest/java/org/odk/collect/android/tasks/AuditEventSaveTaskTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.odk.collect.android.tasks;

import android.os.Environment;

import androidx.test.rule.GrantPermissionRule;
import androidx.test.runner.AndroidJUnit4;

Expand All @@ -32,11 +33,10 @@
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.concurrent.ExecutionException;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.END_OF_FORM;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.FORM_EXIT;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.FORM_FINALIZE;
Expand Down Expand Up @@ -193,22 +193,22 @@ private ArrayList<AuditEvent> getSampleAuditEventsWithoutLocations() {
AuditEvent event;
ArrayList<AuditEvent> auditEvents = new ArrayList<>();
auditEvents.add(new AuditEvent(1548106927319L, FORM_START));
event = new AuditEvent(1548106927323L, QUESTION, false, false, getMockedFormIndex("/data/q1"), "");
event = new AuditEvent(1548106927323L, QUESTION, false, false, getTestFormIndex("/data/q1"), "");
event.setEnd(1548106930112L);
auditEvents.add(event);
event = new AuditEvent(1548106930118L, PROMPT_NEW_REPEAT, false, false, getMockedFormIndex("/data/g1[1]"), "");
event = new AuditEvent(1548106930118L, PROMPT_NEW_REPEAT, false, false, getTestFormIndex("/data/g1[1]"), "");
event.setEnd(1548106931611L);
auditEvents.add(event);
event = new AuditEvent(1548106931612L, QUESTION, false, false, getMockedFormIndex("/data/g1[1]/q2[1]"), "");
event = new AuditEvent(1548106931612L, QUESTION, false, false, getTestFormIndex("/data/g1[1]/q2"), "");
event.setEnd(1548106937122L);
auditEvents.add(event);
event = new AuditEvent(1548106937123L, PROMPT_NEW_REPEAT, false, false, getMockedFormIndex("/data/g1[2]"), "");
event = new AuditEvent(1548106937123L, PROMPT_NEW_REPEAT, false, false, getTestFormIndex("/data/g1[2]"), "");
event.setEnd(1548106938276L);
auditEvents.add(event);
event = new AuditEvent(1548106938277L, QUESTION, false, false, getMockedFormIndex("/data/g1[2]/q2[1]"), "");
event = new AuditEvent(1548106938277L, QUESTION, false, false, getTestFormIndex("/data/g1[2]/q2"), "");
event.setEnd(1548106948127L);
auditEvents.add(event);
event = new AuditEvent(1548106948128L, PROMPT_NEW_REPEAT, false, false, getMockedFormIndex("/data/g1[3]"), "");
event = new AuditEvent(1548106948128L, PROMPT_NEW_REPEAT, false, false, getTestFormIndex("/data/g1[3]"), "");
event.setEnd(1548106949446L);
auditEvents.add(event);
event = new AuditEvent(1548106949448L, END_OF_FORM);
Expand Down Expand Up @@ -304,27 +304,27 @@ private ArrayList<AuditEvent> getSampleAuditEventsWithLocations() {
event = new AuditEvent(548108908259L, LOCATION_PROVIDERS_ENABLED, true, false);
event.setLocationCoordinates("", "", "");
auditEvents.add(event);
event = new AuditEvent(1548106927323L, QUESTION, true, false, getMockedFormIndex("/data/q1"), "");
event = new AuditEvent(1548106927323L, QUESTION, true, false, getTestFormIndex("/data/q1"), "");
event.setLocationCoordinates("54.4112062", "18.5896652", "30.716999053955078");
event.setEnd(1548106930112L);
auditEvents.add(event);
event = new AuditEvent(1548106930118L, PROMPT_NEW_REPEAT, true, false, getMockedFormIndex("/data/g1[1]"), "");
event = new AuditEvent(1548106930118L, PROMPT_NEW_REPEAT, true, false, getTestFormIndex("/data/g1[1]"), "");
event.setLocationCoordinates("54.4112062", "18.5896652", "30.716999053955078");
event.setEnd(1548106931611L);
auditEvents.add(event);
event = new AuditEvent(1548106931612L, QUESTION, true, false, getMockedFormIndex("/data/g1[1]/q2[1]"), "");
event = new AuditEvent(1548106931612L, QUESTION, true, false, getTestFormIndex("/data/g1[1]/q2"), "");
event.setLocationCoordinates("54.4112062", "18.5896652", "30.716999053955078");
event.setEnd(1548106937122L);
auditEvents.add(event);
event = new AuditEvent(1548106937123L, PROMPT_NEW_REPEAT, true, false, getMockedFormIndex("/data/g1[2]"), "");
event = new AuditEvent(1548106937123L, PROMPT_NEW_REPEAT, true, false, getTestFormIndex("/data/g1[2]"), "");
event.setLocationCoordinates("54.4112062", "18.5896652", "30.716999053955078");
event.setEnd(1548106938276L);
auditEvents.add(event);
event = new AuditEvent(1548106938277L, QUESTION, true, false, getMockedFormIndex("/data/g1[2]/q2[1]"), "");
event = new AuditEvent(1548106938277L, QUESTION, true, false, getTestFormIndex("/data/g1[2]/q2"), "");
event.setLocationCoordinates("54.4112062", "18.5896652", "30.716999053955078");
event.setEnd(1548106948127L);
auditEvents.add(event);
event = new AuditEvent(1548106948128L, PROMPT_NEW_REPEAT, true, false, getMockedFormIndex("/data/g1[3]"), "");
event = new AuditEvent(1548106948128L, PROMPT_NEW_REPEAT, true, false, getTestFormIndex("/data/g1[3]"), "");
event.setLocationCoordinates("54.4112062", "18.5896652", "30.716999053955078");
event.setEnd(1548106949446L);
auditEvents.add(event);
Expand Down Expand Up @@ -359,12 +359,12 @@ private ArrayList<AuditEvent> getSampleAuditEventsWithLocationsAndTrackingChange
event = new AuditEvent(548108908259L, LOCATION_PROVIDERS_ENABLED, true, true);
event.setLocationCoordinates("", "", "");
auditEvents.add(event);
event = new AuditEvent(1548106927323L, QUESTION, true, true, getMockedFormIndex("/data/q1"), "Old value");
event = new AuditEvent(1548106927323L, QUESTION, true, true, getTestFormIndex("/data/q1"), "Old value");
event.setLocationCoordinates("54.4112062", "18.5896652", "30.716999053955078");
event.recordValueChange("New Value");
event.setEnd(1548106930112L);
auditEvents.add(event);
event = new AuditEvent(1548106930118L, PROMPT_NEW_REPEAT, true, true, getMockedFormIndex("/data/g1[1]"), null);
event = new AuditEvent(1548106930118L, PROMPT_NEW_REPEAT, true, true, getTestFormIndex("/data/g1[1]"), null);
event.setLocationCoordinates("54.4112062", "18.5896652", "30.716999053955078");
event.setEnd(1548106931611L);
auditEvents.add(event);
Expand All @@ -384,11 +384,39 @@ private ArrayList<AuditEvent> getSampleAuditEventsWithLocationsAndTrackingChange
return auditEvents;
}

private FormIndex getMockedFormIndex(String treeReferenceValue) {
FormIndex formIndex = mock(FormIndex.class);
TreeReference treeReference = mock(TreeReference.class);
when(formIndex.getReference()).thenReturn(treeReference);
when(treeReference.toString()).thenReturn(treeReferenceValue);
/**
* Given an XPath path, generate a corresponding {@link TreeReference} and a fake
* {@link FormIndex} that doesn't correspond to any real form definition. The only thing we care
* about for the {@link FormIndex} are the instance indexes at every level. Everything else can
* be faked.
*
* TODO: once {@link AuditEvent}'s getXPathPath moves to FormIndex, just use a mock
*/
private FormIndex getTestFormIndex(String xpathPath) {
String[] nodes = xpathPath.split("/");
TreeReference treeReference = new TreeReference();
nodes = Arrays.copyOfRange(nodes, 1, nodes.length); // take care of leading /
ArrayList<Integer> positions = new ArrayList<>();

for (String node : nodes) {
String[] parts = node.split("\\[");

String nodeName = parts[0];
int position = 0;
if (parts.length > 1) {
position = Integer.parseInt(parts[1].replace("]", "")) - 1;
positions.add(position);
} else {
positions.add(-1);
}
treeReference.add(nodeName, position);
}

FormIndex formIndex = null;
for (int i = nodes.length - 1; i > 0; i--) { // exclude the root node
formIndex = new FormIndex(formIndex, -1, positions.get(i), treeReference);
}

return formIndex;
}
}
44 changes: 37 additions & 7 deletions collect_app/src/main/java/org/odk/collect/android/logic/AuditEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
import androidx.annotation.NonNull;

import org.javarosa.core.model.FormIndex;
import org.javarosa.core.model.instance.TreeReference;
import org.javarosa.form.api.FormEntryController;
import org.odk.collect.android.utilities.TextUtils;

import java.util.ArrayList;
import java.util.List;

public class AuditEvent {

Expand Down Expand Up @@ -216,13 +221,7 @@ public void recordValueChange(String newValue) {
*/
@NonNull
public String toString() {
String node = formIndex == null || formIndex.getReference() == null ? "" : formIndex.getReference().toString();
if (auditEventType == AuditEvent.AuditEventType.QUESTION || auditEventType == AuditEvent.AuditEventType.GROUP) {
int idx = node.lastIndexOf('[');
if (idx > 0) {
node = node.substring(0, idx);
}
}
String node = formIndex == null || formIndex.getReference() == null ? "" : getXPathPath(formIndex);

String event;
if (isTrackingLocationsEnabled && isTrackingChangesEnabled) {
Expand Down Expand Up @@ -273,4 +272,35 @@ private String getEscapedValueForCsv(String value) {

return "\"" + value + "\"";
}

/**
* Get the XPath path of the node at a particular {@link FormIndex}.
*
* Differs from {@link TreeReference#toString()} in that position predicates are only
* included for repeats. For example, given a group named {@code my-group} that contains a
* repeat named {@code my-repeat} which in turn contains a question named {@code my-question},
* {@link TreeReference#toString()} would return paths that look like
* {@code /my-group[1]/my-repeat[3]/my-question[1]}. In contrast, this method would return
* {@code /my-group/my-repeat[3]/my-question}.
*
* TODO: consider moving to {@link FormIndex}
*/
private static String getXPathPath(FormIndex formIndex) {
List<String> nodeNames = new ArrayList<>();
nodeNames.add(formIndex.getReference().getName(0));

FormIndex walker = formIndex;
int i = 1;
while (walker != null) {
String currentNodeName = formIndex.getReference().getName(i);
if (walker.getInstanceIndex() != -1) {
currentNodeName = currentNodeName + "[" + (walker.getInstanceIndex() + 1) + "]";
}
nodeNames.add(currentNodeName);

walker = walker.getNextLevel();
i++;
}
return "/" + TextUtils.join("/", nodeNames);
}
}
31 changes: 31 additions & 0 deletions collect_app/src/main/java/org/odk/collect/android/utilities/TextUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

import android.text.Html;

import androidx.annotation.NonNull;

import java.util.Iterator;
import java.util.regex.MatchResult;

public class TextUtils {
Expand Down Expand Up @@ -122,5 +125,33 @@ public static String ellipsizeBeginning(String text) {
? text
: "..." + text.substring(text.length() - 97, text.length());
}

/**
* Copyright (C) 2006 The Android Open Source Project
*
* Copied from Android project for testing.
* TODO: replace with String.join when minSdk goes to 26
*
* Returns a string containing the tokens joined by delimiters.
*
* @param delimiter a CharSequence that will be inserted between the tokens. If null, the string
* "null" will be used as the delimiter.
* @param tokens an array objects to be joined. Strings will be formed from the objects by
* calling object.toString(). If tokens is null, a NullPointerException will be thrown. If
* tokens is empty, an empty string will be returned.
*/
public static String join(@NonNull CharSequence delimiter, @NonNull Iterable tokens) {
final Iterator<?> it = tokens.iterator();
if (!it.hasNext()) {
return "";
}
final StringBuilder sb = new StringBuilder();
sb.append(it.next());
while (it.hasNext()) {
sb.append(delimiter);
sb.append(it.next());
}
return sb.toString();
}
}

32 changes: 15 additions & 17 deletions collect_app/src/test/java/org/odk/collect/android/logic/AuditEventTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,25 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.BEGINNING_OF_FORM;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.END_OF_FORM;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.GROUP;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.LOCATION_TRACKING_DISABLED;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.LOCATION_TRACKING_ENABLED;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.CONSTRAINT_ERROR;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.DELETE_REPEAT;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.END_OF_FORM;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.FINALIZE_ERROR;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.FORM_EXIT;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.FORM_FINALIZE;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.FORM_RESUME;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.FORM_SAVE;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.FORM_START;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.GOOGLE_PLAY_SERVICES_NOT_AVAILABLE;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.GROUP;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.HIERARCHY;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.LOCATION_PERMISSIONS_GRANTED;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.LOCATION_PERMISSIONS_NOT_GRANTED;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.LOCATION_PROVIDERS_DISABLED;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.LOCATION_PROVIDERS_ENABLED;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.LOCATION_TRACKING_DISABLED;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.LOCATION_TRACKING_ENABLED;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.PROMPT_NEW_REPEAT;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.QUESTION;
import static org.odk.collect.android.logic.AuditEvent.AuditEventType.REPEAT;
Expand All @@ -58,7 +56,7 @@ public class AuditEventTest {

@Test
public void testToString() {
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, false, false, getMockedFormIndex(), "");
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, false, false, getTestFormIndex(), "");
assertNotNull(auditEvent);
assertTrue(auditEvent.isIntervalAuditEventType());
assertEquals("question,/data/text1,1545392727685,", auditEvent.toString());
Expand All @@ -71,7 +69,7 @@ public void testToString() {

@Test
public void testToStringWithLocationCoordinates() {
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, true, false, getMockedFormIndex(), "");
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, true, false, getTestFormIndex(), "");
assertNotNull(auditEvent);
auditEvent.setLocationCoordinates("54.35202520000001", "18.64663840000003", "10");
assertTrue(auditEvent.isIntervalAuditEventType());
Expand All @@ -84,7 +82,7 @@ public void testToStringWithLocationCoordinates() {

@Test
public void testToStringWithTrackingChanges() {
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, false, true, getMockedFormIndex(), "First answer");
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, false, true, getTestFormIndex(), "First answer");
assertNotNull(auditEvent);
assertTrue(auditEvent.isIntervalAuditEventType());
assertFalse(auditEvent.isEndTimeSet());
Expand All @@ -97,7 +95,7 @@ public void testToStringWithTrackingChanges() {

@Test
public void testToStringWithLocationCoordinatesAndTrackingChanges() {
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, true, true, getMockedFormIndex(), "First answer");
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, true, true, getTestFormIndex(), "First answer");
assertNotNull(auditEvent);
auditEvent.setLocationCoordinates("54.35202520000001", "18.64663840000003", "10");
assertTrue(auditEvent.isIntervalAuditEventType());
Expand All @@ -111,7 +109,7 @@ public void testToStringWithLocationCoordinatesAndTrackingChanges() {

@Test
public void testToStringNullValues() {
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, true, true, getMockedFormIndex(), "Old value");
AuditEvent auditEvent = new AuditEvent(START_TIME, QUESTION, true, true, getTestFormIndex(), "Old value");
assertNotNull(auditEvent);
auditEvent.setLocationCoordinates("", "", "");
assertTrue(auditEvent.isIntervalAuditEventType());
Expand Down Expand Up @@ -256,11 +254,11 @@ public void getAuditEventTypeFromFecTypeTest() {
assertEquals(UNKNOWN_EVENT_TYPE, AuditEvent.getAuditEventTypeFromFecType(100));
}

private FormIndex getMockedFormIndex() {
FormIndex formIndex = mock(FormIndex.class);
TreeReference treeReference = mock(TreeReference.class);
when(formIndex.getReference()).thenReturn(treeReference);
when(treeReference.toString()).thenReturn("/data/text1");
return formIndex;
private FormIndex getTestFormIndex() {
TreeReference treeReference = new TreeReference();
treeReference.add("data", 0);
treeReference.add("text1", 0);

return new FormIndex(0, treeReference);
}
}

0 comments on commit d94df2c

Please sign in to comment.