From 9c08312784ae07634761ea3be402d6478a57e385 Mon Sep 17 00:00:00 2001 From: John Patterson Date: Fri, 28 Aug 2020 13:51:40 +0200 Subject: Do not set an initial value for events. This avoids the empty events being used to show "no events" message. Fixes: 153622413 Test: atest CarCalendarUnitTests CarCalendarUiTests Change-Id: Ib02ba2d8682bd92a560b73b0db4702ec4ac45e73 --- src/com/android/car/calendar/CarCalendarView.java | 39 +++++------ .../car/calendar/common/EventsLiveData.java | 22 +++++-- .../car/calendar/common/EventsLiveDataTest.java | 77 +++++++++++++--------- 3 files changed, 82 insertions(+), 56 deletions(-) diff --git a/src/com/android/car/calendar/CarCalendarView.java b/src/com/android/car/calendar/CarCalendarView.java index 07b9516..1a63588 100644 --- a/src/com/android/car/calendar/CarCalendarView.java +++ b/src/com/android/car/calendar/CarCalendarView.java @@ -17,7 +17,6 @@ package com.android.car.calendar; import static com.google.common.base.Verify.verify; -import static com.google.common.base.Verify.verifyNotNull; import android.Manifest; import android.util.Log; @@ -25,10 +24,10 @@ import android.view.View; import android.view.ViewGroup; import android.widget.TextView; -import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.lifecycle.Observer; import androidx.recyclerview.widget.RecyclerView; +import androidx.recyclerview.widget.RecyclerView.ViewHolder; import com.android.car.calendar.common.CalendarFormatter; import com.android.car.calendar.common.Dialer; @@ -64,15 +63,8 @@ class CarCalendarView { /** Holds an instance of either {@link LocalDate} or {@link Event} for each item in the list. */ private final List mRecyclerViewItems = new ArrayList<>(); - private final RecyclerView.Adapter mAdapter = new EventRecyclerViewAdapter(); - private final Observer> mEventsObserver = - events -> { - if (DEBUG) Log.d(TAG, "Events changed"); - updateRecyclerViewItems(events); - - // TODO(jdp) Only change the affected items (DiffUtil) to allow animated changes. - mAdapter.notifyDataSetChanged(); - }; + private final RecyclerView.Adapter mAdapter = new EventRecyclerViewAdapter(); + private final Observer> mEventsObserver = this::onEventsChanged; CarCalendarView( CarCalendarActivity carCalendarActivity, @@ -102,23 +94,30 @@ class CarCalendarView { private void showWithPermission() { EventsLiveData eventsLiveData = mCarCalendarViewModel.getEventsLiveData(); eventsLiveData.observe(mCarCalendarActivity, mEventsObserver); - updateRecyclerViewItems(verifyNotNull(eventsLiveData.getValue())); + } + + private void onEventsChanged(ImmutableList events) { + updateRecyclerViewItems(events); + + // TODO(jdp) Only change the affected items (DiffUtil) to allow animated changes. + mAdapter.notifyDataSetChanged(); } /** * If the events list is null there is no calendar data available. If the events list is empty * there is calendar data but no events. */ - private void updateRecyclerViewItems(@Nullable ImmutableList carCalendarEvents) { + private void updateRecyclerViewItems(@Nullable ImmutableList events) { + if (DEBUG) Log.d(TAG, "Update events"); LocalDate currentDate = null; mRecyclerViewItems.clear(); - if (carCalendarEvents == null) { + if (events == null) { mNoEventsTextView.setVisibility(View.VISIBLE); mNoEventsTextView.setText(R.string.no_calendars); return; } - if (carCalendarEvents.isEmpty()) { + if (events.isEmpty()) { mNoEventsTextView.setVisibility(View.VISIBLE); mNoEventsTextView.setText(R.string.no_events); return; @@ -130,7 +129,7 @@ class CarCalendarView { // add the event rows after looking at all events for the day. List eventItems = null; List allDayEventItems = null; - for (Event event : carCalendarEvents) { + for (Event event : events) { LocalDate date = event.getDayStartInstant().atZone(ZoneId.systemDefault()).toLocalDate(); @@ -177,17 +176,15 @@ class CarCalendarView { mRecyclerViewItems.addAll(eventItems); } - private class EventRecyclerViewAdapter extends RecyclerView.Adapter { - - @NonNull + private class EventRecyclerViewAdapter extends RecyclerView.Adapter { @Override - public RecyclerView.ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) { + public ViewHolder onCreateViewHolder(ViewGroup parent, int viewType) { CalendarItem.Type type = CalendarItem.Type.values()[viewType]; return type.createViewHolder(parent); } @Override - public void onBindViewHolder(@NonNull RecyclerView.ViewHolder holder, int position) { + public void onBindViewHolder(ViewHolder holder, int position) { mRecyclerViewItems.get(position).bind(holder); } diff --git a/src/com/android/car/calendar/common/EventsLiveData.java b/src/com/android/car/calendar/common/EventsLiveData.java index f2df3fe..92ae0bb 100644 --- a/src/com/android/car/calendar/common/EventsLiveData.java +++ b/src/com/android/car/calendar/common/EventsLiveData.java @@ -31,7 +31,9 @@ import android.provider.CalendarContract; import android.provider.CalendarContract.Instances; import android.util.Log; +import androidx.lifecycle.LifecycleOwner; import androidx.lifecycle.LiveData; +import androidx.lifecycle.Observer; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; @@ -54,6 +56,8 @@ import javax.annotation.Nullable; * Provider. * *

While in the active state the content provider is observed for changes. + * + *

When the value given to the observer is null it signals that there are no calendars. */ public class EventsLiveData extends LiveData> { @@ -72,20 +76,22 @@ public class EventsLiveData extends LiveData> { private final ContentResolver mContentResolver; private final EventDescriptions mEventDescriptions; private final EventLocations mLocations; - private final Runnable mUpdateRunnable = this::updateIfChanged; + private final Runnable mUpdateIfChangedRunnable = this::updateIfChanged; /** The event instances cursor is a field to allow observers to be managed. */ @Nullable private Cursor mEventsCursor; @Nullable private ContentObserver mEventInstancesObserver; + // This can be updated on the background thread but read from any thread. + private volatile boolean mValueUpdated; + public EventsLiveData( Clock clock, Handler backgroundHandler, ContentResolver contentResolver, EventDescriptions eventDescriptions, EventLocations locations) { - super(ImmutableList.of()); mClock = clock; mBackgroundHandler = backgroundHandler; mContentResolver = contentResolver; @@ -95,10 +101,14 @@ public class EventsLiveData extends LiveData> { /** Refreshes the event instances and sets the new value which notifies observers. */ private void updateIfChanged() { + Log.d(TAG, "Update if changed"); ImmutableList latest = getEventsUntilTomorrow(); ImmutableList current = getValue(); - if (!Objects.equals(latest, current)) { + + // Always post the first value even if it is null. + if (!mValueUpdated || !Objects.equals(latest, current)) { postValue(latest); + mValueUpdated = true; } } @@ -187,8 +197,9 @@ public class EventsLiveData extends LiveData> { 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); + Log.d(TAG, "Events changed"); + mBackgroundHandler.removeCallbacks(mUpdateIfChangedRunnable); + mBackgroundHandler.postDelayed(mUpdateIfChangedRunnable, UPDATE_DELAY_MILLIS); } /** Can return multiple events for a single cursor row when an event spans multiple days. */ @@ -277,6 +288,7 @@ public class EventsLiveData extends LiveData> { if (DEBUG) Log.d(TAG, "Live data inactive"); mBackgroundHandler.post(this::cancelScheduledUpdate); mBackgroundHandler.post(this::tearDownCursor); + mValueUpdated = false; } /** Calls {@link #updateIfChanged()} every minute to keep the displayed time range correct. */ 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 81b881d..79b5e29 100644 --- a/tests/unit/src/com/android/car/calendar/common/EventsLiveDataTest.java +++ b/tests/unit/src/com/android/car/calendar/common/EventsLiveDataTest.java @@ -18,10 +18,11 @@ 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; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static java.time.temporal.ChronoUnit.HOURS; @@ -160,6 +161,28 @@ public class EventsLiveDataTest { awaitAndAssertDone(mTestContentProvider.mTestEventCursor.mRegisterContentObserverLatch); } + @Test + public void addObserver_observerCalled() throws InterruptedException { + // Observing triggers content to be read. + Observer> mockObserver = mock(Observer.class); + runOnMain(() -> mEventsLiveData.observeForever(mockObserver)); + + // TODO(jdp) This method of verifying an async behaviour is easier to read. + verify(mockObserver, timeout(1000).times(1)).onChanged(any()); + } + + @Test + public void addTwoObservers_bothObserversCalled() throws InterruptedException { + // Observing triggers content to be read. + Observer> mockObserver1 = mock(Observer.class); + runOnMain(() -> mEventsLiveData.observeForever(mockObserver1)); + Observer> mockObserver2 = mock(Observer.class); + runOnMain(() -> mEventsLiveData.observeForever(mockObserver2)); + + verify(mockObserver1, timeout(1000).times(1)).onChanged(any()); + verify(mockObserver2, timeout(1000).times(1)).onChanged(any()); + } + @Test public void removeObserver_contentNotObserved() throws InterruptedException { // Observing triggers content to be read. @@ -178,8 +201,8 @@ public class EventsLiveDataTest { 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. - CountDownLatch latch = new CountDownLatch(2); + // Expect onChanged to be called for when the data is read. + CountDownLatch latch = new CountDownLatch(1); // Must add observer on main thread. runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); @@ -208,11 +231,11 @@ public class EventsLiveDataTest { 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); + // Expect onChanged to be called for when the data is read. + CountDownLatch initializeCountdownLatch = new CountDownLatch(1); - // Expect the same init callbacks as above but with an extra when the data is updated. - CountDownLatch changeCountdownLatch = new CountDownLatch(3); + // Expect the same callback as above but with an extra when the data is updated. + CountDownLatch changeCountdownLatch = new CountDownLatch(2); // Must add observer on main thread. runOnMain( @@ -238,11 +261,11 @@ public class EventsLiveDataTest { 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 onChanged to be called for when the data is read. + CountDownLatch initializeCountdownLatch = new CountDownLatch(1); - // Expect the same init callbacks as above but with an extra when the data is updated. - CountDownLatch changeCountdownLatch = new CountDownLatch(3); + // Expect the same callback as above but with an extra when the data is updated. + CountDownLatch changeCountdownLatch = new CountDownLatch(2); // Must add observer on main thread. runOnMain( @@ -270,7 +293,7 @@ public class EventsLiveDataTest { mTestHandler.setExpectedMessageCount(2); // Must add observer on main thread. - runOnMain(() -> mEventsLiveData.observeForever((unused) -> {/* Do nothing */ })); + runOnMain(() -> mEventsLiveData.observeForever((unused) -> { /* Do nothing */ })); mTestHandler.awaitExpectedMessages(); @@ -283,8 +306,8 @@ public class EventsLiveDataTest { 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); + // Expect onChanged to be called for when the data is read. + CountDownLatch latch = new CountDownLatch(1); runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); // Wait for the data to be read on the background thread. @@ -307,8 +330,7 @@ public class EventsLiveDataTest { // Replace the default event with one that lasts 24 hours. mTestContentProvider.addRow(buildTestRowWithDuration(CURRENT_DATE_TIME, 24)); - // Expect onChanged to be called for when we start to observe and when the data is read. - CountDownLatch latch = new CountDownLatch(2); + CountDownLatch latch = new CountDownLatch(1); runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); @@ -325,8 +347,7 @@ public class EventsLiveDataTest { int hours = 48; mTestContentProvider.addRow(buildTestRowWithDuration(CURRENT_DATE_TIME, hours)); - // Expect onChanged to be called for when we start to observe and when the data is read. - CountDownLatch latch = new CountDownLatch(2); + CountDownLatch latch = new CountDownLatch(1); runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); @@ -349,8 +370,7 @@ public class EventsLiveDataTest { mTestContentProvider.addRow(buildTestRowWithDuration(twoHoursAfterCurrentTime, 1)); mTestContentProvider.addRow(buildTestRowWithDuration(CURRENT_DATE_TIME, 1)); - // Expect onChanged to be called for when we start to observe and when the data is read. - CountDownLatch latch = new CountDownLatch(2); + CountDownLatch latch = new CountDownLatch(1); runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); @@ -372,8 +392,8 @@ public class EventsLiveDataTest { 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); + // Expect onChanged to be called for when the data is read. + CountDownLatch latch = new CountDownLatch(1); runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); @@ -394,8 +414,8 @@ public class EventsLiveDataTest { CURRENT_DATE_TIME.withZoneSameLocal(ZoneId.of("UTC")).truncatedTo(ChronoUnit.DAYS); mTestContentProvider.addRow(buildTestRowAllDay(utcMidnightStart)); - // Expect onChanged to be called for when we start to observe and when the data is read. - CountDownLatch latch = new CountDownLatch(2); + // Expect onChanged to be called when the data is read. + CountDownLatch latch = new CountDownLatch(1); runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); @@ -418,8 +438,8 @@ public class EventsLiveDataTest { // Set the time to 23:XX in the BERLIN_ZONE_ID which will be after the event end time. mTestClock.setTime(CURRENT_DATE_TIME.with(ChronoField.HOUR_OF_DAY, 23)); - // Expect onChanged to be called for when we start to observe and when the data is read. - CountDownLatch latch = new CountDownLatch(2); + // Expect onChanged to be called for when the data is read. + CountDownLatch latch = new CountDownLatch(1); runOnMain(() -> mEventsLiveData.observeForever((value) -> latch.countDown())); @@ -603,10 +623,7 @@ public class EventsLiveDataTest { } static long addHoursAndTruncate(ZonedDateTime dateTime, int hours) { - return dateTime.truncatedTo(HOURS) - .plus(Duration.ofHours(hours)) - .toInstant() - .toEpochMilli(); + return dateTime.truncatedTo(HOURS).plus(Duration.ofHours(hours)).toInstant().toEpochMilli(); } static Object[] buildTestRowWithDuration(ZonedDateTime startDateTime, int eventDurationHours) { -- cgit v1.2.3