diff options
4 files changed, 186 insertions, 74 deletions
diff --git a/src/com/android/car/calendar/common/Dialer.java b/src/com/android/car/calendar/common/Dialer.java index 889a3c8..df843aa 100644 --- a/src/com/android/car/calendar/common/Dialer.java +++ b/src/com/android/car/calendar/common/Dialer.java @@ -26,6 +26,8 @@ import android.util.Log; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; +import java.util.Objects; + import javax.annotation.Nullable; /** Calls the default dialer with an optional access code. */ @@ -94,5 +96,18 @@ public class Dialer { .add("mAccess", mAccess) .toString(); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + NumberAndAccess that = (NumberAndAccess) o; + return mNumber.equals(that.mNumber) && Objects.equals(mAccess, that.mAccess); + } + + @Override + public int hashCode() { + return Objects.hash(mNumber, mAccess); + } } } diff --git a/src/com/android/car/calendar/common/Event.java b/src/com/android/car/calendar/common/Event.java index 4395d33..6f88717 100644 --- a/src/com/android/car/calendar/common/Event.java +++ b/src/com/android/car/calendar/common/Event.java @@ -20,6 +20,9 @@ import com.android.car.calendar.common.Dialer.NumberAndAccess; import java.time.Duration; import java.time.Instant; +import java.util.Objects; + +import javax.annotation.Nullable; /** * An immutable value representing a calendar event. Should contain only details that are relevant @@ -34,9 +37,7 @@ public final class Event { NONE, } - /** - * The details required for display of the calendar indicator. - */ + /** The details required for display of the calendar indicator. */ public static class CalendarDetails { private final String mName; private final int mColor; @@ -53,6 +54,19 @@ public final class Event { public String getName() { return mName; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CalendarDetails that = (CalendarDetails) o; + return mColor == that.mColor && mName.equals(that.mName); + } + + @Override + public int hashCode() { + return Objects.hash(mName, mColor); + } } private final boolean mAllDay; @@ -62,8 +76,8 @@ public final class Event { private final Instant mDayEndInstant; private final String mTitle; private final Status mStatus; - private final String mLocation; - private final NumberAndAccess mNumberAndAccess; + @Nullable private final String mLocation; + @Nullable private final NumberAndAccess mNumberAndAccess; private final CalendarDetails mCalendarDetails; Event( @@ -74,8 +88,8 @@ public final class Event { Instant dayEndInstant, String title, Status status, - String location, - NumberAndAccess numberAndAccess, + @Nullable String location, + @Nullable NumberAndAccess numberAndAccess, CalendarDetails calendarDetails) { mAllDay = allDay; mStartInstant = startInstant; @@ -109,6 +123,7 @@ public final class Event { return mTitle; } + @Nullable public NumberAndAccess getNumberAndAccess() { return mNumberAndAccess; } @@ -117,6 +132,7 @@ public final class Event { return mCalendarDetails; } + @Nullable public String getLocation() { return mLocation; } @@ -132,4 +148,36 @@ public final class Event { public Duration getDuration() { return Duration.between(getStartInstant(), getEndInstant()); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Event event = (Event) o; + return mAllDay == event.mAllDay + && mStartInstant.equals(event.mStartInstant) + && mDayStartInstant.equals(event.mDayStartInstant) + && mEndInstant.equals(event.mEndInstant) + && mDayEndInstant.equals(event.mDayEndInstant) + && mTitle.equals(event.mTitle) + && mStatus == event.mStatus + && Objects.equals(mLocation, event.mLocation) + && Objects.equals(mNumberAndAccess, event.mNumberAndAccess) + && mCalendarDetails.equals(event.mCalendarDetails); + } + + @Override + public int hashCode() { + return Objects.hash( + mAllDay, + mStartInstant, + mDayStartInstant, + mEndInstant, + mDayEndInstant, + mTitle, + mStatus, + mLocation, + mNumberAndAccess, + mCalendarDetails); + } } diff --git a/src/com/android/car/calendar/common/EventsLiveData.java b/src/com/android/car/calendar/common/EventsLiveData.java index 12c91e7..f2df3fe 100644 --- a/src/com/android/car/calendar/common/EventsLiveData.java +++ b/src/com/android/car/calendar/common/EventsLiveData.java @@ -44,6 +44,7 @@ import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Comparator; import java.util.List; +import java.util.Objects; import javax.annotation.Nullable; @@ -59,6 +60,9 @@ public class EventsLiveData extends LiveData<ImmutableList<Event>> { private static final String TAG = "CarCalendarEventsLiveData"; private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); + // The duration to delay before updating the value to reduce the frequency of changes. + private static final int UPDATE_DELAY_MILLIS = 1000; + // Sort events by start date and title. private static final Comparator<Event> EVENT_COMPARATOR = Comparator.comparing(Event::getDayStartInstant).thenComparing(Event::getTitle); @@ -68,6 +72,7 @@ public class EventsLiveData extends LiveData<ImmutableList<Event>> { private final ContentResolver mContentResolver; private final EventDescriptions mEventDescriptions; private final EventLocations mLocations; + private final Runnable mUpdateRunnable = this::updateIfChanged; /** The event instances cursor is a field to allow observers to be managed. */ @Nullable private Cursor mEventsCursor; @@ -89,8 +94,12 @@ public class EventsLiveData extends LiveData<ImmutableList<Event>> { } /** Refreshes the event instances and sets the new value which notifies observers. */ - private void update() { - postValue(getEventsUntilTomorrow()); + private void updateIfChanged() { + ImmutableList<Event> latest = getEventsUntilTomorrow(); + ImmutableList<Event> current = getValue(); + if (!Objects.equals(latest, current)) { + postValue(latest); + } } /** Queries the content provider for event instances. */ @@ -167,7 +176,7 @@ public class EventsLiveData extends LiveData<ImmutableList<Event>> { @Override public void onChange(boolean selfChange) { if (DEBUG) Log.d(TAG, "Events changed"); - update(); + updateWithDelay(); } }; cursor.setNotificationUri(mContentResolver, eventInstanceUri); @@ -176,6 +185,12 @@ public class EventsLiveData extends LiveData<ImmutableList<Event>> { return cursor; } + private void updateWithDelay() { + // Do not update the events until there have been no changes for a given duration. + mBackgroundHandler.removeCallbacks(mUpdateRunnable); + mBackgroundHandler.postDelayed(mUpdateRunnable, UPDATE_DELAY_MILLIS); + } + /** Can return multiple events for a single cursor row when an event spans multiple days. */ private List<Event> createEventsForRow( Cursor eventInstancesCursor, EventDescriptions eventDescriptions) { @@ -264,11 +279,11 @@ public class EventsLiveData extends LiveData<ImmutableList<Event>> { mBackgroundHandler.post(this::tearDownCursor); } - /** Calls {@link #update()} every minute to keep the displayed time range correct. */ + /** Calls {@link #updateIfChanged()} every minute to keep the displayed time range correct. */ private void updateAndScheduleNext() { if (DEBUG) Log.d(TAG, "Update and schedule"); if (hasActiveObservers()) { - update(); + updateIfChanged(); ZonedDateTime now = ZonedDateTime.now(mClock); ZonedDateTime truncatedNowTime = now.truncatedTo(MINUTES); ZonedDateTime updateTime = truncatedNowTime.plus(1, MINUTES); diff --git a/tests/unit/src/com/android/car/calendar/common/EventsLiveDataTest.java b/tests/unit/src/com/android/car/calendar/common/EventsLiveDataTest.java index ff00e8d..81b881d 100644 --- a/tests/unit/src/com/android/car/calendar/common/EventsLiveDataTest.java +++ b/tests/unit/src/com/android/car/calendar/common/EventsLiveDataTest.java @@ -18,6 +18,7 @@ package com.android.car.calendar.common; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; @@ -138,12 +139,11 @@ public class EventsLiveDataTest { @Test @UiThreadTest public void addObserver_queryMade() throws InterruptedException { - // Expect onChanged to be called for when we start to observe and when the data is read. - CountDownLatch latch = new CountDownLatch(2); - mEventsLiveData.observeForever((value) -> latch.countDown()); + // Observing triggers content to be read. + mEventsLiveData.observeForever((unused) -> { /* Do nothing */ }); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + mTestContentProvider.awaitCalendarQuery(); assertThat(mTestContentProvider.mTestEventCursor).isNotNull(); } @@ -151,39 +151,31 @@ public class EventsLiveDataTest { @Test @UiThreadTest public void addObserver_contentObserved() throws InterruptedException { - // Expect onChanged to be called for when we start to observe and when the data is read. - CountDownLatch latch = new CountDownLatch(2); - mEventsLiveData.observeForever((value) -> latch.countDown()); + // Observing triggers content to be read. + mEventsLiveData.observeForever((unused) -> { /* Do nothing */ }); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + mTestContentProvider.awaitCalendarQuery(); - assertThat(mTestContentProvider.mTestEventCursor.mLastContentObserver).isNotNull(); + awaitAndAssertDone(mTestContentProvider.mTestEventCursor.mRegisterContentObserverLatch); } @Test - @UiThreadTest public void removeObserver_contentNotObserved() throws InterruptedException { - // Expect onChanged when we observe, when the data is read, and when we stop observing. - final CountDownLatch latch = new CountDownLatch(2); - Observer<ImmutableList<Event>> observer = (value) -> latch.countDown(); - mEventsLiveData.observeForever(observer); + // Observing triggers content to be read. + Observer<ImmutableList<Event>> observer = (unused) -> { /* Do nothing */ }; + runOnMain(() -> mEventsLiveData.observeForever(observer)); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); - - final CountDownLatch latch2 = new CountDownLatch(1); - mEventsLiveData.removeObserver(observer); - - // Wait for the observer to be unregistered on the background thread. - latch2.await(5, TimeUnit.SECONDS); + mTestContentProvider.awaitCalendarQuery(); - assertThat(mTestContentProvider.mTestEventCursor.mLastContentObserver).isNull(); + awaitAndAssertDone(mTestContentProvider.mTestEventCursor.mRegisterContentObserverLatch); + runOnMain(() -> mEventsLiveData.removeObserver(observer)); + awaitAndAssertDone(mTestContentProvider.mTestEventCursor.mUnregisterContentObserverLatch); } @Test public void addObserver_oneEventResult() throws InterruptedException { - mTestContentProvider.addRow(buildTestRowWithDuration(CURRENT_DATE_TIME, 1)); // Expect onChanged to be called for when we start to observe and when the data is read. @@ -193,7 +185,7 @@ public class EventsLiveDataTest { runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + awaitAndAssertDone(latch); ImmutableList<Event> events = mEventsLiveData.getValue(); assertThat(events).isNotNull(); @@ -213,7 +205,9 @@ public class EventsLiveDataTest { } @Test - public void changeCursorData_onChangedCalled() throws InterruptedException { + public void notifyDataChange_dataNotChanged_onChangedNotCalled() throws InterruptedException { + mTestContentProvider.addRow(buildTestRow()); + // Expect onChanged to be called for when we start to observe and when the data is read. CountDownLatch initializeCountdownLatch = new CountDownLatch(2); @@ -231,32 +225,54 @@ public class EventsLiveDataTest { })); // Wait for the data to be read on the background thread. - initializeCountdownLatch.await(5, TimeUnit.SECONDS); + awaitAndAssertDone(initializeCountdownLatch); - // Signal that the content has changed. + // Signal that the content has changed but do not update the data. mTestContentProvider.mTestEventCursor.signalDataChanged(); // Wait for the changed data to be read on the background thread. - changeCountdownLatch.await(5, TimeUnit.SECONDS); - } - - private void runOnMain(Runnable runnable) { - InstrumentationRegistry.getInstrumentation().runOnMainSync(runnable); + awaitAndAssertNotDone(changeCountdownLatch); } @Test - public void addObserver_updateScheduled() throws InterruptedException { - mTestHandler.setExpectedMessageCount(2); + public void notifyDataChange_dataChanged_onChangedCalled() throws InterruptedException { + mTestContentProvider.addRow(buildTestRow()); + + // Expect onChanged to be called for when we start to observe and when the data is read. + CountDownLatch initializeCountdownLatch = new CountDownLatch(2); + + // Expect the same init callbacks as above but with an extra when the data is updated. + CountDownLatch changeCountdownLatch = new CountDownLatch(3); // Must add observer on main thread. runOnMain( () -> mEventsLiveData.observeForever( + // Count down both latches when data is changed. (value) -> { - /* Do nothing */ + initializeCountdownLatch.countDown(); + changeCountdownLatch.countDown(); })); - mTestHandler.awaitExpectedMessages(5); + // Wait for the data to be read on the background thread. + awaitAndAssertDone(initializeCountdownLatch); + + // Change the data and signal that the content has changed. + mTestContentProvider.addRow(buildTestRowWithTitle("Another event")); + mTestContentProvider.mTestEventCursor.signalDataChanged(); + + // Wait for the changed data to be read on the background thread. + awaitAndAssertDone(changeCountdownLatch); + } + + @Test + public void addObserver_updateScheduled() throws InterruptedException { + mTestHandler.setExpectedMessageCount(2); + + // Must add observer on main thread. + runOnMain(() -> mEventsLiveData.observeForever((unused) -> {/* Do nothing */ })); + + mTestHandler.awaitExpectedMessages(); // Show that a message was scheduled for the future. assertThat(mTestHandler.mLastUptimeMillis).isAtLeast(SystemClock.uptimeMillis()); @@ -265,13 +281,14 @@ public class EventsLiveDataTest { @Test public void noCalendars_valueNull() throws InterruptedException { mTestContentProvider.mAddFakeCalendar = false; + mTestContentProvider.addRow(buildTestRow()); // Expect onChanged to be called for when we start to observe and when the data is read. CountDownLatch latch = new CountDownLatch(2); runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + awaitAndAssertDone(latch); assertThat(mEventsLiveData.getValue()).isNull(); } @@ -280,15 +297,9 @@ public class EventsLiveDataTest { @UiThreadTest public void noCalendars_contentObserved() throws InterruptedException { mTestContentProvider.mAddFakeCalendar = false; - - // Expect onChanged to be called for when we start to observe and when the data is read. - CountDownLatch latch = new CountDownLatch(2); - mEventsLiveData.observeForever((value) -> latch.countDown()); - - // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); - - assertThat(mTestContentProvider.mTestEventCursor.mLastContentObserver).isNotNull(); + mEventsLiveData.observeForever((unused) -> { /* Do nothing */ }); + mTestContentProvider.awaitCalendarQuery(); + awaitAndAssertDone(mTestContentProvider.mTestEventCursor.mRegisterContentObserverLatch); } @Test @@ -302,7 +313,7 @@ public class EventsLiveDataTest { runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + awaitAndAssertDone(latch); // Expect an event for the 2 parts of the split event instance. assertThat(mEventsLiveData.getValue()).hasSize(2); @@ -320,7 +331,7 @@ public class EventsLiveDataTest { runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + awaitAndAssertDone(latch); Event middlePartEvent = mEventsLiveData.getValue().get(1); @@ -344,7 +355,7 @@ public class EventsLiveDataTest { runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + awaitAndAssertDone(latch); ImmutableList<Event> events = mEventsLiveData.getValue(); @@ -357,9 +368,9 @@ public class EventsLiveDataTest { @Test public void multipleEvents_resultsSortedTitle() throws InterruptedException { // Replace the default event with two that are out of time order. - mTestContentProvider.addRow(buildTestRowWithTitle(CURRENT_DATE_TIME, "Title B")); - mTestContentProvider.addRow(buildTestRowWithTitle(CURRENT_DATE_TIME, "Title A")); - mTestContentProvider.addRow(buildTestRowWithTitle(CURRENT_DATE_TIME, "Title C")); + mTestContentProvider.addRow(buildTestRowWithTitle("Title B")); + mTestContentProvider.addRow(buildTestRowWithTitle("Title A")); + mTestContentProvider.addRow(buildTestRowWithTitle("Title C")); // Expect onChanged to be called for when we start to observe and when the data is read. CountDownLatch latch = new CountDownLatch(2); @@ -367,7 +378,7 @@ public class EventsLiveDataTest { runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + awaitAndAssertDone(latch); ImmutableList<Event> events = mEventsLiveData.getValue(); @@ -389,7 +400,7 @@ public class EventsLiveDataTest { runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + awaitAndAssertDone(latch); ImmutableList<Event> events = mEventsLiveData.getValue(); @@ -413,16 +424,29 @@ public class EventsLiveDataTest { runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); // Wait for the data to be read on the background thread. - latch.await(5, TimeUnit.SECONDS); + awaitAndAssertDone(latch); // Show that the event is included even though its end time is before the current time. assertThat(mEventsLiveData.getValue()).isNotEmpty(); } + private void runOnMain(Runnable runnable) { + InstrumentationRegistry.getInstrumentation().runOnMainSync(runnable); + } + + private static void awaitAndAssertDone(CountDownLatch latch) throws InterruptedException { + assertThat(latch.await(2, TimeUnit.SECONDS)).isTrue(); + } + + private static void awaitAndAssertNotDone(CountDownLatch latch) throws InterruptedException { + assertThat(latch.await(2, TimeUnit.SECONDS)).isFalse(); + } + private static class TestContentProvider extends MockContentProvider { TestEventCursor mTestEventCursor; boolean mAddFakeCalendar = true; List<Object[]> mEventRows = new ArrayList<>(); + CountDownLatch mCalendarQueryLatch = new CountDownLatch(1); TestContentProvider(Context context) { super(context); @@ -449,14 +473,20 @@ public class EventsLiveDataTest { if (mAddFakeCalendar) { calendarsCursor.addRow(new String[] {"Test value"}); } + mCalendarQueryLatch.countDown(); return calendarsCursor; } throw new IllegalStateException("Unexpected query uri " + uri); } + void awaitCalendarQuery() throws InterruptedException { + awaitAndAssertDone(mCalendarQueryLatch); + } + static class TestEventCursor extends MatrixCursor { final Uri mUri; - ContentObserver mLastContentObserver; + CountDownLatch mRegisterContentObserverLatch = new CountDownLatch(1); + CountDownLatch mUnregisterContentObserverLatch = new CountDownLatch(1); TestEventCursor(Uri uri) { super( @@ -477,13 +507,13 @@ public class EventsLiveDataTest { @Override public void registerContentObserver(ContentObserver observer) { super.registerContentObserver(observer); - mLastContentObserver = observer; + mRegisterContentObserverLatch.countDown(); } @Override public void unregisterContentObserver(ContentObserver observer) { super.unregisterContentObserver(observer); - mLastContentObserver = null; + mUnregisterContentObserverLatch.countDown(); } void signalDataChanged() { @@ -519,8 +549,8 @@ public class EventsLiveDataTest { mCountDownLatch = new CountDownLatch(expectedMessageCount); } - void awaitExpectedMessages(int seconds) throws InterruptedException { - mCountDownLatch.await(seconds, TimeUnit.SECONDS); + void awaitExpectedMessages() throws InterruptedException { + awaitAndAssertDone(mCountDownLatch); } @Override @@ -588,8 +618,12 @@ public class EventsLiveDataTest { return buildTestRowWithDuration(startDateTime, 24, EVENT_TITLE, true); } - static Object[] buildTestRowWithTitle(ZonedDateTime startDateTime, String title) { - return buildTestRowWithDuration(startDateTime, 1, title, EVENT_ALL_DAY); + static Object[] buildTestRowWithTitle(String title) { + return buildTestRowWithDuration(CURRENT_DATE_TIME, 1, title, EVENT_ALL_DAY); + } + + static Object[] buildTestRow() { + return buildTestRowWithDuration(CURRENT_DATE_TIME, 1, EVENT_TITLE, EVENT_ALL_DAY); } static Object[] buildTestRowWithDuration( |