refactor: update after review

Author: Plamen5kov

lib/constants.ts

@@ -24,6 +24,7 @@ export const SRC_DIR = "src";
 export const MAIN_DIR = "main";
 export const ASSETS_DIR = "assets";
 export const MANIFEST_FILE_NAME = "AndroidManifest.xml";
+export const INCLUDE_GRADLE_NAME = "include.gradle";
 export const BUILD_DIR = "build";
 export const OUTPUTS_DIR = "outputs";
 export const APK_DIR = "apk";

lib/definitions/android-plugin-migrator.d.ts

@@ -1,5 +1,8 @@
 
-interface IBuildOptions {
+interface IBuildOptions extends IAndroidBuildOptions{
+}
+
+interface IAndroidBuildOptions {
     platformsAndroidDirPath: string,
     pluginName: string,
     aarOutputDir: string,

lib/services/android-plugin-build-service.ts

@@ -1,16 +1,16 @@
 import * as path from "path";
-import * as shell from "shelljs";
+import { MANIFEST_FILE_NAME, INCLUDE_GRADLE_NAME, ASSETS_DIR, RESOURCES_DIR } from "../constants";
 import { Builder, parseString } from "xml2js";
+import { ILogger } from "log4js";
 
 export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 
 	constructor(private $fs: IFileSystem,
 		private $childProcess: IChildProcess,
 		private $hostInfo: IHostInfo,
-		private $androidToolsInfo: IAndroidToolsInfo) { }
+		private $androidToolsInfo: IAndroidToolsInfo,
+		private $logger: ILogger) { }
 
-	private static ANDROID_MANIFEST_XML = "AndroidManifest.xml";
-	private static INCLUDE_GRADLE = "include.gradle";
 	private static MANIFEST_ROOT = {
 		$: {
 			"xmlns:android": "http://schemas.android.com/apk/res/android"
@@ -19,7 +19,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 	private static ANDROID_PLUGIN_GRADLE_TEMPLATE = "../../vendor/gradle-plugin";
 
 	private getAndroidSourceDirectories(source: string): Array<string> {
-		const directories = ["res", "java", "assets", "jniLibs"];
+		const directories = [RESOURCES_DIR, "java", ASSETS_DIR, "jniLibs"];
 		const resultArr: Array<string> = [];
 
 		this.$fs.enumerateFilesInDirectorySync(source, (file, stat) => {
@@ -33,7 +33,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 	}
 
 	private getManifest(platformsDir: string): string {
-		const manifest = path.join(platformsDir, AndroidPluginBuildService.ANDROID_MANIFEST_XML);
+		const manifest = path.join(platformsDir, MANIFEST_FILE_NAME);
 		return this.$fs.exists(manifest) ? manifest : null;
 	}
 
@@ -42,10 +42,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 	}
 
 	private async updateManifestContent(oldManifestContent: string, defaultPackageName: string): Promise<string> {
-		const content = oldManifestContent;
-		let newManifestContent;
-
-		let xml: any = await this.getXml(content);
+		let xml: any = await this.getXml(oldManifestContent);
 
 		let packageName = defaultPackageName;
 		// if the manifest file is full-featured and declares settings inside the manifest scope
@@ -67,17 +64,16 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 		newManifest.manifest["$"]["package"] = packageName;
 
 		const xmlBuilder = new Builder();
-		newManifestContent = xmlBuilder.buildObject(newManifest);
+		const newManifestContent = xmlBuilder.buildObject(newManifest);
 
 		return newManifestContent;
 	}
 
 	private createManifestContent(packageName: string): string {
-		let newManifestContent;
 		const newManifest: any = { manifest: AndroidPluginBuildService.MANIFEST_ROOT };
 		newManifest.manifest["$"]["package"] = packageName;
 		const xmlBuilder: any = new Builder();
-		newManifestContent = xmlBuilder.buildObject(newManifest);
+		const newManifestContent = xmlBuilder.buildObject(newManifest);
 
 		return newManifestContent;
 	}
@@ -96,10 +92,6 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 		return promise;
 	}
 
-	private copyRecursive(source: string, destination: string): void {
-		shell.cp("-R", source, destination);
-	}
-
 	private getIncludeGradleCompileDependenciesScope(includeGradleFileContent: string): Array<string> {
 		const indexOfDependenciesScope = includeGradleFileContent.indexOf("dependencies");
 		const result: Array<string> = [];
@@ -125,23 +117,23 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 	private getScope(scopeName: string, content: string): string {
 		const indexOfScopeName = content.indexOf(scopeName);
 		let result = "";
-		const OPENING_BRACKET = "{";
-		const CLOSING_BRACKET = "}";
+		const openingBracket = "{";
+		const closingBracket = "}";
 		let openBrackets = 0;
 		let foundFirstBracket = false;
 
 		let i = indexOfScopeName;
 		while (i < content.length) {
 			const currCharacter = content[i];
-			if (currCharacter === OPENING_BRACKET) {
+			if (currCharacter === openingBracket) {
 				if (openBrackets === 0) {
 					foundFirstBracket = true;
 				}
 
 				openBrackets++;
 			}
 
-			if (currCharacter === CLOSING_BRACKET) {
+			if (currCharacter === closingBracket) {
 				openBrackets--;
 			}
 
@@ -180,7 +172,6 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 		// find manifest file
 		//// prepare manifest file content
 		const manifestFilePath = this.getManifest(options.platformsAndroidDirPath);
-		let updatedManifestContent;
 		let shouldBuildAar = false;
 
 		// look for AndroidManifest.xml
@@ -197,6 +188,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 
 		// if a manifest OR/AND resource files are present - write files, build plugin
 		if (shouldBuildAar) {
+			let updatedManifestContent;
 			this.$fs.ensureDirectoryExists(newPluginMainSrcDir);
 
 			if (manifestFilePath) {
@@ -215,7 +207,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 			}
 
 			// write the AndroidManifest in the temp-dir/plugin-name/src/main
-			const pathToNewAndroidManifest = path.join(newPluginMainSrcDir, AndroidPluginBuildService.ANDROID_MANIFEST_XML);
+			const pathToNewAndroidManifest = path.join(newPluginMainSrcDir, MANIFEST_FILE_NAME);
 			try {
 				this.$fs.writeFile(pathToNewAndroidManifest, updatedManifestContent);
 			} catch (e) {
@@ -231,14 +223,14 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 				const destination = path.join(newPluginMainSrcDir, dirName);
 				this.$fs.ensureDirectoryExists(destination);
 
-				this.copyRecursive(path.join(dir, "*"), destination);
+				this.$fs.copyFile(path.join(dir, "*"), destination);
 			}
 
 			// copy the preconfigured gradle android library project template to the temporary android library
-			this.copyRecursive(path.join(path.resolve(path.join(__dirname, AndroidPluginBuildService.ANDROID_PLUGIN_GRADLE_TEMPLATE), "*")), newPluginDir);
+			this.$fs.copyFile(path.join(path.resolve(path.join(__dirname, AndroidPluginBuildService.ANDROID_PLUGIN_GRADLE_TEMPLATE), "*")), newPluginDir);
 
 			// sometimes the AndroidManifest.xml or certain resources in /res may have a compile dependency to a library referenced in include.gradle. Make sure to compile the plugin with a compile dependency to those libraries
-			const includeGradlePath = path.join(options.platformsAndroidDirPath, "include.gradle");
+			const includeGradlePath = path.join(options.platformsAndroidDirPath, INCLUDE_GRADLE_NAME);
 			if (this.$fs.exists(includeGradlePath)) {
 				const includeGradleContent = this.$fs.readText(includeGradlePath);
 				const repositoriesAndDependenciesScopes = this.getIncludeGradleCompileDependenciesScope(includeGradleContent);
@@ -280,7 +272,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 			if (this.$fs.exists(pathToBuiltAar)) {
 				try {
 					if (options.aarOutputDir) {
-						this.copyRecursive(pathToBuiltAar, path.join(options.aarOutputDir, `${shortPluginName}.aar`));
+						this.$fs.copyFile(pathToBuiltAar, path.join(options.aarOutputDir, `${shortPluginName}.aar`));
 					}
 				} catch (e) {
 					throw new Error(`Failed to copy built aar to destination. ${e.message}`);
@@ -302,7 +294,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 	public migrateIncludeGradle(options: IBuildOptions): void {
 		this.validatePlatformsAndroidDirPathOption(options);
 
-		const includeGradleFilePath = path.join(options.platformsAndroidDirPath, AndroidPluginBuildService.INCLUDE_GRADLE);
+		const includeGradleFilePath = path.join(options.platformsAndroidDirPath, INCLUDE_GRADLE_NAME);
 
 		if (this.$fs.exists(includeGradleFilePath)) {
 			let includeGradleFileContent: string;
@@ -330,11 +322,11 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
 		}
 
 		if (!options.pluginName) {
-			console.log("No plugin name provided, defaulting to 'myPlugin'.");
+			this.$logger.info("No plugin name provided, defaulting to 'myPlugin'.");
 		}
 
 		if (!options.aarOutputDir) {
-			console.log("No aarOutputDir provided, defaulting to the build outputs directory of the plugin");
+			this.$logger.info("No aarOutputDir provided, defaulting to the build outputs directory of the plugin");
 		}
 
 		if (!options.tempPluginDirPath) {

lib/services/android-project-service.ts

@@ -453,8 +453,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
 			return path.resolve(pathToPlatformsAndroid, item.directory);
 		});
 
-		for (const dependencyKey in productionDependencies) {
-			const dependency = productionDependencies[dependencyKey];
+		for (const dependency of productionDependencies) {
 			const jsonContent = this.$fs.readJson(path.join(dependency, constants.PACKAGE_JSON_FILE_NAME));
 			const isPlugin = !!jsonContent.nativescript;
 			const pluginName = jsonContent.name;

lib/services/platform-service.ts

@@ -203,7 +203,7 @@ export class PlatformService extends EventEmitter implements IPlatformService {
 
 	public async preparePlatform(platformInfo: IPreparePlatformInfo): Promise<boolean> {
 		const changesInfo = await this.getChangesInfo(platformInfo);
-		const shouldPrepare = changesInfo.nativeChanged || await this.shouldPrepare({ platformInfo, changesInfo });
+		const shouldPrepare = await this.shouldPrepare({ platformInfo, changesInfo });
 
 		if (shouldPrepare) {
 			// Always clear up the app directory in platforms if `--bundle` value has changed in between builds or is passed in general

package.json

@@ -80,7 +80,7 @@
     "winreg": "0.0.17",
     "ws": "2.2.0",
     "xcode": "https://github.com/NativeScript/node-xcode/archive/1.4.0.tar.gz",
-    "xml2js": "^0.4.19",
+    "xml2js": "0.4.19",
     "xmldom": "0.1.21",
     "xmlhttprequest": "https://github.com/telerik/node-XMLHttpRequest/tarball/master",
     "yargs": "6.0.0",

test/ios-entitlements-service.ts

@@ -47,7 +47,7 @@ describe("IOSEntitlements Service Tests", () => {
 		injector = createTestInjector();
 
 		platformsData = injector.resolve("platformsData");
-		projectData = $injector.resolve<IProjectData>("projectData");
+		projectData = injector.resolve<IProjectData>("projectData");
 		projectData.projectName = 'testApp';
 
 		projectData.platformsDir = temp.mkdirSync("platformsDir");

test/services/android-plugin-build-service.ts

@@ -110,7 +110,7 @@ dependencies {
 			await androidBuildPluginService.buildAar(config);
 			const hasGeneratedAar = fs.exists(path.join(tempFolder, expectedAarName));
 
-			assert.equal(hasGeneratedAar, true);
+			assert.isTrue(hasGeneratedAar);
 		});
 
 		it('if android manifest is missing', async () => {
@@ -126,7 +126,7 @@ dependencies {
 			await androidBuildPluginService.buildAar(config);
 			const hasGeneratedAar = fs.exists(path.join(tempFolder, expectedAarName));
 
-			assert.equal(hasGeneratedAar, true);
+			assert.isTrue(hasGeneratedAar);
 		});
 
 		it('if there is only an android manifest file', async () => {
@@ -142,7 +142,7 @@ dependencies {
 			await androidBuildPluginService.buildAar(config);
 			const hasGeneratedAar = fs.exists(path.join(tempFolder, expectedAarName));
 
-			assert.equal(hasGeneratedAar, true);
+			assert.isTrue(hasGeneratedAar);
 		});
 	});