Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 30551a25 authored by Steve McKay's avatar Steve McKay
Browse files

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
parent e4735a99

packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java

+5 −1
Original line number Original line Diff line number Diff line
@@ -536,7 +536,11 @@ public class DirectoryFragment extends Fragment implements DocumentsAdapter.Envi 
        @Override

        @Override

        public void onItemStateChanged(String modelId, boolean selected) {

        public void onItemStateChanged(String modelId, boolean selected) {

            final Cursor cursor = mModel.getItem(modelId);

            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

            // TODO: Should this be happening in onSelectionChanged? Technically this callback is

            // triggered on "silent" selection updates (i.e. we might be reacting to unfinalized

            // triggered on "silent" selection updates (i.e. we might be reacting to unfinalized

packages/DocumentsUI/src/com/android/documentsui/model/DocumentInfo.java

+15 −7
Original line number Original line Diff line number Diff line
@@ -39,6 +39,7 @@ import java.io.FileNotFoundException; 
import java.io.IOException;

import java.io.IOException;

import java.net.ProtocolException;

import java.net.ProtocolException;

import java.text.Collator;

import java.text.Collator;

import java.util.Objects;





/**

/**

 * Representation of a {@link Document}.

 * Representation of a {@link Document}.
@@ -263,16 +264,23 @@ public class DocumentInfo implements Durable, Parcelable { 
        return derivedUri.hashCode() + mimeType.hashCode();

        return derivedUri.hashCode() + mimeType.hashCode();

    }

    }





    public boolean equals(Object other) {

    public boolean equals(Object o) {

        if (this == other) {

        if (o == null) {

            return true;

        } else if (!(other instanceof DocumentInfo)) {

            return false;

            return false;

        }

        }





        DocumentInfo that = (DocumentInfo) other;

        if (this == o) {

            return true;

        }



        if (o instanceof DocumentInfo) {

            DocumentInfo other = (DocumentInfo) o;

            // Uri + mime type should be totally unique.

            // Uri + mime type should be totally unique.

        return derivedUri.equals(that.derivedUri) && mimeType.equals(that.mimeType);

            return Objects.equals(derivedUri, other.derivedUri)

                    && Objects.equals(mimeType, other.mimeType);

        }



        return false;

    }

    }





    public static String getCursorString(Cursor cursor, String columnName) {

    public static String getCursorString(Cursor cursor, String columnName) {

packages/DocumentsUI/src/com/android/documentsui/model/RootInfo.java

+13 −4
Original line number Original line Diff line number Diff line
@@ -276,12 +276,21 @@ public class RootInfo implements Durable, Parcelable { 




    @Override

    @Override

    public boolean equals(Object o) {

    public boolean equals(Object o) {

        if (o instanceof RootInfo) {

        if (o == null) {

            final RootInfo root = (RootInfo) o;

            return Objects.equals(authority, root.authority) && Objects.equals(rootId, root.rootId);

        } else {

            return false;

            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

    @Override

packages/DocumentsUI/tests/src/com/android/documentsui/StateTest.java

+39 −13
Original line number Original line Diff line number Diff line
@@ -23,18 +23,44 @@ import com.android.documentsui.model.DocumentInfo; 




@SmallTest

@SmallTest

public class StateTest extends AndroidTestCase {

public class StateTest extends AndroidTestCase {

    public void testPushDocument() {



        final State state = new State();

    private static final DocumentInfo DIR_1;

        final DocumentInfo infoFirst = new DocumentInfo();

    private static final DocumentInfo DIR_2;

        infoFirst.displayName = "firstDirectory";



        final DocumentInfo infoSecond = new DocumentInfo();

    private State mState;

        infoSecond.displayName = "secondDirectory";



        assertFalse(state.hasLocationChanged());

    static {

        state.pushDocument(infoFirst);

        DIR_1 = new DocumentInfo();

        state.pushDocument(infoSecond);

        DIR_1.displayName = "firstDirectory";

        assertTrue(state.hasLocationChanged());

        DIR_2 = new DocumentInfo();

        assertEquals("secondDirectory", state.stack.getFirst().displayName);

        DIR_2.displayName = "secondDirectory";

        state.popDocument();

    }

        assertEquals("firstDirectory", state.stack.getFirst().displayName);



    @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());

    }

    }

}
 
}

packages/DocumentsUI/tests/src/com/android/documentsui/model/DocumentInfoTest.java

+18 −12
Original line number Original line Diff line number Diff line
@@ -22,30 +22,36 @@ import android.test.suitebuilder.annotation.SmallTest; 
@SmallTest

@SmallTest

public class DocumentInfoTest extends AndroidTestCase {

public class DocumentInfoTest extends AndroidTestCase {





    private static final DocumentInfo TEST_DOC

            = createDocInfo("authority.a", "doc.1", "text/plain");



    public void testEquals() throws Exception {

    public void testEquals() throws Exception {

        DocumentInfo doc = createDocInfo("authority.a", "doc.1", "text/plain");

        assertEquals(TEST_DOC, TEST_DOC);

        assertEquals(doc, 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 {

    public void testNotEquals_differentAuthority() throws Exception {

        DocumentInfo docA = createDocInfo("authority.a", "doc.1", "text/plain");

        assertFalse(TEST_DOC.equals(createDocInfo("authority.b", "doc.1", "text/plain")));

        DocumentInfo docB = createDocInfo("authority.b", "doc.1", "text/plain");

        assertFalse(docA.equals(docB));

    }

    }





    public void testNotEquals_differentDocId() throws Exception {

    public void testNotEquals_differentDocId() throws Exception {

        DocumentInfo docA = createDocInfo("authority.a", "doc.1", "text/plain");

        assertFalse(TEST_DOC.equals(createDocInfo("authority.a", "doc.2", "text/plain")));

        DocumentInfo docB = createDocInfo("authority.a", "doc.2", "text/plain");

        assertFalse(docA.equals(docB));

    }

    }





    public void testNotEquals_differentMimetype() throws Exception {

    public void testNotEquals_differentMimetype() throws Exception {

        DocumentInfo docA = createDocInfo("authority.a", "doc.1", "text/plain");

        assertFalse(TEST_DOC.equals(createDocInfo("authority.a", "doc.1", "image/png")));

        DocumentInfo docB = createDocInfo("authority.a", "doc.1", "image/png");

        assertFalse(docA.equals(docB));

    }

    }





    private DocumentInfo createDocInfo(String authority, String docId, String mimeType) {

    private static DocumentInfo createDocInfo(String authority, String docId, String mimeType) {

        DocumentInfo doc = new DocumentInfo();

        DocumentInfo doc = new DocumentInfo();

        doc.authority = authority;

        doc.authority = authority;

        doc.documentId = docId;

        doc.documentId = docId;