Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

General improvements / refactoring

Browse Source 
Fix documentation, command line formatting and improve logging. No
functional changes.

To improve logging, the refreshIfRequiredAndReschedule() method has been
renamed to refreshAndRescheduleIfRequired() (because it now may not
reschedule) and it can now accept a null Network parameter to log the
case that was previously handled in the untested code. Tests have been
updated accordingly.

Test: manual / treehugger
Test: atest services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java
Bug: 269425914
Change-Id: Ib54da77d85ae0949b560b22659263f4f13f7a61e
This commit is contained in:
Neil Fuller
2023-02-15 18:15:10 +00:00
parent 6b5b4c4181
commit 9511093518
4 changed files with 78 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java
View File

@@ -228,16 +228,14 @@ public class NetworkTimeUpdateService extends Binder {
}
private void onPollNetworkTime(@NonNull String reason) {
// If we don't have any default network, don't bother.
Network network;
synchronized (mLock) {
network = mDefaultNetwork;
}
if (network == null) return;
mWakeLock.acquire();
try {
mEngine.refreshIfRequiredAndReschedule(network, reason, mRefreshCallbacks);
mEngine.refreshAndRescheduleIfRequired(network, reason, mRefreshCallbacks);
} finally {
mWakeLock.release();
}
@@ -338,10 +336,10 @@ public class NetworkTimeUpdateService extends Binder {
* Attempts to refresh the network time if required, i.e. if there isn't a recent-enough
* network time available. It must also schedule the next call. This is a blocking call.
*
* @param network the network to use
* @param network the network to use, or null if no network is available
* @param reason the reason for the refresh (for logging)
*/
void refreshIfRequiredAndReschedule(@NonNull Network network, @NonNull String reason,
void refreshAndRescheduleIfRequired(@Nullable Network network, @NonNull String reason,
@NonNull RefreshCallbacks refreshCallbacks);
void dump(@NonNull PrintWriter pw);
@@ -391,7 +389,7 @@ public class NetworkTimeUpdateService extends Binder {
/**
* Records the time of the last refresh attempt (successful or otherwise) by this service.
* This is used when scheduling the next refresh attempt. In cases where {@link
* #refreshIfRequiredAndReschedule} is called too frequently, this will prevent each call
* #refreshAndRescheduleIfRequired} is called too frequently, this will prevent each call
* resulting in a network request. See also {@link #mShortPollingIntervalMillis}.
*
* <p>Time servers are a shared resource and so Android should avoid loading them.
@@ -443,9 +441,19 @@ public class NetworkTimeUpdateService extends Binder {
}
@Override
public void refreshIfRequiredAndReschedule(
@NonNull Network network, @NonNull String reason,
public void refreshAndRescheduleIfRequired(
@Nullable Network network, @NonNull String reason,
@NonNull RefreshCallbacks refreshCallbacks) {
if (network == null) {
// If we don't have any default network, don't do anything: When a new network
// is available then this method will be called again.
logToDebugAndDumpsys("refreshIfRequiredAndReschedule:"
+ " reason=" + reason
+ ": No default network available. No refresh attempted and no next"
+ " attempt scheduled.");
return;
}
// Attempt to refresh the network time if there is no latest time result, or if the
// latest time result is considered too old.
NtpTrustedTime.TimeResult initialTimeResult = mNtpTrustedTime.getCachedTimeResult();

2
services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java
View File

@@ -157,7 +157,7 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand {
SET_SERVER_CONFIG_SERVER_ARG, SET_SERVER_CONFIG_SERVER_ARG,
SET_SERVER_CONFIG_TIMEOUT_ARG);
pw.printf(" NTP server URIs must be in the form \"ntp://hostname\" or"
+ " \"ntp://hostname:port\"");
+ " \"ntp://hostname:port\"\n");
pw.printf(" %s\n", SHELL_COMMAND_RESET_SERVER_CONFIG);
pw.printf(" Resets/clears the NTP server config set via %s.\n",
SHELL_COMMAND_SET_SERVER_CONFIG);

11
services/core/java/com/android/server/timedetector/ServerFlags.java
View File

@@ -304,7 +304,7 @@ public final class ServerFlags {
/**
* Returns an {@link Instant} from {@link DeviceConfig} from the system_time
* namespace, returns the {@code defaultValue} if the value is missing or invalid.
* namespace, returns {@link Optional#empty()} if there is no explicit value set.
*/
@NonNull
public Optional<Instant> getOptionalInstant(@DeviceConfigKey String key) {
@@ -341,16 +341,17 @@ public final class ServerFlags {
}
/**
* Returns a boolean value from {@link DeviceConfig} from the system_time
* namespace, or {@code defaultValue} if there is no explicit value set.
* Returns a boolean value from {@link DeviceConfig} from the system_time namespace, or
* {@code defaultValue} if there is no explicit value set.
*/
public boolean getBoolean(@DeviceConfigKey String key, boolean defaultValue) {
return DeviceConfig.getBoolean(NAMESPACE_SYSTEM_TIME, key, defaultValue);
}
/**
* Returns a positive duration from {@link DeviceConfig} from the system_time
* namespace, or {@code defaultValue} if there is no explicit value set.
* Returns a positive duration from {@link DeviceConfig} from the system_time namespace,
* or {@code defaultValue} if there is no explicit value set or if the value is not a number or
* is negative.
*/
@Nullable
public Duration getDurationFromMillis(

88
services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java
View File

@@ -22,6 +22,7 @@ import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import android.app.time.UnixEpochTime;
@@ -60,7 +61,28 @@ public class NetworkTimeUpdateServiceTest {
}
@Test
public void engineImpl_refreshIfRequiredAndReschedule_success() {
public void engineImpl_refreshAndRescheduleIfRequired_nullNetwork() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
int shortPollingIntervalMillis = 3333;
int tryAgainTimesMax = 5;
NetworkTimeUpdateService.Engine engine = new NetworkTimeUpdateService.EngineImpl(
mFakeElapsedRealtimeClock,
normalPollingIntervalMillis, shortPollingIntervalMillis, tryAgainTimesMax,
mMockNtpTrustedTime);
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshAndRescheduleIfRequired(null, "Test", mockCallback);
// Expect nothing to have happened.
verifyNoMoreInteractions(mMockNtpTrustedTime);
verifyNoMoreInteractions(mockCallback);
}
@Test
public void engineImpl_refreshAndRescheduleIfRequired_success() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
@@ -83,7 +105,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect the refresh attempt to have been made.
verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork);
@@ -98,7 +120,7 @@ public class NetworkTimeUpdateServiceTest {
}
@Test
public void engineImpl_refreshIfRequiredAndReschedule_failThenFailRepeatedly() {
public void engineImpl_refreshAndRescheduleIfRequired_failThenFailRepeatedly() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
@@ -120,7 +142,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect a refresh attempt each time: there's no currently cached result.
verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork);
@@ -141,7 +163,7 @@ public class NetworkTimeUpdateServiceTest {
}
@Test
public void engineImpl_refreshIfRequiredAndReschedule_successThenFailRepeatedly() {
public void engineImpl_refreshAndRescheduleIfRequired_successThenFailRepeatedly() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
@@ -168,7 +190,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect the refresh attempt to have been made: there is no cached network time
// initially.
@@ -182,7 +204,7 @@ public class NetworkTimeUpdateServiceTest {
}
// Increment the current time by enough so that an attempt to refresh the time should be
// made every time refreshIfRequiredAndReschedule() is called.
// made every time refreshAndRescheduleIfRequired() is called.
mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis);
// Test multiple follow-up calls.
@@ -195,7 +217,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect a refresh attempt each time as the cached network time is too old.
verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork);
@@ -221,7 +243,7 @@ public class NetworkTimeUpdateServiceTest {
}
@Test
public void engineImpl_refreshIfRequiredAndReschedule_successThenFail_tryAgainTimesZero() {
public void engineImpl_refreshAndRescheduleIfRequired_successThenFail_tryAgainTimesZero() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
@@ -248,7 +270,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect the refresh attempt to have been made: there is no cached network time
// initially.
@@ -262,7 +284,7 @@ public class NetworkTimeUpdateServiceTest {
}
// Increment the current time by enough so that an attempt to refresh the time should be
// made every time refreshIfRequiredAndReschedule() is called.
// made every time refreshAndRescheduleIfRequired() is called.
mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis);
// Test multiple follow-up calls.
@@ -275,7 +297,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect a refresh attempt each time as the cached network time is too old.
verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork);
@@ -297,7 +319,7 @@ public class NetworkTimeUpdateServiceTest {
}
@Test
public void engineImpl_refreshIfRequiredAndReschedule_successThenFail_tryAgainTimesNegative() {
public void engineImpl_refreshAndRescheduleIfRequired_successThenFail_tryAgainTimesNegative() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
@@ -324,7 +346,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect the refresh attempt to have been made: there is no cached network time
// initially.
@@ -338,7 +360,7 @@ public class NetworkTimeUpdateServiceTest {
}
// Increment the current time by enough so that an attempt to refresh the time should be
// made every time refreshIfRequiredAndReschedule() is called.
// made every time refreshAndRescheduleIfRequired() is called.
mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis);
// Test multiple follow-up calls.
@@ -351,7 +373,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect a refresh attempt each time as the cached network time is too old.
verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork);
@@ -373,7 +395,7 @@ public class NetworkTimeUpdateServiceTest {
}
@Test
public void engineImpl_refreshIfRequiredAndReschedule_successFailSuccess() {
public void engineImpl_refreshAndRescheduleIfRequired_successFailSuccess() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
@@ -398,7 +420,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect the refresh attempt to have been made: there is no cached network time
// initially.
@@ -413,7 +435,7 @@ public class NetworkTimeUpdateServiceTest {
}
// Increment the current time by enough so that the cached time result is too old and an
// attempt to refresh the time should be made every time refreshIfRequiredAndReschedule() is
// attempt to refresh the time should be made every time refreshAndRescheduleIfRequired() is
// called.
mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis);
@@ -426,7 +448,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect the refresh attempt to have been made: the timeResult is too old.
verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork);
@@ -458,7 +480,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect the refresh attempt to have been made: the timeResult is too old.
verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork);
@@ -473,12 +495,12 @@ public class NetworkTimeUpdateServiceTest {
}
/**
* Confirms that if a refreshIfRequiredAndReschedule() call is made, e.g. for reasons besides
* Confirms that if a refreshAndRescheduleIfRequired() call is made, e.g. for reasons besides
* scheduled alerts, and the latest time is not too old, then an NTP refresh won't be attempted.
* A suggestion will still be made.
*/
@Test
public void engineImpl_refreshIfRequiredAndReschedule_noRefreshIfLatestIsFresh() {
public void engineImpl_refreshAndRescheduleIfRequired_noRefreshIfLatestIsFresh() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
@@ -498,7 +520,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect no refresh attempt to have been made.
verify(mMockNtpTrustedTime, never()).forceRefresh(any());
@@ -516,11 +538,11 @@ public class NetworkTimeUpdateServiceTest {
}
/**
* Confirms that if a refreshIfRequiredAndReschedule() call is made, e.g. for reasons besides
* Confirms that if a refreshAndRescheduleIfRequired() call is made, e.g. for reasons besides
* scheduled alerts, and the latest time is too old, then an NTP refresh will be attempted.
*/
@Test
public void engineImpl_refreshIfRequiredAndReschedule_failureHandlingAfterLatestIsTooOld() {
public void engineImpl_refreshAndRescheduleIfRequired_failureHandlingAfterLatestIsTooOld() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
@@ -541,7 +563,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect a refresh attempt to have been made.
verify(mMockNtpTrustedTime, times(1)).forceRefresh(mDummyNetwork);
@@ -556,11 +578,11 @@ public class NetworkTimeUpdateServiceTest {
}
/**
* Confirms that if a refreshIfRequiredAndReschedule() call is made and there was a recently
* Confirms that if a refreshAndRescheduleIfRequired() call is made and there was a recently
* failed refresh, then another won't be scheduled too soon.
*/
@Test
public void engineImpl_refreshIfRequiredAndReschedule_minimumRefreshTimeEnforced() {
public void engineImpl_refreshAndRescheduleIfRequired_minimumRefreshTimeEnforced() {
mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS);
int normalPollingIntervalMillis = 7777777;
@@ -574,7 +596,7 @@ public class NetworkTimeUpdateServiceTest {
NtpTrustedTime.TimeResult timeResult = createNtpTimeResult(
mFakeElapsedRealtimeClock.getElapsedRealtimeMillis());
// Simulate an initial call to refreshIfRequiredAndReschedule() prime the "last refresh
// Simulate an initial call to refreshAndRescheduleIfRequired() prime the "last refresh
// attempt" time. A cached time value is available, but it's too old but the refresh
// attempt will fail.
long lastRefreshAttemptElapsedMillis;
@@ -586,7 +608,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect a refresh attempt to have been made.
verify(mMockNtpTrustedTime, times(1)).forceRefresh(mDummyNetwork);
@@ -606,7 +628,7 @@ public class NetworkTimeUpdateServiceTest {
reset(mMockNtpTrustedTime);
}
// Simulate a second call to refreshIfRequiredAndReschedule() very soon after the first, as
// Simulate a second call to refreshAndRescheduleIfRequired() very soon after the first, as
// might happen if there were a network state change.
// The cached time value is available, but it's still too old. Because the last call was so
// recent, no refresh should take place and the next scheduled refresh time should be
@@ -619,7 +641,7 @@ public class NetworkTimeUpdateServiceTest {
RefreshCallbacks mockCallback = mock(RefreshCallbacks.class);
// Trigger the engine's logic.
engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback);
engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback);
// Expect no refresh attempt to have been made: time elapsed isn't enough.
verify(mMockNtpTrustedTime, never()).forceRefresh(any());