From 06c9281eee70e8b34ec89f1e0aef7b174e29de2c Mon Sep 17 00:00:00 2001 From: Nancy Chen Date: Mon, 23 Nov 2015 11:26:38 -0800 Subject: Fix crash when business is closed today but open tomorrow. Attempted to index into -1 position of opening hours array when business is not open today but open tomorrow. That is because entries exist in the array for tomorrow, but not today. Bug: 25830349 Change-Id: Ib3def69e3d2813bf9cb4739a954b7b9f3724b56a --- .../incallui/InCallContactInteractions.java | 66 ++++++++++++++++------ .../incallui/InCallContactInteractionsTest.java | 13 ++++- 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/InCallUI/src/com/android/incallui/InCallContactInteractions.java b/InCallUI/src/com/android/incallui/InCallContactInteractions.java index 6fade9bdf..e62766844 100644 --- a/InCallUI/src/com/android/incallui/InCallContactInteractions.java +++ b/InCallUI/src/com/android/incallui/InCallContactInteractions.java @@ -49,6 +49,8 @@ import java.util.Locale; * is a business contact or not and logic for the manipulation of data for the call context. */ public class InCallContactInteractions { + private static final String TAG = InCallContactInteractions.class.getSimpleName(); + private Context mContext; private InCallContactInteractionsListAdapter mListAdapter; private boolean mIsBusiness; @@ -126,7 +128,13 @@ public class InCallContactInteractions { * business is open or not and the details set to the hours of operation. */ private BusinessContextInfo constructHoursInfo(List> openingHours) { - return constructHoursInfo(Calendar.getInstance(), openingHours); + try { + return constructHoursInfo(Calendar.getInstance(), openingHours); + } catch (Exception e) { + // Catch all exceptions here because we don't want any crashes if something goes wrong. + Log.e(TAG, "Error constructing hours info: ", e); + } + return null; } /** @@ -144,10 +152,12 @@ public class InCallContactInteractions { boolean isOpenNow = false; // This variable records which interval the current time is after. 0 denotes none of the - // intervals, 1 after the first interval, etc. + // intervals, 1 after the first interval, etc. It is also the index of the interval the + // current time is in (if open) or the next interval (if closed). int afterInterval = 0; - // This variable records counts the number of time intervals in today's opening hours. + // This variable counts the number of time intervals in today's opening hours. int todaysIntervalCount = 0; + for (Pair hours : openingHours) { if (hours.first.compareTo(currentTime) <= 0 && currentTime.compareTo(hours.second) < 0) { @@ -177,31 +187,53 @@ public class InCallContactInteractions { * - Display next opening time if currently closed but opens later today. * - Display last time it closed today if closed now and tomorrow's hours are unknown. * - Display tomorrow's first open time if closed today and tomorrow's hours are known. + * + * NOTE: The logic below assumes that the intervals are sorted by ascending time. Possible + * TODO to modify the logic above and ensure this is true. */ if (isOpenNow) { if (todaysIntervalCount == 1) { hoursInfo.detail = getTimeSpanStringForHours(openingHours.get(0)); } else if (todaysIntervalCount == 2) { - hoursInfo.detail = mContext.getString(R.string.opening_hours, + hoursInfo.detail = mContext.getString( + R.string.opening_hours, getTimeSpanStringForHours(openingHours.get(0)), getTimeSpanStringForHours(openingHours.get(1))); - } else { - hoursInfo.detail = mContext.getString(R.string.closes_today_at, + } else if (afterInterval < openingHours.size()) { + // This check should not be necessary since if it is currently open, we should not + // be after the last interval, but just in case, we don't want to crash. + hoursInfo.detail = mContext.getString( + R.string.closes_today_at, getFormattedTimeForCalendar(openingHours.get(afterInterval).second)); } - } else { - // Currently closed + } else { // Currently closed final int lastIntervalToday = todaysIntervalCount - 1; - if (currentTime.before(openingHours.get(lastIntervalToday).first)) { - hoursInfo.detail = mContext.getString(R.string.opens_today_at, + if (todaysIntervalCount == 0) { // closed today + hoursInfo.detail = mContext.getString( + R.string.opens_tomorrow_at, + getFormattedTimeForCalendar(openingHours.get(0).first)); + } else if (currentTime.after(openingHours.get(lastIntervalToday).second)) { + // Passed hours for today + if (todaysIntervalCount < openingHours.size()) { + // If all of today's intervals are exhausted, assume the next are tomorrow's. + hoursInfo.detail = mContext.getString( + R.string.opens_tomorrow_at, + getFormattedTimeForCalendar( + openingHours.get(todaysIntervalCount).first)); + } else { + // Grab the last time it was open today. + hoursInfo.detail = mContext.getString( + R.string.closed_today_at, + getFormattedTimeForCalendar( + openingHours.get(lastIntervalToday).second)); + } + } else if (afterInterval < openingHours.size()) { + // This check should not be necessary since if it is currently before the last + // interval, afterInterval should be less than the count of intervals, but just in + // case, we don't want to crash. + hoursInfo.detail = mContext.getString( + R.string.opens_today_at, getFormattedTimeForCalendar(openingHours.get(afterInterval).first)); - } else if (todaysIntervalCount < openingHours.size()){ - // Assuming all intervals after today's intervals are exhausted are tomorrow's. - hoursInfo.detail = mContext.getString(R.string.opens_tomorrow_at, - getFormattedTimeForCalendar(openingHours.get(todaysIntervalCount).first)); - } else { - hoursInfo.detail = mContext.getString(R.string.closed_today_at, - getFormattedTimeForCalendar(openingHours.get(lastIntervalToday).second)); } } diff --git a/InCallUI/tests/src/com/android/incallui/InCallContactInteractionsTest.java b/InCallUI/tests/src/com/android/incallui/InCallContactInteractionsTest.java index 1c2e9ded7..625cda448 100644 --- a/InCallUI/tests/src/com/android/incallui/InCallContactInteractionsTest.java +++ b/InCallUI/tests/src/com/android/incallui/InCallContactInteractionsTest.java @@ -174,6 +174,17 @@ public class InCallContactInteractionsTest extends AndroidTestCase { .detail); } + public void testOpeningHours_NotOpenTodayOpenTomorrow() { + assertEquals("Opens tomorrow at 8:00 AM", + mInCallContactInteractions.constructHoursInfo( + getTestCalendarWithHour(21), + Arrays.asList( + Pair.create( + getTestCalendarWithHourAndDaysFromToday(8, 1), + getTestCalendarWithHourAndDaysFromToday(10, 1)))) + .detail); + } + public void testMultipleOpenRanges_BeforeOpen() { assertEquals("Opens today at 8:00 AM", mInCallContactInteractions.constructHoursInfo( @@ -214,7 +225,7 @@ public class InCallContactInteractionsTest extends AndroidTestCase { .detail); } - public void testInvalidOpeningHours() { + public void testNotOpenTodayOrTomorrow() { assertEquals(null, mInCallContactInteractions.constructHoursInfo( getTestCalendarWithHour(21), -- cgit v1.2.3