Guard against NPE in equals checks.
Update State test to cover different conditions separately.
Update DirectoryInfo test to best practices.
Update RootInfo to follow DirectoryInfo equals pattern.
Also, allow null cursors from Model...since they happen...I seen it mahself.
Bug: 27118725
Change-Id: Id0a0be053b91c887a745bfca5416e64f0edba995
diff --git a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java
index 4233b36..0ae2a5c 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java
@@ -536,7 +536,11 @@
@Override
public void onItemStateChanged(String modelId, boolean selected) {
final Cursor cursor = mModel.getItem(modelId);
- checkNotNull(cursor, "Cursor cannot be null.");
+ if (cursor == null) {
+ Log.e(TAG, "Model returned null cursor for document: " + modelId
+ + ". Ignoring state changed event.");
+ return;
+ }
// TODO: Should this be happening in onSelectionChanged? Technically this callback is
// triggered on "silent" selection updates (i.e. we might be reacting to unfinalized
diff --git a/packages/DocumentsUI/src/com/android/documentsui/model/DocumentInfo.java b/packages/DocumentsUI/src/com/android/documentsui/model/DocumentInfo.java
index 4b5499a..1c696ad 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/model/DocumentInfo.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/model/DocumentInfo.java
@@ -39,6 +39,7 @@
import java.io.IOException;
import java.net.ProtocolException;
import java.text.Collator;
+import java.util.Objects;
/**
* Representation of a {@link Document}.
@@ -263,16 +264,23 @@
return derivedUri.hashCode() + mimeType.hashCode();
}
- public boolean equals(Object other) {
- if (this == other) {
- return true;
- } else if (!(other instanceof DocumentInfo)) {
+ public boolean equals(Object o) {
+ if (o == null) {
return false;
}
- DocumentInfo that = (DocumentInfo) other;
- // Uri + mime type should be totally unique.
- return derivedUri.equals(that.derivedUri) && mimeType.equals(that.mimeType);
+ if (this == o) {
+ return true;
+ }
+
+ if (o instanceof DocumentInfo) {
+ DocumentInfo other = (DocumentInfo) o;
+ // Uri + mime type should be totally unique.
+ return Objects.equals(derivedUri, other.derivedUri)
+ && Objects.equals(mimeType, other.mimeType);
+ }
+
+ return false;
}
public static String getCursorString(Cursor cursor, String columnName) {
diff --git a/packages/DocumentsUI/src/com/android/documentsui/model/RootInfo.java b/packages/DocumentsUI/src/com/android/documentsui/model/RootInfo.java
index 3f14a55..08a3b85 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/model/RootInfo.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/model/RootInfo.java
@@ -276,12 +276,21 @@
@Override
public boolean equals(Object o) {
- if (o instanceof RootInfo) {
- final RootInfo root = (RootInfo) o;
- return Objects.equals(authority, root.authority) && Objects.equals(rootId, root.rootId);
- } else {
+ if (o == null) {
return false;
}
+
+ if (this == o) {
+ return true;
+ }
+
+ if (o instanceof RootInfo) {
+ RootInfo other = (RootInfo) o;
+ return Objects.equals(authority, other.authority)
+ && Objects.equals(rootId, other.rootId);
+ }
+
+ return false;
}
@Override
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/StateTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/StateTest.java
index b74b985..f057850 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/StateTest.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/StateTest.java
@@ -23,18 +23,44 @@
@SmallTest
public class StateTest extends AndroidTestCase {
- public void testPushDocument() {
- final State state = new State();
- final DocumentInfo infoFirst = new DocumentInfo();
- infoFirst.displayName = "firstDirectory";
- final DocumentInfo infoSecond = new DocumentInfo();
- infoSecond.displayName = "secondDirectory";
- assertFalse(state.hasLocationChanged());
- state.pushDocument(infoFirst);
- state.pushDocument(infoSecond);
- assertTrue(state.hasLocationChanged());
- assertEquals("secondDirectory", state.stack.getFirst().displayName);
- state.popDocument();
- assertEquals("firstDirectory", state.stack.getFirst().displayName);
+
+ private static final DocumentInfo DIR_1;
+ private static final DocumentInfo DIR_2;
+
+ private State mState;
+
+ static {
+ DIR_1 = new DocumentInfo();
+ DIR_1.displayName = "firstDirectory";
+ DIR_2 = new DocumentInfo();
+ DIR_2.displayName = "secondDirectory";
+ }
+
+ @Override
+ protected void setUp() throws Exception {
+ mState = new State();
+ }
+
+ public void testInitialStateEmpty() {
+ assertFalse(mState.hasLocationChanged());
+ }
+
+ public void testPushDocument_ChangesLocation() {
+ mState.pushDocument(DIR_1);
+ mState.pushDocument(DIR_2);
+ assertTrue(mState.hasLocationChanged());
+ }
+
+ public void testPushDocument_ModifiesStack() {
+ mState.pushDocument(DIR_1);
+ mState.pushDocument(DIR_2);
+ assertEquals(DIR_2, mState.stack.getFirst());
+ }
+
+ public void testPopDocument_ModifiesStack() {
+ mState.pushDocument(DIR_1);
+ mState.pushDocument(DIR_2);
+ mState.popDocument();
+ assertEquals(DIR_1, mState.stack.getFirst());
}
}
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/model/DocumentInfoTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/model/DocumentInfoTest.java
index a6aba7b..2481dc3 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/model/DocumentInfoTest.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/model/DocumentInfoTest.java
@@ -22,30 +22,36 @@
@SmallTest
public class DocumentInfoTest extends AndroidTestCase {
+ private static final DocumentInfo TEST_DOC
+ = createDocInfo("authority.a", "doc.1", "text/plain");
+
public void testEquals() throws Exception {
- DocumentInfo doc = createDocInfo("authority.a", "doc.1", "text/plain");
- assertEquals(doc, doc);
+ assertEquals(TEST_DOC, TEST_DOC);
+ assertEquals(TEST_DOC, createDocInfo("authority.a", "doc.1", "text/plain"));
+ }
+
+ public void testEquals_HandlesNulls() throws Exception {
+ assertFalse(TEST_DOC.equals(null));
+ }
+
+ public void testEquals_HandlesNullFields() throws Exception {
+ assertFalse(TEST_DOC.equals(new DocumentInfo()));
+ assertFalse(new DocumentInfo().equals(TEST_DOC));
}
public void testNotEquals_differentAuthority() throws Exception {
- DocumentInfo docA = createDocInfo("authority.a", "doc.1", "text/plain");
- DocumentInfo docB = createDocInfo("authority.b", "doc.1", "text/plain");
- assertFalse(docA.equals(docB));
+ assertFalse(TEST_DOC.equals(createDocInfo("authority.b", "doc.1", "text/plain")));
}
public void testNotEquals_differentDocId() throws Exception {
- DocumentInfo docA = createDocInfo("authority.a", "doc.1", "text/plain");
- DocumentInfo docB = createDocInfo("authority.a", "doc.2", "text/plain");
- assertFalse(docA.equals(docB));
+ assertFalse(TEST_DOC.equals(createDocInfo("authority.a", "doc.2", "text/plain")));
}
public void testNotEquals_differentMimetype() throws Exception {
- DocumentInfo docA = createDocInfo("authority.a", "doc.1", "text/plain");
- DocumentInfo docB = createDocInfo("authority.a", "doc.1", "image/png");
- assertFalse(docA.equals(docB));
+ assertFalse(TEST_DOC.equals(createDocInfo("authority.a", "doc.1", "image/png")));
}
- private DocumentInfo createDocInfo(String authority, String docId, String mimeType) {
+ private static DocumentInfo createDocInfo(String authority, String docId, String mimeType) {
DocumentInfo doc = new DocumentInfo();
doc.authority = authority;
doc.documentId = docId;