Merge "Switch to streaming data for time zone update" am: f093d4e13e am: 4fee968fa8 am: dbe6beb0bb (9eea4b20) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/timezone/FileDescriptorHelper.java

deleted100644 → 0

+0 −30

Original line number	Diff line number	Diff line
		/*
		* Copyright (C) 2017 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 com.android.server.timezone;

		import android.os.ParcelFileDescriptor;

		import java.io.IOException;

		/**
		* An easy-to-mock interface around use of {@link ParcelFileDescriptor} for use by
		* {@link RulesManagerService}.
		*/
		interface FileDescriptorHelper {

		byte[] readFully(ParcelFileDescriptor parcelFileDescriptor) throws IOException;
		}

services/core/java/com/android/server/timezone/RulesManagerService.java

+56 −35

Original line number	Diff line number	Diff line
		@@ -20,8 +20,8 @@ import com.android.internal.annotations.VisibleForTesting;
		import com.android.server.SystemService;
		import com.android.timezone.distro.DistroException;
		import com.android.timezone.distro.DistroVersion;
		import com.android.timezone.distro.TimeZoneDistro;
		import com.android.timezone.distro.StagedDistroOperation;
		import com.android.timezone.distro.TimeZoneDistro;

		import android.app.timezone.Callback;
		import android.app.timezone.DistroFormatVersion;
		@@ -36,7 +36,9 @@ import android.os.RemoteException;
		import android.util.Slog;

		import java.io.File;
		import java.io.FileInputStream;
		import java.io.IOException;
		import java.io.InputStream;
		import java.util.Arrays;
		import java.util.concurrent.Executor;
		import java.util.concurrent.atomic.AtomicBoolean;
		@@ -83,26 +85,22 @@ public final class RulesManagerService extends IRulesManager.Stub {
		private final PackageTracker mPackageTracker;
		private final Executor mExecutor;
		private final TimeZoneDistroInstaller mInstaller;
		private final FileDescriptorHelper mFileDescriptorHelper;

		private static RulesManagerService create(Context context) {
		RulesManagerServiceHelperImpl helper = new RulesManagerServiceHelperImpl(context);
		return new RulesManagerService(
		helper /* permissionHelper */,
		helper /* executor */,
		helper /* fileDescriptorHelper */,
		PackageTracker.create(context),
		new TimeZoneDistroInstaller(TAG, SYSTEM_TZ_DATA_FILE, TZ_DATA_DIR));
		}

		// A constructor that can be used by tests to supply mocked / faked dependencies.
		RulesManagerService(PermissionHelper permissionHelper,
		Executor executor,
		FileDescriptorHelper fileDescriptorHelper, PackageTracker packageTracker,
		Executor executor, PackageTracker packageTracker,
		TimeZoneDistroInstaller timeZoneDistroInstaller) {
		mPermissionHelper = permissionHelper;
		mExecutor = executor;
		mFileDescriptorHelper = fileDescriptorHelper;
		mPackageTracker = packageTracker;
		mInstaller = timeZoneDistroInstaller;
		}
		@@ -177,17 +175,21 @@ public final class RulesManagerService extends IRulesManager.Stub {
		}

		@Override
		public int requestInstall(
		ParcelFileDescriptor timeZoneDistro, byte[] checkTokenBytes, ICallback callback) {
		public int requestInstall(ParcelFileDescriptor distroParcelFileDescriptor,
		byte[] checkTokenBytes, ICallback callback) {

		boolean closeParcelFileDescriptorOnExit = true;
		try {
		mPermissionHelper.enforceCallerHasPermission(REQUIRED_UPDATER_PERMISSION);

		CheckToken checkToken = null;
		if (checkTokenBytes != null) {
		checkToken = createCheckTokenOrThrow(checkTokenBytes);
		}

		synchronized (this) {
		if (timeZoneDistro == null) {
		throw new NullPointerException("timeZoneDistro == null");
		if (distroParcelFileDescriptor == null) {
		throw new NullPointerException("distroParcelFileDescriptor == null");
		}
		if (callback == null) {
		throw new NullPointerException("observer == null");
		@@ -198,34 +200,53 @@ public final class RulesManagerService extends IRulesManager.Stub {
		mOperationInProgress.set(true);

		// Execute the install asynchronously.
		mExecutor.execute(new InstallRunnable(timeZoneDistro, checkToken, callback));
		mExecutor.execute(
		new InstallRunnable(distroParcelFileDescriptor, checkToken, callback));

		// The InstallRunnable now owns the ParcelFileDescriptor, so it will close it after
		// it executes (and we do not have to).
		closeParcelFileDescriptorOnExit = false;

		return RulesManager.SUCCESS;
		}
		} finally {
		// We should close() the local ParcelFileDescriptor we were passed if it hasn't been
		// passed to another thread to handle.
		if (distroParcelFileDescriptor != null && closeParcelFileDescriptorOnExit) {
		try {
		distroParcelFileDescriptor.close();
		} catch (IOException e) {
		Slog.w(TAG, "Failed to close distroParcelFileDescriptor", e);
		}
		}
		}
		}

		private class InstallRunnable implements Runnable {

		private final ParcelFileDescriptor mTimeZoneDistro;
		private final ParcelFileDescriptor mDistroParcelFileDescriptor;
		private final CheckToken mCheckToken;
		private final ICallback mCallback;

		InstallRunnable(
		ParcelFileDescriptor timeZoneDistro, CheckToken checkToken, ICallback callback) {
		mTimeZoneDistro = timeZoneDistro;
		InstallRunnable(ParcelFileDescriptor distroParcelFileDescriptor, CheckToken checkToken,
		ICallback callback) {
		mDistroParcelFileDescriptor = distroParcelFileDescriptor;
		mCheckToken = checkToken;
		mCallback = callback;
		}

		@Override
		public void run() {
		boolean success = false;
		// Adopt the ParcelFileDescriptor into this try-with-resources so it is closed
		// when we are done.
		boolean success = false;
		try {
		byte[] distroBytes =
		RulesManagerService.this.mFileDescriptorHelper.readFully(mTimeZoneDistro);
		TimeZoneDistro distro = new TimeZoneDistro(distroBytes);
		try (ParcelFileDescriptor pfd = mDistroParcelFileDescriptor) {
		// The ParcelFileDescriptor owns the underlying FileDescriptor and we'll close
		// it at the end of the try-with-resources.
		final boolean isFdOwner = false;
		InputStream is = new FileInputStream(pfd.getFileDescriptor(), isFdOwner);

		TimeZoneDistro distro = new TimeZoneDistro(is);
		int installerResult = mInstaller.stageInstallWithErrorCode(distro);
		int resultCode = mapInstallerResultToApiCode(installerResult);
		sendFinishedStatus(mCallback, resultCode);

services/core/java/com/android/server/timezone/RulesManagerServiceHelperImpl.java

+1 −11

Original line number	Diff line number	Diff line
		@@ -27,8 +27,7 @@ import libcore.io.Streams;
		/**
		* A single class that implements multiple helper interfaces for use by {@link RulesManagerService}.
		*/
		final class RulesManagerServiceHelperImpl
		implements PermissionHelper, Executor, FileDescriptorHelper {
		final class RulesManagerServiceHelperImpl implements PermissionHelper, Executor {

		private final Context mContext;

		@@ -47,13 +46,4 @@ final class RulesManagerServiceHelperImpl
		// TODO Is there a better way?
		new Thread(runnable).start();
		}

		@Override
		public byte[] readFully(ParcelFileDescriptor parcelFileDescriptor) throws IOException {
		try (ParcelFileDescriptor pfd = parcelFileDescriptor) {
		// Read bytes
		FileInputStream in = new FileInputStream(pfd.getFileDescriptor(), false /* isOwner */);
		return Streams.readFully(in);
		}
		}
		}

services/tests/servicestests/src/com/android/server/timezone/RulesManagerServiceTest.java

+49 −73

Original line number	Diff line number	Diff line
		@@ -30,6 +30,8 @@ import android.app.timezone.RulesManager;
		import android.app.timezone.RulesState;
		import android.os.ParcelFileDescriptor;

		import java.io.File;
		import java.io.FileOutputStream;
		import java.io.IOException;
		import java.util.concurrent.Executor;
		import javax.annotation.Nullable;
		@@ -61,7 +63,6 @@ public class RulesManagerServiceTest {

		private FakeExecutor mFakeExecutor;
		private PermissionHelper mMockPermissionHelper;
		private FileDescriptorHelper mMockFileDescriptorHelper;
		private PackageTracker mMockPackageTracker;
		private TimeZoneDistroInstaller mMockTimeZoneDistroInstaller;

		@@ -69,7 +70,6 @@ public class RulesManagerServiceTest {
		public void setUp() {
		mFakeExecutor = new FakeExecutor();

		mMockFileDescriptorHelper = mock(FileDescriptorHelper.class);
		mMockPackageTracker = mock(PackageTracker.class);
		mMockPermissionHelper = mock(PermissionHelper.class);
		mMockTimeZoneDistroInstaller = mock(TimeZoneDistroInstaller.class);
		@@ -77,7 +77,6 @@ public class RulesManagerServiceTest {
		mRulesManagerService = new RulesManagerService(
		mMockPermissionHelper,
		mFakeExecutor,
		mMockFileDescriptorHelper,
		mMockPackageTracker,
		mMockTimeZoneDistroInstaller);
		}
		@@ -273,9 +272,8 @@ public class RulesManagerServiceTest {
		revision);
		configureInstalledDistroVersion(installedDistroVersion);

		byte[] expectedContent = createArbitraryBytes(1000);
		ParcelFileDescriptor parcelFileDescriptor = createFakeParcelFileDescriptor();
		configureParcelFileDescriptorReadSuccess(parcelFileDescriptor, expectedContent);
		ParcelFileDescriptor parcelFileDescriptor =
		createParcelFileDescriptor(createArbitraryBytes(1000));

		// Start an async operation so there is one in progress. The mFakeExecutor won't actually
		// execute it.
		@@ -298,24 +296,27 @@ public class RulesManagerServiceTest {
		public void requestInstall_operationInProgress() throws Exception {
		configureCallerHasPermission();

		byte[] expectedContent = createArbitraryBytes(1000);
		ParcelFileDescriptor parcelFileDescriptor = createFakeParcelFileDescriptor();
		configureParcelFileDescriptorReadSuccess(parcelFileDescriptor, expectedContent);
		ParcelFileDescriptor parcelFileDescriptor1 =
		createParcelFileDescriptor(createArbitraryBytes(1000));

		byte[] tokenBytes = createArbitraryTokenBytes();
		ICallback callback = new StubbedCallback();

		// First request should succeed.
		assertEquals(RulesManager.SUCCESS,
		mRulesManagerService.requestInstall(parcelFileDescriptor, tokenBytes, callback));
		mRulesManagerService.requestInstall(parcelFileDescriptor1, tokenBytes, callback));

		// Something async should be enqueued. Clear it but do not execute it so we can detect the
		// second request does nothing.
		mFakeExecutor.getAndResetLastCommand();

		// Second request should fail.
		ParcelFileDescriptor parcelFileDescriptor2 =
		createParcelFileDescriptor(createArbitraryBytes(1000));
		assertEquals(RulesManager.ERROR_OPERATION_IN_PROGRESS,
		mRulesManagerService.requestInstall(parcelFileDescriptor, tokenBytes, callback));
		mRulesManagerService.requestInstall(parcelFileDescriptor2, tokenBytes, callback));

		assertClosed(parcelFileDescriptor2);

		// Assert nothing async was enqueued.
		mFakeExecutor.assertNothingQueued();
		@@ -327,9 +328,8 @@ public class RulesManagerServiceTest {
		public void requestInstall_badToken() throws Exception {
		configureCallerHasPermission();

		byte[] expectedContent = createArbitraryBytes(1000);
		ParcelFileDescriptor parcelFileDescriptor = createFakeParcelFileDescriptor();
		configureParcelFileDescriptorReadSuccess(parcelFileDescriptor, expectedContent);
		ParcelFileDescriptor parcelFileDescriptor =
		createParcelFileDescriptor(createArbitraryBytes(1000));

		byte[] badTokenBytes = new byte[2];
		ICallback callback = new StubbedCallback();
		@@ -340,6 +340,8 @@ public class RulesManagerServiceTest {
		} catch (IllegalArgumentException expected) {
		}

		assertClosed(parcelFileDescriptor);

		// Assert nothing async was enqueued.
		mFakeExecutor.assertNothingQueued();
		verifyNoInstallerCallsMade();
		@@ -369,7 +371,8 @@ public class RulesManagerServiceTest {
		public void requestInstall_nullCallback() throws Exception {
		configureCallerHasPermission();

		ParcelFileDescriptor parcelFileDescriptor = createFakeParcelFileDescriptor();
		ParcelFileDescriptor parcelFileDescriptor =
		createParcelFileDescriptor(createArbitraryBytes(1000));
		byte[] tokenBytes = createArbitraryTokenBytes();
		ICallback callback = null;

		@@ -378,6 +381,8 @@ public class RulesManagerServiceTest {
		fail();
		} catch (NullPointerException expected) {}

		assertClosed(parcelFileDescriptor);

		// Assert nothing async was enqueued.
		mFakeExecutor.assertNothingQueued();
		verifyNoInstallerCallsMade();
		@@ -388,9 +393,8 @@ public class RulesManagerServiceTest {
		public void requestInstall_asyncSuccess() throws Exception {
		configureCallerHasPermission();

		ParcelFileDescriptor parcelFileDescriptor = createFakeParcelFileDescriptor();
		byte[] expectedContent = createArbitraryBytes(1000);
		configureParcelFileDescriptorReadSuccess(parcelFileDescriptor, expectedContent);
		ParcelFileDescriptor parcelFileDescriptor =
		createParcelFileDescriptor(createArbitraryBytes(1000));

		CheckToken token = createArbitraryToken();
		byte[] tokenBytes = token.toByteArray();
		@@ -406,14 +410,14 @@ public class RulesManagerServiceTest {
		verifyNoInstallerCallsMade();
		verifyNoPackageTrackerCallsMade();

		TimeZoneDistro expectedDistro = new TimeZoneDistro(expectedContent);

		// Set up the installer.
		configureStageInstallExpectation(TimeZoneDistroInstaller.INSTALL_SUCCESS);

		// Simulate the async execution.
		mFakeExecutor.simulateAsyncExecutionOfLastCommand();

		assertClosed(parcelFileDescriptor);

		// Verify the expected calls were made to other components.
		verifyStageInstallCalled();
		verifyPackageTrackerCalled(token, true /* success */);
		@@ -426,9 +430,8 @@ public class RulesManagerServiceTest {
		public void requestInstall_nullTokenBytes() throws Exception {
		configureCallerHasPermission();

		ParcelFileDescriptor parcelFileDescriptor = createFakeParcelFileDescriptor();
		byte[] expectedContent = createArbitraryBytes(1000);
		configureParcelFileDescriptorReadSuccess(parcelFileDescriptor, expectedContent);
		ParcelFileDescriptor parcelFileDescriptor =
		createParcelFileDescriptor(createArbitraryBytes(1000));

		TestCallback callback = new TestCallback();

		@@ -447,6 +450,8 @@ public class RulesManagerServiceTest {
		// Simulate the async execution.
		mFakeExecutor.simulateAsyncExecutionOfLastCommand();

		assertClosed(parcelFileDescriptor);

		// Verify the expected calls were made to other components.
		verifyStageInstallCalled();
		verifyPackageTrackerCalled(null /* expectedToken /, true / success */);
		@@ -459,9 +464,8 @@ public class RulesManagerServiceTest {
		public void requestInstall_asyncInstallFail() throws Exception {
		configureCallerHasPermission();

		byte[] expectedContent = createArbitraryBytes(1000);
		ParcelFileDescriptor parcelFileDescriptor = createFakeParcelFileDescriptor();
		configureParcelFileDescriptorReadSuccess(parcelFileDescriptor, expectedContent);
		ParcelFileDescriptor parcelFileDescriptor =
		createParcelFileDescriptor(createArbitraryBytes(1000));

		CheckToken token = createArbitraryToken();
		byte[] tokenBytes = token.toByteArray();
		@@ -482,6 +486,8 @@ public class RulesManagerServiceTest {
		// Simulate the async execution.
		mFakeExecutor.simulateAsyncExecutionOfLastCommand();

		assertClosed(parcelFileDescriptor);

		// Verify the expected calls were made to other components.
		verifyStageInstallCalled();

		@@ -493,38 +499,6 @@ public class RulesManagerServiceTest {
		callback.assertResultReceived(Callback.ERROR_INSTALL_VALIDATION_ERROR);
		}

		@Test
		public void requestInstall_asyncParcelFileDescriptorReadFail() throws Exception {
		configureCallerHasPermission();

		ParcelFileDescriptor parcelFileDescriptor = createFakeParcelFileDescriptor();
		configureParcelFileDescriptorReadFailure(parcelFileDescriptor);

		CheckToken token = createArbitraryToken();
		byte[] tokenBytes = token.toByteArray();

		TestCallback callback = new TestCallback();

		// Request the install.
		assertEquals(RulesManager.SUCCESS,
		mRulesManagerService.requestInstall(parcelFileDescriptor, tokenBytes, callback));

		// Simulate the async execution.
		mFakeExecutor.simulateAsyncExecutionOfLastCommand();

		// Verify nothing else happened.
		verifyNoInstallerCallsMade();

		// A failure to read the ParcelFileDescriptor is treated as a failure. It might be the
		// result of a file system error. This is a fairly arbitrary choice.
		verifyPackageTrackerCalled(token, false /* success */);

		verifyNoPackageTrackerCallsMade();

		// Check the callback was received.
		callback.assertResultReceived(Callback.ERROR_UNKNOWN_FAILURE);
		}

		@Test
		public void requestUninstall_operationInProgress() throws Exception {
		configureCallerHasPermission();
		@@ -773,17 +747,6 @@ public class RulesManagerServiceTest {
		.enforceCallerHasPermission(REQUIRED_UPDATER_PERMISSION);
		}

		private void configureParcelFileDescriptorReadSuccess(ParcelFileDescriptor parcelFileDescriptor,
		byte[] content) throws Exception {
		when(mMockFileDescriptorHelper.readFully(parcelFileDescriptor)).thenReturn(content);
		}

		private void configureParcelFileDescriptorReadFailure(ParcelFileDescriptor parcelFileDescriptor)
		throws Exception {
		when(mMockFileDescriptorHelper.readFully(parcelFileDescriptor))
		.thenThrow(new IOException("Simulated failure"));
		}

		private void configureStageInstallExpectation(int resultCode)
		throws Exception {
		when(mMockTimeZoneDistroInstaller.stageInstallWithErrorCode(any(TimeZoneDistro.class)))
		@@ -827,10 +790,6 @@ public class RulesManagerServiceTest {
		return new CheckToken(1, new PackageVersions(1, 1));
		}

		private ParcelFileDescriptor createFakeParcelFileDescriptor() {
		return new ParcelFileDescriptor((ParcelFileDescriptor) null);
		}

		private void configureDeviceSystemRulesVersion(String systemRulesVersion) throws Exception {
		when(mMockTimeZoneDistroInstaller.getSystemRulesVersion()).thenReturn(systemRulesVersion);
		}
		@@ -870,6 +829,10 @@ public class RulesManagerServiceTest {
		.thenThrow(new IOException("Simulated failure"));
		}

		private static void assertClosed(ParcelFileDescriptor parcelFileDescriptor) {
		assertFalse(parcelFileDescriptor.getFileDescriptor().valid());
		}

		private static class FakeExecutor implements Executor {

		private Runnable mLastCommand;
		@@ -926,4 +889,17 @@ public class RulesManagerServiceTest {
		fail("Unexpected call");
		}
		}

		private static ParcelFileDescriptor createParcelFileDescriptor(byte[] bytes)
		throws IOException {
		File file = File.createTempFile("pfd", null);
		try (FileOutputStream fos = new FileOutputStream(file)) {
		fos.write(bytes);
		}
		ParcelFileDescriptor pfd =
		ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY);
		// This should now be safe to delete. The ParcelFileDescriptor has an open fd.
		file.delete();
		return pfd;
		}
		}