TimeDetector improvements
TimeDetector improvements for production code and tests. Many of these are based on feedback given for TimeZoneDetector and to improve consistency across the two. 1) Add a builder for PhoneTimeSuggestion, making it immutable (except for debug info that doesn't affect behavior). 2) Improve debug logging. 3) Improve comments / annotations. Test: atest android.app.timedetector Test: atest com.android.server.timedetector Bug: 140712361 Change-Id: Ibd0d357c536d512ffe4995c4e6560e483a260ae5
This commit is contained in:
@@ -85,7 +85,8 @@ public final class ManualTimeSuggestion implements Parcelable {
|
||||
|
||||
@NonNull
|
||||
public List<String> getDebugInfo() {
|
||||
return Collections.unmodifiableList(mDebugInfo);
|
||||
return mDebugInfo == null
|
||||
? Collections.emptyList() : Collections.unmodifiableList(mDebugInfo);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -23,7 +23,6 @@ import android.os.Parcelable;
|
||||
import android.util.TimestampedValue;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Objects;
|
||||
@@ -52,20 +51,25 @@ public final class PhoneTimeSuggestion implements Parcelable {
|
||||
};
|
||||
|
||||
private final int mPhoneId;
|
||||
@Nullable private TimestampedValue<Long> mUtcTime;
|
||||
@Nullable private final TimestampedValue<Long> mUtcTime;
|
||||
@Nullable private ArrayList<String> mDebugInfo;
|
||||
|
||||
public PhoneTimeSuggestion(int phoneId) {
|
||||
mPhoneId = phoneId;
|
||||
private PhoneTimeSuggestion(Builder builder) {
|
||||
mPhoneId = builder.mPhoneId;
|
||||
mUtcTime = builder.mUtcTime;
|
||||
mDebugInfo = builder.mDebugInfo != null ? new ArrayList<>(builder.mDebugInfo) : null;
|
||||
}
|
||||
|
||||
private static PhoneTimeSuggestion createFromParcel(Parcel in) {
|
||||
int phoneId = in.readInt();
|
||||
PhoneTimeSuggestion suggestion = new PhoneTimeSuggestion(phoneId);
|
||||
suggestion.setUtcTime(in.readParcelable(null /* classLoader */));
|
||||
PhoneTimeSuggestion suggestion = new PhoneTimeSuggestion.Builder(phoneId)
|
||||
.setUtcTime(in.readParcelable(null /* classLoader */))
|
||||
.build();
|
||||
@SuppressWarnings("unchecked")
|
||||
ArrayList<String> debugInfo = (ArrayList<String>) in.readArrayList(null /* classLoader */);
|
||||
suggestion.mDebugInfo = debugInfo;
|
||||
if (debugInfo != null) {
|
||||
suggestion.addDebugInfo(debugInfo);
|
||||
}
|
||||
return suggestion;
|
||||
}
|
||||
|
||||
@@ -85,10 +89,6 @@ public final class PhoneTimeSuggestion implements Parcelable {
|
||||
return mPhoneId;
|
||||
}
|
||||
|
||||
public void setUtcTime(@Nullable TimestampedValue<Long> utcTime) {
|
||||
mUtcTime = utcTime;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
public TimestampedValue<Long> getUtcTime() {
|
||||
return mUtcTime;
|
||||
@@ -96,7 +96,8 @@ public final class PhoneTimeSuggestion implements Parcelable {
|
||||
|
||||
@NonNull
|
||||
public List<String> getDebugInfo() {
|
||||
return Collections.unmodifiableList(mDebugInfo);
|
||||
return mDebugInfo == null
|
||||
? Collections.emptyList() : Collections.unmodifiableList(mDebugInfo);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -104,11 +105,23 @@ public final class PhoneTimeSuggestion implements Parcelable {
|
||||
* information is present in {@link #toString()} but is not considered for
|
||||
* {@link #equals(Object)} and {@link #hashCode()}.
|
||||
*/
|
||||
public void addDebugInfo(String... debugInfos) {
|
||||
public void addDebugInfo(String debugInfo) {
|
||||
if (mDebugInfo == null) {
|
||||
mDebugInfo = new ArrayList<>();
|
||||
}
|
||||
mDebugInfo.addAll(Arrays.asList(debugInfos));
|
||||
mDebugInfo.add(debugInfo);
|
||||
}
|
||||
|
||||
/**
|
||||
* Associates information with the instance that can be useful for debugging / logging. The
|
||||
* information is present in {@link #toString()} but is not considered for
|
||||
* {@link #equals(Object)} and {@link #hashCode()}.
|
||||
*/
|
||||
public void addDebugInfo(@NonNull List<String> debugInfo) {
|
||||
if (mDebugInfo == null) {
|
||||
mDebugInfo = new ArrayList<>(debugInfo.size());
|
||||
}
|
||||
mDebugInfo.addAll(debugInfo);
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -137,4 +150,39 @@ public final class PhoneTimeSuggestion implements Parcelable {
|
||||
+ ", mDebugInfo=" + mDebugInfo
|
||||
+ '}';
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds {@link PhoneTimeSuggestion} instances.
|
||||
*
|
||||
* @hide
|
||||
*/
|
||||
public static class Builder {
|
||||
private final int mPhoneId;
|
||||
private TimestampedValue<Long> mUtcTime;
|
||||
private List<String> mDebugInfo;
|
||||
|
||||
public Builder(int phoneId) {
|
||||
mPhoneId = phoneId;
|
||||
}
|
||||
|
||||
/** Returns the builder for call chaining. */
|
||||
public Builder setUtcTime(TimestampedValue<Long> utcTime) {
|
||||
mUtcTime = utcTime;
|
||||
return this;
|
||||
}
|
||||
|
||||
/** Returns the builder for call chaining. */
|
||||
public Builder addDebugInfo(@NonNull String debugInfo) {
|
||||
if (mDebugInfo == null) {
|
||||
mDebugInfo = new ArrayList<>();
|
||||
}
|
||||
mDebugInfo.add(debugInfo);
|
||||
return this;
|
||||
}
|
||||
|
||||
/** Returns the {@link PhoneTimeSuggestion}. */
|
||||
public PhoneTimeSuggestion build() {
|
||||
return new PhoneTimeSuggestion(this);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
/*
|
||||
* Copyright 2019 The Android Open Source Project
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package android.app.timedetector;
|
||||
|
||||
import static android.app.timezonedetector.ParcelableTestSupport.assertRoundTripParcelable;
|
||||
import static android.app.timezonedetector.ParcelableTestSupport.roundTripParcelable;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNotEquals;
|
||||
|
||||
import android.util.TimestampedValue;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
public class ManualTimeSuggestionTest {
|
||||
|
||||
private static final TimestampedValue<Long> ARBITRARY_TIME =
|
||||
new TimestampedValue<>(1111L, 2222L);
|
||||
|
||||
@Test
|
||||
public void testEquals() {
|
||||
ManualTimeSuggestion one = new ManualTimeSuggestion(ARBITRARY_TIME);
|
||||
assertEquals(one, one);
|
||||
|
||||
ManualTimeSuggestion two = new ManualTimeSuggestion(ARBITRARY_TIME);
|
||||
assertEquals(one, two);
|
||||
assertEquals(two, one);
|
||||
|
||||
TimestampedValue<Long> differentTime = new TimestampedValue<>(
|
||||
ARBITRARY_TIME.getReferenceTimeMillis() + 1,
|
||||
ARBITRARY_TIME.getValue());
|
||||
ManualTimeSuggestion three = new ManualTimeSuggestion(differentTime);
|
||||
assertNotEquals(one, three);
|
||||
assertNotEquals(three, one);
|
||||
|
||||
// DebugInfo must not be considered in equals().
|
||||
one.addDebugInfo("Debug info 1");
|
||||
two.addDebugInfo("Debug info 2");
|
||||
assertEquals(one, two);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testParcelable() {
|
||||
ManualTimeSuggestion suggestion = new ManualTimeSuggestion(ARBITRARY_TIME);
|
||||
assertRoundTripParcelable(suggestion);
|
||||
|
||||
// DebugInfo should also be stored (but is not checked by equals()
|
||||
suggestion.addDebugInfo("This is debug info");
|
||||
ManualTimeSuggestion rtSuggestion = roundTripParcelable(suggestion);
|
||||
assertEquals(suggestion.getDebugInfo(), rtSuggestion.getDebugInfo());
|
||||
}
|
||||
}
|
||||
@@ -16,11 +16,12 @@
|
||||
|
||||
package android.app.timedetector;
|
||||
|
||||
import static android.app.timezonedetector.ParcelableTestSupport.assertRoundTripParcelable;
|
||||
import static android.app.timezonedetector.ParcelableTestSupport.roundTripParcelable;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNotEquals;
|
||||
|
||||
import android.os.Parcel;
|
||||
import android.os.Parcelable;
|
||||
import android.util.TimestampedValue;
|
||||
|
||||
import org.junit.Test;
|
||||
@@ -30,53 +31,67 @@ public class PhoneTimeSuggestionTest {
|
||||
|
||||
@Test
|
||||
public void testEquals() {
|
||||
PhoneTimeSuggestion one = new PhoneTimeSuggestion(PHONE_ID);
|
||||
assertEquals(one, one);
|
||||
PhoneTimeSuggestion.Builder builder1 = new PhoneTimeSuggestion.Builder(PHONE_ID);
|
||||
{
|
||||
PhoneTimeSuggestion one = builder1.build();
|
||||
assertEquals(one, one);
|
||||
}
|
||||
|
||||
PhoneTimeSuggestion two = new PhoneTimeSuggestion(PHONE_ID);
|
||||
assertEquals(one, two);
|
||||
assertEquals(two, one);
|
||||
PhoneTimeSuggestion.Builder builder2 = new PhoneTimeSuggestion.Builder(PHONE_ID);
|
||||
{
|
||||
PhoneTimeSuggestion one = builder1.build();
|
||||
PhoneTimeSuggestion two = builder2.build();
|
||||
assertEquals(one, two);
|
||||
assertEquals(two, one);
|
||||
}
|
||||
|
||||
one.setUtcTime(new TimestampedValue<>(1111L, 2222L));
|
||||
assertEquals(one, one);
|
||||
builder1.setUtcTime(new TimestampedValue<>(1111L, 2222L));
|
||||
{
|
||||
PhoneTimeSuggestion one = builder1.build();
|
||||
assertEquals(one, one);
|
||||
}
|
||||
|
||||
two.setUtcTime(new TimestampedValue<>(1111L, 2222L));
|
||||
assertEquals(one, two);
|
||||
assertEquals(two, one);
|
||||
builder2.setUtcTime(new TimestampedValue<>(1111L, 2222L));
|
||||
{
|
||||
PhoneTimeSuggestion one = builder1.build();
|
||||
PhoneTimeSuggestion two = builder2.build();
|
||||
assertEquals(one, two);
|
||||
assertEquals(two, one);
|
||||
}
|
||||
|
||||
PhoneTimeSuggestion three = new PhoneTimeSuggestion(PHONE_ID + 1);
|
||||
three.setUtcTime(new TimestampedValue<>(1111L, 2222L));
|
||||
assertNotEquals(one, three);
|
||||
assertNotEquals(three, one);
|
||||
PhoneTimeSuggestion.Builder builder3 = new PhoneTimeSuggestion.Builder(PHONE_ID + 1);
|
||||
builder3.setUtcTime(new TimestampedValue<>(1111L, 2222L));
|
||||
{
|
||||
PhoneTimeSuggestion one = builder1.build();
|
||||
PhoneTimeSuggestion three = builder3.build();
|
||||
assertNotEquals(one, three);
|
||||
assertNotEquals(three, one);
|
||||
}
|
||||
|
||||
// DebugInfo must not be considered in equals().
|
||||
one.addDebugInfo("Debug info 1");
|
||||
two.addDebugInfo("Debug info 2");
|
||||
assertEquals(one, two);
|
||||
builder1.addDebugInfo("Debug info 1");
|
||||
builder2.addDebugInfo("Debug info 2");
|
||||
{
|
||||
PhoneTimeSuggestion one = builder1.build();
|
||||
PhoneTimeSuggestion two = builder2.build();
|
||||
assertEquals(one, two);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testParcelable() {
|
||||
PhoneTimeSuggestion one = new PhoneTimeSuggestion(PHONE_ID);
|
||||
assertEquals(one, roundTripParcelable(one));
|
||||
PhoneTimeSuggestion.Builder builder = new PhoneTimeSuggestion.Builder(PHONE_ID);
|
||||
assertRoundTripParcelable(builder.build());
|
||||
|
||||
one.setUtcTime(new TimestampedValue<>(1111L, 2222L));
|
||||
assertEquals(one, roundTripParcelable(one));
|
||||
builder.setUtcTime(new TimestampedValue<>(1111L, 2222L));
|
||||
assertRoundTripParcelable(builder.build());
|
||||
|
||||
// DebugInfo should also be stored (but is not checked by equals()
|
||||
one.addDebugInfo("This is debug info");
|
||||
PhoneTimeSuggestion two = roundTripParcelable(one);
|
||||
assertEquals(one.getDebugInfo(), two.getDebugInfo());
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
private static <T extends Parcelable> T roundTripParcelable(T one) {
|
||||
Parcel parcel = Parcel.obtain();
|
||||
parcel.writeTypedObject(one, 0);
|
||||
parcel.setDataPosition(0);
|
||||
|
||||
T toReturn = (T) parcel.readTypedObject(PhoneTimeSuggestion.CREATOR);
|
||||
parcel.recycle();
|
||||
return toReturn;
|
||||
{
|
||||
PhoneTimeSuggestion suggestion1 = builder.build();
|
||||
builder.addDebugInfo("This is debug info");
|
||||
PhoneTimeSuggestion rtSuggestion1 = roundTripParcelable(suggestion1);
|
||||
assertEquals(suggestion1.getDebugInfo(), rtSuggestion1.getDebugInfo());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -150,7 +150,8 @@ public final class SimpleTimeDetectorStrategy implements TimeDetectorStrategy {
|
||||
if (!mCallback.isAutoTimeDetectionEnabled()) {
|
||||
if (DBG) {
|
||||
Slog.d(LOG_TAG, "Auto time detection is not enabled."
|
||||
+ " time=" + time
|
||||
+ " origin=" + origin
|
||||
+ ", time=" + time
|
||||
+ ", cause=" + cause);
|
||||
}
|
||||
return;
|
||||
@@ -159,7 +160,8 @@ public final class SimpleTimeDetectorStrategy implements TimeDetectorStrategy {
|
||||
if (mCallback.isAutoTimeDetectionEnabled()) {
|
||||
if (DBG) {
|
||||
Slog.d(LOG_TAG, "Auto time detection is enabled."
|
||||
+ " time=" + time
|
||||
+ " origin=" + origin
|
||||
+ ", time=" + time
|
||||
+ ", cause=" + cause);
|
||||
}
|
||||
return;
|
||||
@@ -232,16 +234,16 @@ public final class SimpleTimeDetectorStrategy implements TimeDetectorStrategy {
|
||||
|
||||
@Override
|
||||
public synchronized void dump(@NonNull PrintWriter pw, @Nullable String[] args) {
|
||||
pw.println("mLastPhoneSuggestion=" + mLastPhoneSuggestion);
|
||||
pw.println("mLastAutoSystemClockTimeSet=" + mLastAutoSystemClockTimeSet);
|
||||
pw.println("mLastAutoSystemClockTime=" + mLastAutoSystemClockTime);
|
||||
pw.println("mLastAutoSystemClockTimeSendNetworkBroadcast="
|
||||
IndentingPrintWriter ipw = new IndentingPrintWriter(pw, " ");
|
||||
ipw.println("TimeDetectorStrategy:");
|
||||
ipw.increaseIndent(); // level 1
|
||||
|
||||
ipw.println("mLastPhoneSuggestion=" + mLastPhoneSuggestion);
|
||||
ipw.println("mLastAutoSystemClockTimeSet=" + mLastAutoSystemClockTimeSet);
|
||||
ipw.println("mLastAutoSystemClockTime=" + mLastAutoSystemClockTime);
|
||||
ipw.println("mLastAutoSystemClockTimeSendNetworkBroadcast="
|
||||
+ mLastAutoSystemClockTimeSendNetworkBroadcast);
|
||||
|
||||
IndentingPrintWriter ipw = new IndentingPrintWriter(pw, " ");
|
||||
|
||||
ipw.println("TimeDetectorStrategyImpl logs:");
|
||||
ipw.increaseIndent(); // level 1
|
||||
|
||||
ipw.println("Time change log:");
|
||||
ipw.increaseIndent(); // level 2
|
||||
@@ -249,6 +251,7 @@ public final class SimpleTimeDetectorStrategy implements TimeDetectorStrategy {
|
||||
ipw.decreaseIndent(); // level 2
|
||||
|
||||
ipw.decreaseIndent(); // level 1
|
||||
ipw.flush();
|
||||
}
|
||||
|
||||
@GuardedBy("this")
|
||||
|
||||
@@ -39,6 +39,12 @@ public interface TimeDetectorStrategy {
|
||||
|
||||
/**
|
||||
* The interface used by the strategy to interact with the surrounding service.
|
||||
*
|
||||
* <p>Note: Because the system properties-derived value {@link #isAutoTimeDetectionEnabled()}
|
||||
* can be modified independently and from different threads (and processes!). its use is prone
|
||||
* to race conditions. That will be true until the responsibility for setting their values is
|
||||
* moved to {@link TimeDetectorStrategy}. There are similar issues with
|
||||
* {@link #systemClockMillis()} while any process can modify the system clock.
|
||||
*/
|
||||
interface Callback {
|
||||
|
||||
|
||||
@@ -640,9 +640,9 @@ public class SimpleTimeDetectorStrategyTest {
|
||||
|
||||
private static PhoneTimeSuggestion createPhoneTimeSuggestion(int phoneId,
|
||||
TimestampedValue<Long> utcTime) {
|
||||
PhoneTimeSuggestion timeSuggestion = new PhoneTimeSuggestion(phoneId);
|
||||
timeSuggestion.setUtcTime(utcTime);
|
||||
return timeSuggestion;
|
||||
return new PhoneTimeSuggestion.Builder(phoneId)
|
||||
.setUtcTime(utcTime)
|
||||
.build();
|
||||
}
|
||||
|
||||
private ManualTimeSuggestion createManualTimeSuggestion(long timeMillis) {
|
||||
|
||||
@@ -140,10 +140,10 @@ public class TimeDetectorServiceTest {
|
||||
|
||||
private static PhoneTimeSuggestion createPhoneTimeSuggestion() {
|
||||
int phoneId = 1234;
|
||||
PhoneTimeSuggestion suggestion = new PhoneTimeSuggestion(phoneId);
|
||||
TimestampedValue<Long> timeValue = new TimestampedValue<>(100L, 1_000_000L);
|
||||
suggestion.setUtcTime(timeValue);
|
||||
return suggestion;
|
||||
return new PhoneTimeSuggestion.Builder(phoneId)
|
||||
.setUtcTime(timeValue)
|
||||
.build();
|
||||
}
|
||||
|
||||
private static ManualTimeSuggestion createManualTimeSuggestion() {
|
||||
|
||||
Reference in New Issue
Block a user