From 06249199e4ba6d8747d024139853dfee917a32a2 Mon Sep 17 00:00:00 2001 From: Tsuyoshi Murakami Date: Fri, 26 May 2017 19:20:27 +0900 Subject: [PATCH] Refactor --- .../example/dexopener/MainActivityTest.java | 7 +- .../tmurakami/dexopener/FinalClassTest.java | 7 +- .../dexopener/AndroidClassSource.java | 13 +- .../github/tmurakami/dexopener/IOUtils.java | 22 ++++ .../dexopener/AndroidClassSourceTest.java | 42 +++--- .../dexopener/ClassNameFilterWrapperTest.java | 2 +- .../dexopener/ClassNameReaderTest.java | 32 ++--- .../dexopener/DexClassSourceTest.java | 121 ++++++++++-------- 8 files changed, 143 insertions(+), 103 deletions(-) create mode 100644 dexopener/src/main/java/com/github/tmurakami/dexopener/IOUtils.java diff --git a/dexopener-example/src/androidTest/java/com/example/dexopener/MainActivityTest.java b/dexopener-example/src/androidTest/java/com/example/dexopener/MainActivityTest.java index a45ff50..4d7cba4 100644 --- a/dexopener-example/src/androidTest/java/com/example/dexopener/MainActivityTest.java +++ b/dexopener-example/src/androidTest/java/com/example/dexopener/MainActivityTest.java @@ -21,10 +21,11 @@ public class MainActivityTest implements ActivityLifecycleCallback { @Rule - public final ActivityTestRule rule = new ActivityTestRule<>(MainActivity.class, true, false); + public final ActivityTestRule rule = + new ActivityTestRule<>(MainActivity.class, true, false); @Mock - MainService service; + private MainService service; private MockitoSession session; @@ -48,7 +49,7 @@ public void onActivityLifecycleChanged(Activity activity, Stage stage) { } @Test - public void the_title_of_the_MainActivity_should_be_replaced_by_the_given_mock() throws Exception { + public void the_title_of_MainActivity_should_be_replaced_by_the_given_mock() throws Exception { String s = "test"; given(service.getString(isA(MainActivity.class))).willReturn(s); assertEquals(s, rule.launchActivity(null).getTitle()); diff --git a/dexopener/src/androidTest/java/test/com/github/tmurakami/dexopener/FinalClassTest.java b/dexopener/src/androidTest/java/test/com/github/tmurakami/dexopener/FinalClassTest.java index 8089bc6..f0f1b26 100644 --- a/dexopener/src/androidTest/java/test/com/github/tmurakami/dexopener/FinalClassTest.java +++ b/dexopener/src/androidTest/java/test/com/github/tmurakami/dexopener/FinalClassTest.java @@ -2,16 +2,15 @@ import org.junit.Test; -import java.lang.reflect.Modifier; - +import static java.lang.reflect.Modifier.isFinal; import static org.junit.Assert.assertFalse; public class FinalClassTest { @Test public void DexOpener_should_remove_all_final_modifiers() throws Exception { - assertFalse(Modifier.isFinal(FinalClass.class.getModifiers())); - assertFalse(Modifier.isFinal(FinalClass.class.getDeclaredMethod("doIt").getModifiers())); + assertFalse(isFinal(FinalClass.class.getModifiers())); + assertFalse(isFinal(FinalClass.class.getDeclaredMethod("doIt").getModifiers())); } private static final class FinalClass { diff --git a/dexopener/src/main/java/com/github/tmurakami/dexopener/AndroidClassSource.java b/dexopener/src/main/java/com/github/tmurakami/dexopener/AndroidClassSource.java index 00f5647..4a05653 100644 --- a/dexopener/src/main/java/com/github/tmurakami/dexopener/AndroidClassSource.java +++ b/dexopener/src/main/java/com/github/tmurakami/dexopener/AndroidClassSource.java @@ -7,10 +7,8 @@ import com.github.tmurakami.classinjector.ClassSources; import com.github.tmurakami.dexopener.repackaged.org.ow2.asmdex.ApplicationReader; -import java.io.ByteArrayOutputStream; import java.io.FileInputStream; import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -56,7 +54,7 @@ private ClassSource newDelegate() throws IOException { for (ZipEntry e; (e = in.getNextEntry()) != null; ) { String name = e.getName(); if (name.startsWith("classes") && name.endsWith(".dex")) { - byte[] byteCode = readByteCode(in); + byte[] byteCode = IOUtils.readBytes(in); ApplicationReader ar = new ApplicationReader(ASM4, byteCode); Set classNames = r.readClassNames(ar); if (!classNames.isEmpty()) { @@ -70,13 +68,4 @@ private ClassSource newDelegate() throws IOException { return new ClassSources(sources); } - private static byte[] readByteCode(InputStream in) throws IOException { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - byte[] buffer = new byte[16384]; - for (int l; (l = in.read(buffer)) != -1; ) { - out.write(buffer, 0, l); - } - return out.toByteArray(); - } - } diff --git a/dexopener/src/main/java/com/github/tmurakami/dexopener/IOUtils.java b/dexopener/src/main/java/com/github/tmurakami/dexopener/IOUtils.java new file mode 100644 index 0000000..09da1f8 --- /dev/null +++ b/dexopener/src/main/java/com/github/tmurakami/dexopener/IOUtils.java @@ -0,0 +1,22 @@ +package com.github.tmurakami.dexopener; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; + +final class IOUtils { + + private IOUtils() { + throw new AssertionError("Do not instantiate"); + } + + static byte[] readBytes(InputStream in) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buffer = new byte[16384]; + for (int l; (l = in.read(buffer)) != -1; ) { + out.write(buffer, 0, l); + } + return out.toByteArray(); + } + +} diff --git a/dexopener/src/test/java/com/github/tmurakami/dexopener/AndroidClassSourceTest.java b/dexopener/src/test/java/com/github/tmurakami/dexopener/AndroidClassSourceTest.java index c62cbf7..7b81809 100644 --- a/dexopener/src/test/java/com/github/tmurakami/dexopener/AndroidClassSourceTest.java +++ b/dexopener/src/test/java/com/github/tmurakami/dexopener/AndroidClassSourceTest.java @@ -32,48 +32,60 @@ public class AndroidClassSourceTest { public final TemporaryFolder folder = new TemporaryFolder(); @Mock - ClassNameFilter classNameFilter; + private ClassNameFilter classNameFilter; @Mock - DexClassSourceFactory dexClassSourceFactory; + private DexClassSourceFactory dexClassSourceFactory; @Mock - ClassSource classSource; + private ClassSource classSource; @Mock - ClassFile classFile; + private ClassFile classFile; @Captor - ArgumentCaptor byteCodeCaptor; + private ArgumentCaptor byteCodeCaptor; @Captor - ArgumentCaptor> classNamesCaptor; + private ArgumentCaptor> classNamesCaptor; @SuppressWarnings("TryFinallyCanBeTryWithResources") @Test - public void the_getClassFile_method_should_return_the_ClassFile_with_the_given_name() throws Exception { + public void getClassFile_should_return_the_ClassFile_with_the_given_name() throws Exception { ApplicationWriter aw = new ApplicationWriter(); - aw.visitClass(0, "Lfoo/Bar;", null, "Ljava/lang/Object;", null); + aw.visitClass(0, + "Lfoo/Bar;", + null, + "Ljava/lang/Object;", + null); aw.visitEnd(); - byte[] bytes = aw.toByteArray(); + byte[] byteCode = aw.toByteArray(); File apk = folder.newFile(); ZipOutputStream out = new ZipOutputStream(new FileOutputStream(apk)); try { out.putNextEntry(new ZipEntry("classes.dex")); - out.write(bytes); + out.write(byteCode); } finally { out.close(); } given(classNameFilter.accept("foo.Bar")).willReturn(true); - given(dexClassSourceFactory.newClassSource(byteCodeCaptor.capture(), classNamesCaptor.capture())).willReturn(classSource); + given(dexClassSourceFactory.newClassSource(byteCodeCaptor.capture(), + classNamesCaptor.capture())) + .willReturn(classSource); given(classSource.getClassFile("foo.Bar")).willReturn(classFile); - AndroidClassSource testTarget = new AndroidClassSource(apk.getCanonicalPath(), classNameFilter, dexClassSourceFactory); + AndroidClassSource testTarget = new AndroidClassSource(apk.getCanonicalPath(), + classNameFilter, + dexClassSourceFactory); assertSame(classFile, testTarget.getClassFile("foo.Bar")); - assertArrayEquals(bytes, byteCodeCaptor.getValue()); + assertArrayEquals(byteCode, byteCodeCaptor.getValue()); Set classNames = classNamesCaptor.getValue(); assertEquals(1, classNames.size()); assertEquals("foo.Bar", classNames.iterator().next()); } @Test - public void the_getClassFile_method_should_return_null_if_the_class_name_does_not_pass_the_ClassNameFilter() throws Exception { - assertNull(new AndroidClassSource("", classNameFilter, dexClassSourceFactory).getClassFile("foo.Bar")); + public void getClassFile_should_return_null_if_the_given_name_does_not_pass_through_the_filter() + throws Exception { + AndroidClassSource classSource = new AndroidClassSource("", + classNameFilter, + dexClassSourceFactory); + assertNull(classSource.getClassFile("foo.Bar")); } } diff --git a/dexopener/src/test/java/com/github/tmurakami/dexopener/ClassNameFilterWrapperTest.java b/dexopener/src/test/java/com/github/tmurakami/dexopener/ClassNameFilterWrapperTest.java index 99850b6..42caaa0 100644 --- a/dexopener/src/test/java/com/github/tmurakami/dexopener/ClassNameFilterWrapperTest.java +++ b/dexopener/src/test/java/com/github/tmurakami/dexopener/ClassNameFilterWrapperTest.java @@ -62,7 +62,7 @@ public static Iterable parameters() { } @Test - public void the_accept_method_should_return_a_value_equal_to_the_expected_value() throws Exception { + public void accept_should_return_a_value_equal_to_the_expected_value() throws Exception { assertSame(expected, testTarget.accept(className)); } diff --git a/dexopener/src/test/java/com/github/tmurakami/dexopener/ClassNameReaderTest.java b/dexopener/src/test/java/com/github/tmurakami/dexopener/ClassNameReaderTest.java index d57e772..d70d9f6 100644 --- a/dexopener/src/test/java/com/github/tmurakami/dexopener/ClassNameReaderTest.java +++ b/dexopener/src/test/java/com/github/tmurakami/dexopener/ClassNameReaderTest.java @@ -13,37 +13,37 @@ import static com.github.tmurakami.dexopener.repackaged.org.ow2.asmdex.Opcodes.ASM4; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.mockito.BDDMockito.given; @RunWith(MockitoJUnitRunner.StrictStubs.class) public class ClassNameReaderTest { @InjectMocks - ClassNameReader testTarget; + private ClassNameReader testTarget; @Mock - ClassNameFilter filter; + private ClassNameFilter filter; @Test - public void the_readClassNames_method_should_read_class_names_that_pass_the_ClassNameFilter() throws Exception { + public void readClassNames_should_return_only_class_names_that_passed_through_the_filter() + throws Exception { given(filter.accept("foo.Bar")).willReturn(true); ApplicationWriter aw = new ApplicationWriter(); - aw.visitClass(0, "Lfoo/Bar;", null, "Ljava/lang/Object;", null); + aw.visitClass(0, + "Lfoo/Bar;", + null, + "Ljava/lang/Object;", + null); + aw.visitClass(0, + "Lfoo/bar/Baz;", + null, + "Ljava/lang/Object;", + null); aw.visitEnd(); - byte[] bytes = aw.toByteArray(); - Set classNames = testTarget.readClassNames(new ApplicationReader(ASM4, bytes)); + byte[] byteCode = aw.toByteArray(); + Set classNames = testTarget.readClassNames(new ApplicationReader(ASM4, byteCode)); assertEquals(1, classNames.size()); assertEquals("foo.Bar", classNames.iterator().next()); } - @Test - public void the_readClassNames_method_should_not_read_class_names_that_do_not_pass_the_ClassNameFilter() throws Exception { - ApplicationWriter aw = new ApplicationWriter(); - aw.visitClass(0, "Lfoo/Bar;", null, "Ljava/lang/Object;", null); - aw.visitEnd(); - byte[] bytes = aw.toByteArray(); - assertTrue(testTarget.readClassNames(new ApplicationReader(ASM4, bytes)).isEmpty()); - } - } diff --git a/dexopener/src/test/java/com/github/tmurakami/dexopener/DexClassSourceTest.java b/dexopener/src/test/java/com/github/tmurakami/dexopener/DexClassSourceTest.java index fab2708..8ba50c4 100644 --- a/dexopener/src/test/java/com/github/tmurakami/dexopener/DexClassSourceTest.java +++ b/dexopener/src/test/java/com/github/tmurakami/dexopener/DexClassSourceTest.java @@ -1,7 +1,6 @@ package com.github.tmurakami.dexopener; import com.github.tmurakami.classinjector.ClassFile; -import com.github.tmurakami.dexopener.repackaged.org.ow2.asmdex.ApplicationReader; import com.github.tmurakami.dexopener.repackaged.org.ow2.asmdex.ApplicationWriter; import org.junit.Rule; @@ -12,11 +11,9 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOError; import java.io.IOException; -import java.io.InputStream; import java.util.Arrays; import java.util.Set; import java.util.zip.ZipEntry; @@ -24,7 +21,6 @@ import dalvik.system.DexFile; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.mockito.ArgumentMatchers.argThat; @@ -38,96 +34,117 @@ public class DexClassSourceTest { public final TemporaryFolder folder = new TemporaryFolder(); @Mock - ApplicationReader applicationReader; + private Set classNames; @Mock - Set classNames; + private File cacheDir; @Mock - File cacheDir; + private DexFileLoader dexFileLoader; @Mock - DexFileLoader dexFileLoader; + private DexClassFileFactory classFileFactory; @Mock - DexClassFileFactory classFileFactory; + private DexFile dexFile; @Mock - DexFile dexFile; - @Mock - ClassFile classFile; + private ClassFile classFile; @Test - public void the_getClassFile_method_should_return_the_ClassFile_with_the_given_name() throws Exception { + public void getClassFile_should_return_the_ClassFile_with_the_given_name() throws Exception { final File cacheDir = folder.newFolder(); ApplicationWriter aw = new ApplicationWriter(); - aw.visitClass(0, "Lfoo/Bar;", null, "Ljava/lang/Object;", null); + aw.visitClass(0, + "Lfoo/Bar;", + null, + "Ljava/lang/Object;", + null); aw.visitEnd(); - final byte[] bytes = aw.toByteArray(); + final byte[] byteCode = aw.toByteArray(); given(classNames.contains("foo.Bar")).willReturn(true); given(dexFileLoader.loadDex(argThat(new ArgumentMatcher() { @Override public boolean matches(String argument) { File f = new File(argument); String name = f.getName(); - if (!cacheDir.equals(f.getParentFile()) || !name.startsWith("classes") || !name.endsWith(".zip")) { - return false; - } - ZipFile zipFile = null; - try { - zipFile = new ZipFile(f); - ZipEntry e = zipFile.getEntry("classes.dex"); - assertNotNull(e); - InputStream in = zipFile.getInputStream(e); - ByteArrayOutputStream out = new ByteArrayOutputStream(); - byte[] buffer = new byte[16384]; - for (int l; (l = in.read(buffer)) != -1; ) { - out.write(buffer, 0, l); - } - return Arrays.equals(bytes, out.toByteArray()); - } catch (IOException e) { - throw new IOError(e); - } finally { - if (zipFile != null) { - try { - zipFile.close(); - } catch (IOException ignored) { - } - } - } + return cacheDir.equals(f.getParentFile()) + && name.startsWith("classes") + && name.endsWith(".zip") + && Arrays.equals(byteCode, readByteCode(f)); } }), argThat(new ArgumentMatcher() { @Override public boolean matches(String argument) { File f = new File(argument); String name = f.getName(); - return cacheDir.equals(f.getParentFile()) && name.startsWith("classes") && name.endsWith(".zip.dex"); + return cacheDir.equals(f.getParentFile()) + && name.startsWith("classes") + && name.endsWith(".zip.dex"); } }), eq(0))).willReturn(dexFile); given(classFileFactory.newClassFile("foo.Bar", dexFile)).willReturn(classFile); - assertSame(classFile, new DexClassSource(bytes, classNames, cacheDir, dexFileLoader, classFileFactory).getClassFile("foo.Bar")); + DexClassSource source = new DexClassSource(byteCode, + classNames, + cacheDir, + dexFileLoader, + classFileFactory); + assertSame(classFile, source.getClassFile("foo.Bar")); } @Test - public void the_getClassFile_method_should_return_null_if_the_given_name_does_not_contains_class_names() throws Exception { + public void getClassFile_should_return_null_if_the_given_name_is_not_in_the_list_of_class_names() + throws Exception { ApplicationWriter aw = new ApplicationWriter(); aw.visitEnd(); - byte[] bytes = aw.toByteArray(); - assertNull(new DexClassSource(bytes, classNames, cacheDir, dexFileLoader, classFileFactory).getClassFile("foo.Bar")); + DexClassSource classSource = new DexClassSource(aw.toByteArray(), + classNames, + cacheDir, + dexFileLoader, + classFileFactory); + assertNull(classSource.getClassFile("foo.Bar")); } @Test(expected = IllegalStateException.class) - public void the_getClassFile_method_should_throw_an_IllegalStateException_if_the_class_with_the_given_name_cannot_be_found() throws Exception { + public void getClassFile_should_throw_IllegalStateException_if_the_class_for_the_given_name_cannot_be_found() throws Exception { ApplicationWriter aw = new ApplicationWriter(); aw.visitEnd(); - byte[] bytes = aw.toByteArray(); given(classNames.contains("foo.Bar")).willReturn(true); - assertNull(new DexClassSource(bytes, classNames, cacheDir, dexFileLoader, classFileFactory).getClassFile("foo.Bar")); + new DexClassSource(aw.toByteArray(), + classNames, + cacheDir, + dexFileLoader, + classFileFactory).getClassFile("foo.Bar"); } @Test(expected = IllegalStateException.class) - public void the_getClassFile_method_should_throw_an_IllegalArgumentException_if_the_cache_dir_cannot_be_created() throws Exception { + public void getClassFile_should_throw_IllegalStateException_if_the_cache_dir_cannot_be_created() throws Exception { ApplicationWriter aw = new ApplicationWriter(); - aw.visitClass(0, "Lfoo/Bar;", null, "Ljava/lang/Object;", null); + aw.visitClass(0, + "Lfoo/Bar;", + null, + "Ljava/lang/Object;", + null); aw.visitEnd(); - byte[] bytes = aw.toByteArray(); given(classNames.contains("foo.Bar")).willReturn(true); - new DexClassSource(bytes, classNames, cacheDir, dexFileLoader, classFileFactory).getClassFile("foo.Bar"); + new DexClassSource(aw.toByteArray(), + classNames, + cacheDir, + dexFileLoader, + classFileFactory).getClassFile("foo.Bar"); + } + + private static byte[] readByteCode(File zip) { + ZipFile zipFile = null; + try { + zipFile = new ZipFile(zip); + ZipEntry e = zipFile.getEntry("classes.dex"); + return IOUtils.readBytes(zipFile.getInputStream(e)); + } catch (IOException e) { + throw new IOError(e); + } finally { + if (zipFile != null) { + try { + zipFile.close(); + } catch (IOException ignored) { + } + } + } } }