From b0361ffa3e25ab3e234a5ccaa5587b7dca88de76 Mon Sep 17 00:00:00 2001 From: Zhomart Mukhamejanov Date: Fri, 18 May 2018 10:22:13 -0700 Subject: updater_config: make utils/PayloadSpecs non-static PayloadSpecs has complicated methods, which makes hard to test other dependent classes. Making it non-static allows mocking it, which makes testing other classes easier. Test: manually Test: using JUnit4 Change-Id: I818dc59c6bb0e7d131439d0d41f88d5cd9a451b4 Signed-off-by: Zhomart Mukhamejanov --- .../systemupdatersample/services/PrepareStreamingService.java | 10 ++++++---- .../example/android/systemupdatersample/ui/MainActivity.java | 3 ++- .../example/android/systemupdatersample/util/PayloadSpecs.java | 8 +++----- .../android/systemupdatersample/util/PayloadSpecsTest.java | 9 ++++++--- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/updater_sample/src/com/example/android/systemupdatersample/services/PrepareStreamingService.java b/updater_sample/src/com/example/android/systemupdatersample/services/PrepareStreamingService.java index 222bb0a58..ac6e223e3 100644 --- a/updater_sample/src/com/example/android/systemupdatersample/services/PrepareStreamingService.java +++ b/updater_sample/src/com/example/android/systemupdatersample/services/PrepareStreamingService.java @@ -116,6 +116,8 @@ public class PrepareStreamingService extends IntentService { PackageFiles.PAYLOAD_PROPERTIES_FILE_NAME ); + private final PayloadSpecs mPayloadSpecs = new PayloadSpecs(); + @Override protected void onHandleIntent(Intent intent) { Log.d(TAG, "On handle intent is called"); @@ -137,7 +139,7 @@ public class PrepareStreamingService extends IntentService { * 3. Checks OTA package compatibility with the device. * 4. Constructs {@link PayloadSpec} for streaming update. */ - private static PayloadSpec execute(UpdateConfig config) + private PayloadSpec execute(UpdateConfig config) throws IOException, PreparationFailedException { downloadPreStreamingFiles(config, OTA_PACKAGE_DIR); @@ -164,7 +166,7 @@ public class PrepareStreamingService extends IntentService { } } - return PayloadSpecs.forStreaming(config.getUrl(), + return mPayloadSpecs.forStreaming(config.getUrl(), payloadBinary.get().getOffset(), payloadBinary.get().getSize(), Paths.get(OTA_PACKAGE_DIR, PAYLOAD_PROPERTIES_FILE_NAME).toFile()); @@ -176,7 +178,7 @@ public class PrepareStreamingService extends IntentService { * in directory {@code dir}. * @throws IOException when can't download a file */ - private static void downloadPreStreamingFiles(UpdateConfig config, String dir) + private void downloadPreStreamingFiles(UpdateConfig config, String dir) throws IOException { Log.d(TAG, "Deleting existing files from " + dir); for (String file : PRE_STREAMING_FILES_SET) { @@ -200,7 +202,7 @@ public class PrepareStreamingService extends IntentService { * @param file physical location of {@link PackageFiles#COMPATIBILITY_ZIP_FILE_NAME} * @return true if OTA package is compatible with this device */ - private static boolean verifyPackageCompatibility(File file) { + private boolean verifyPackageCompatibility(File file) { try { return RecoverySystem.verifyPackageCompatibility(file); } catch (IOException e) { diff --git a/updater_sample/src/com/example/android/systemupdatersample/ui/MainActivity.java b/updater_sample/src/com/example/android/systemupdatersample/ui/MainActivity.java index c5a7f9556..9bab1319d 100644 --- a/updater_sample/src/com/example/android/systemupdatersample/ui/MainActivity.java +++ b/updater_sample/src/com/example/android/systemupdatersample/ui/MainActivity.java @@ -77,6 +77,7 @@ public class MainActivity extends Activity { new AtomicInteger(UpdateEngine.UpdateStatusConstants.IDLE); private PayloadSpec mLastPayloadSpec; private AtomicBoolean mManualSwitchSlotRequired = new AtomicBoolean(true); + private final PayloadSpecs mPayloadSpecs = new PayloadSpecs(); /** * Listen to {@code update_engine} events. @@ -338,7 +339,7 @@ public class MainActivity extends Activity { if (config.getInstallType() == UpdateConfig.AB_INSTALL_TYPE_NON_STREAMING) { PayloadSpec payload; try { - payload = PayloadSpecs.forNonStreaming(config.getUpdatePackageFile()); + payload = mPayloadSpecs.forNonStreaming(config.getUpdatePackageFile()); } catch (IOException e) { Log.e(TAG, "Error creating payload spec", e); Toast.makeText(this, "Error creating payload spec", Toast.LENGTH_LONG) diff --git a/updater_sample/src/com/example/android/systemupdatersample/util/PayloadSpecs.java b/updater_sample/src/com/example/android/systemupdatersample/util/PayloadSpecs.java index 4db448a31..b98b97c37 100644 --- a/updater_sample/src/com/example/android/systemupdatersample/util/PayloadSpecs.java +++ b/updater_sample/src/com/example/android/systemupdatersample/util/PayloadSpecs.java @@ -43,7 +43,7 @@ public final class PayloadSpecs { * zip file. So we enumerate the entries to identify the offset of the payload file. * http://developer.android.com/reference/java/util/zip/ZipFile.html#entries() */ - public static PayloadSpec forNonStreaming(File packageFile) throws IOException { + public PayloadSpec forNonStreaming(File packageFile) throws IOException { boolean payloadFound = false; long payloadOffset = 0; long payloadSize = 0; @@ -100,7 +100,7 @@ public final class PayloadSpecs { /** * Creates a {@link PayloadSpec} for streaming update. */ - public static PayloadSpec forStreaming(String updateUrl, + public PayloadSpec forStreaming(String updateUrl, long offset, long size, File propertiesFile) throws IOException { @@ -115,7 +115,7 @@ public final class PayloadSpecs { /** * Converts an {@link PayloadSpec} to a string. */ - public static String toString(PayloadSpec payloadSpec) { + public String specToString(PayloadSpec payloadSpec) { return ""; } - private PayloadSpecs() {} - } diff --git a/updater_sample/tests/src/com/example/android/systemupdatersample/util/PayloadSpecsTest.java b/updater_sample/tests/src/com/example/android/systemupdatersample/util/PayloadSpecsTest.java index d9e54652f..3ba84c116 100644 --- a/updater_sample/tests/src/com/example/android/systemupdatersample/util/PayloadSpecsTest.java +++ b/updater_sample/tests/src/com/example/android/systemupdatersample/util/PayloadSpecsTest.java @@ -55,6 +55,8 @@ public class PayloadSpecsTest { private Context mTargetContext; private Context mTestContext; + private PayloadSpecs mPayloadSpecs; + @Rule public final ExpectedException thrown = ExpectedException.none(); @@ -64,6 +66,7 @@ public class PayloadSpecsTest { mTestContext = InstrumentationRegistry.getContext(); mTestDir = mTargetContext.getFilesDir(); + mPayloadSpecs = new PayloadSpecs(); } @Test @@ -75,7 +78,7 @@ public class PayloadSpecsTest { java.nio.file.Files.deleteIfExists(packageFile.toPath()); java.nio.file.Files.copy(mTestContext.getResources().openRawResource(R.raw.ota_002_package), packageFile.toPath()); - PayloadSpec spec = PayloadSpecs.forNonStreaming(packageFile); + PayloadSpec spec = mPayloadSpecs.forNonStreaming(packageFile); assertEquals("correct url", "file://" + packageFile.getAbsolutePath(), spec.getUrl()); assertEquals("correct payload offset", @@ -90,7 +93,7 @@ public class PayloadSpecsTest { @Test public void forNonStreaming_IOException() throws Exception { thrown.expect(IOException.class); - PayloadSpecs.forNonStreaming(new File("/fake/news.zip")); + mPayloadSpecs.forNonStreaming(new File("/fake/news.zip")); } @Test @@ -100,7 +103,7 @@ public class PayloadSpecsTest { long size = 200; File propertiesFile = createMockPropertiesFile(); - PayloadSpec spec = PayloadSpecs.forStreaming(url, offset, size, propertiesFile); + PayloadSpec spec = mPayloadSpecs.forStreaming(url, offset, size, propertiesFile); assertEquals("same url", url, spec.getUrl()); assertEquals("same offset", offset, spec.getOffset()); assertEquals("same size", size, spec.getSize()); -- cgit v1.2.3