fix: Skip preparation of plugins native files in case they are not changed

lib/constants.ts

@@ -219,3 +219,4 @@ export class AddPlaformErrors {
 }
 
 export const PLUGIN_BUILD_DATA_FILENAME = "plugin-data.json";
+export const PLUGINS_BUILD_DATA_FILENAME = ".ns-plugins-build-data.json";

lib/services/plugins-service.ts

@@ -36,6 +36,7 @@ export class PluginsService implements IPluginsService {
 		private $options: IOptions,
 		private $logger: ILogger,
 		private $errors: IErrors,
+		private $filesHashService: IFilesHashService,
 		private $injector: IInjector) { }
 
 	public async add(plugin: string, projectData: IProjectData): Promise<void> {
@@ -107,7 +108,7 @@ export class PluginsService implements IPluginsService {
 		return await platformData.platformProjectService.validatePlugins(projectData);
 	}
 
-	public async prepare(dependencyData: IDependencyData, platform: string, projectData: IProjectData,  projectFilesConfig: IProjectFilesConfig): Promise<void> {
+	public async prepare(dependencyData: IDependencyData, platform: string, projectData: IProjectData, projectFilesConfig: IProjectFilesConfig): Promise<void> {
 		platform = platform.toLowerCase();
 		const platformData = this.$platformsData.getPlatformData(platform, projectData);
 		const pluginData = this.convertToPluginData(dependencyData, projectData.projectDir);
@@ -141,9 +142,26 @@ export class PluginsService implements IPluginsService {
 
 	public async preparePluginNativeCode(pluginData: IPluginData, platform: string, projectData: IProjectData): Promise<void> {
 		const platformData = this.$platformsData.getPlatformData(platform, projectData);
-
 		pluginData.pluginPlatformsFolderPath = (_platform: string) => path.join(pluginData.fullPath, "platforms", _platform);
-		await platformData.platformProjectService.preparePluginNativeCode(pluginData, projectData);
+
+		const pluginPlatformsFolderPath = pluginData.pluginPlatformsFolderPath(platform);
+		if (this.$fs.exists(pluginPlatformsFolderPath)) {
+			const pathToPluginsBuildFile = path.join(platformData.projectRoot, constants.PLUGINS_BUILD_DATA_FILENAME);
+
+			const allPluginsNativeHashes = this.getAllPluginsNativeHashes(pathToPluginsBuildFile);
+			const oldPluginNativeHashes = allPluginsNativeHashes[pluginData.name];
+			const currentPluginNativeHashes = await this.getPluginNativeHashes(pluginPlatformsFolderPath);
+
+			if (!oldPluginNativeHashes || this.$filesHashService.hasChangesInShasums(oldPluginNativeHashes, currentPluginNativeHashes)) {
+				await platformData.platformProjectService.preparePluginNativeCode(pluginData, projectData);
+				this.setPluginNativeHashes({
+					pathToPluginsBuildFile,
+					pluginData,
+					currentPluginNativeHashes,
+					allPluginsNativeHashes
+				});
+			}
+		}
 	}
 
 	public async ensureAllDependenciesAreInstalled(projectData: IProjectData): Promise<void> {
@@ -307,6 +325,30 @@ export class PluginsService implements IPluginsService {
 
 		return isValid;
 	}
+
+	private async getPluginNativeHashes(pluginPlatformsDir: string): Promise<IStringDictionary> {
+		let data: IStringDictionary = {};
+		if (this.$fs.exists(pluginPlatformsDir)) {
+			const pluginNativeDataFiles = this.$fs.enumerateFilesInDirectorySync(pluginPlatformsDir);
+			data = await this.$filesHashService.generateHashes(pluginNativeDataFiles);
+		}
+
+		return data;
+	}
+
+	private getAllPluginsNativeHashes(pathToPluginsBuildFile: string): IDictionary<IStringDictionary> {
+		let data: IDictionary<IStringDictionary> = {};
+		if (this.$fs.exists(pathToPluginsBuildFile)) {
+			data = this.$fs.readJson(pathToPluginsBuildFile);
+		}
+
+		return data;
+	}
+
+	private setPluginNativeHashes(opts: { pathToPluginsBuildFile: string, pluginData: IPluginData, currentPluginNativeHashes: IStringDictionary, allPluginsNativeHashes: IDictionary<IStringDictionary> }): void {
+		opts.allPluginsNativeHashes[opts.pluginData.name] = opts.currentPluginNativeHashes;
+		this.$fs.writeJson(opts.pathToPluginsBuildFile, opts.allPluginsNativeHashes);
+	}
 }
 
 $injector.register("pluginsService", PluginsService);

lib/services/project-changes-service.ts

@@ -54,6 +54,7 @@ export class ProjectChangesService implements IProjectChangesService {
 		private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
 		private $fs: IFileSystem,
 		private $filesHashService: IFilesHashService,
+		private $logger: ILogger,
 		private $injector: IInjector) {
 	}
 
@@ -83,9 +84,13 @@ export class ProjectChangesService implements IProjectChangesService {
 				projectData,
 				this.fileChangeRequiresBuild);
 
+			this.$logger.trace(`Set nativeChanged to ${this._changesInfo.nativeChanged}. skipModulesNativeCheck is: ${projectChangesOptions.skipModulesNativeCheck}`);
+
 			if (this._newFiles > 0 || this._changesInfo.nativeChanged) {
+				this.$logger.trace(`Setting modulesChanged to true, newFiles: ${this._newFiles}, nativeChanged: ${this._changesInfo.nativeChanged}`);
 				this._changesInfo.modulesChanged = true;
 			}
+
 			if (platform === this.$devicePlatformsConstants.iOS.toLowerCase()) {
 				this._changesInfo.configChanged = this.filesChanged([path.join(platformResourcesDir, platformData.configurationFileName),
 				path.join(platformResourcesDir, "LaunchScreen.storyboard"),
@@ -97,12 +102,15 @@ export class ProjectChangesService implements IProjectChangesService {
 					path.join(platformResourcesDir, APP_GRADLE_FILE_NAME)
 				]);
 			}
+
+			this.$logger.trace(`Set value of configChanged to ${this._changesInfo.configChanged}`);
 		}
 
 		const projectService = platformData.platformProjectService;
 		await projectService.checkForChanges(this._changesInfo, projectChangesOptions, projectData);
 
 		if (projectChangesOptions.bundle !== this._prepareInfo.bundle || projectChangesOptions.release !== this._prepareInfo.release) {
+			this.$logger.trace(`Setting all setting to true. Current options are: `, projectChangesOptions, " old prepare info is: ", this._prepareInfo);
 			this._changesInfo.appFilesChanged = true;
 			this._changesInfo.appResourcesChanged = true;
 			this._changesInfo.modulesChanged = true;
@@ -112,9 +120,11 @@ export class ProjectChangesService implements IProjectChangesService {
 			this._prepareInfo.bundle = projectChangesOptions.bundle;
 		}
 		if (this._changesInfo.packageChanged) {
+			this.$logger.trace("Set modulesChanged to true as packageChanged is true");
 			this._changesInfo.modulesChanged = true;
 		}
 		if (this._changesInfo.modulesChanged || this._changesInfo.appResourcesChanged) {
+			this.$logger.trace(`Set configChanged to true, current value of moduleChanged is: ${this._changesInfo.modulesChanged}, appResourcesChanged is: ${this._changesInfo.appResourcesChanged}`);
 			this._changesInfo.configChanged = true;
 		}
 		if (this._changesInfo.hasChanges) {
@@ -129,6 +139,7 @@ export class ProjectChangesService implements IProjectChangesService {
 
 		this._changesInfo.nativePlatformStatus = this._prepareInfo.nativePlatformStatus;
 
+		this.$logger.trace("checkForChanges returns", this._changesInfo);
 		return this._changesInfo;
 	}
 
@@ -234,14 +245,16 @@ export class ProjectChangesService implements IProjectChangesService {
 	}
 
 	private containsNewerFiles(dir: string, skipDir: string, projectData: IProjectData, processFunc?: (filePath: string, projectData: IProjectData) => boolean): boolean {
-
 		const dirName = path.basename(dir);
+		this.$logger.trace(`containsNewerFiles will check ${dir}`);
 		if (_.startsWith(dirName, '.')) {
+			this.$logger.trace(`containsNewerFiles returns false for ${dir} as its name starts with dot (.) .`);
 			return false;
 		}
 
 		const dirFileStat = this.$fs.getFsStats(dir);
 		if (this.isFileModified(dirFileStat, dir)) {
+			this.$logger.trace(`containsNewerFiles returns true for ${dir} as the dir itself has been modified.`);
 			return true;
 		}
 
@@ -256,24 +269,30 @@ export class ProjectChangesService implements IProjectChangesService {
 			const changed = this.isFileModified(fileStats, filePath);
 
 			if (changed) {
+				this.$logger.trace(`File ${filePath} has been changed.`);
 				if (processFunc) {
 					this._newFiles++;
+					this.$logger.trace(`Incremented the newFiles counter. Current value is ${this._newFiles}`);
 					const filePathRelative = path.relative(projectData.projectDir, filePath);
 					if (processFunc.call(this, filePathRelative, projectData)) {
+						this.$logger.trace(`containsNewerFiles returns true for ${dir}. The modified file is ${filePath}`);
 						return true;
 					}
 				} else {
+					this.$logger.trace(`containsNewerFiles returns true for ${dir}. The modified file is ${filePath}`);
 					return true;
 				}
 			}
 
 			if (fileStats.isDirectory()) {
 				if (this.containsNewerFiles(filePath, skipDir, projectData, processFunc)) {
+					this.$logger.trace(`containsNewerFiles returns true for ${dir}.`);
 					return true;
 				}
 			}
-
 		}
+
+		this.$logger.trace(`containsNewerFiles returns false for ${dir}.`);
 		return false;
 	}
 
@@ -291,7 +310,7 @@ export class ProjectChangesService implements IProjectChangesService {
 	}
 
 	private fileChangeRequiresBuild(file: string, projectData: IProjectData) {
-		if (path.basename(file) === "package.json") {
+		if (path.basename(file) === PACKAGE_JSON_FILE_NAME) {
 			return true;
 		}
 		const projectDir = projectData.projectDir;
@@ -302,7 +321,7 @@ export class ProjectChangesService implements IProjectChangesService {
 			let filePath = file;
 			while (filePath !== NODE_MODULES_FOLDER_NAME) {
 				filePath = path.dirname(filePath);
-				const fullFilePath = path.join(projectDir, path.join(filePath, "package.json"));
+				const fullFilePath = path.join(projectDir, path.join(filePath, PACKAGE_JSON_FILE_NAME));
 				if (this.$fs.exists(fullFilePath)) {
 					const json = this.$fs.readJson(fullFilePath);
 					if (json["nativescript"] && _.startsWith(file, path.join(filePath, "platforms"))) {

test/ios-project-service.ts

@@ -126,6 +126,10 @@ function createTestInjector(projectPath: string, projectName: string, xcode?: IX
 		on: () => ({})
 	});
 	testInjector.register("emulatorHelper", {});
+	testInjector.register("filesHashService", {
+		hasChangesInShasums: (oldPluginNativeHashes: IStringDictionary, currentPluginNativeHashes: IStringDictionary) => true,
+		generateHashes: async (files: string[]): Promise<IStringDictionary> => ({})
+	});
 
 	return testInjector;
 }

test/plugins-service.ts

@@ -34,6 +34,7 @@ import { SettingsService } from "../lib/common/test/unit-tests/stubs";
 import StaticConfigLib = require("../lib/config");
 import * as path from "path";
 import * as temp from "temp";
+import { PLUGINS_BUILD_DATA_FILENAME } from '../lib/constants';
 temp.track();
 
 let isErrorThrown = false;
@@ -119,6 +120,10 @@ function createTestInjector() {
 	testInjector.register("androidResourcesMigrationService", stubs.AndroidResourcesMigrationServiceStub);
 
 	testInjector.register("platformEnvironmentRequirements", {});
+	testInjector.register("filesHashService", {
+		hasChangesInShasums: (oldPluginNativeHashes: IStringDictionary, currentPluginNativeHashes: IStringDictionary) => true,
+		generateHashes: async (files: string[]): Promise<IStringDictionary> => ({})
+	});
 	return testInjector;
 }
 
@@ -541,4 +546,90 @@ describe("Plugins service", () => {
 			await pluginsService.prepare(pluginJsonData, "android", projectData, {});
 		});
 	});
+
+	describe("preparePluginNativeCode", () => {
+		const setupTest = (opts: { hasChangesInShasums?: boolean, newPluginHashes?: IStringDictionary, buildDataFileExists?: boolean, hasPluginPlatformsDir?: boolean }): any => {
+			const testData: any = {
+				pluginsService: null,
+				isPreparePluginNativeCodeCalled: false,
+				dataPassedToWriteJson: null
+			};
+
+			const unitTestsInjector = new Yok();
+			unitTestsInjector.register("platformsData", {
+				getPlatformData: (platform: string, projectData: IProjectData) => ({
+					projectRoot: "projectRoot",
+					platformProjectService: {
+						preparePluginNativeCode: async (pluginData: IPluginData, projData: IProjectData) => {
+							testData.isPreparePluginNativeCodeCalled = true;
+						}
+					}
+				})
+			});
+
+			const pluginHashes = opts.newPluginHashes || { "file1": "hash1" };
+			const pluginData: IPluginData = <any>{
+				fullPath: "plugin_full_path",
+				name: "plugin_name"
+			};
+
+			unitTestsInjector.register("filesHashService", {
+				hasChangesInShasums: (oldPluginNativeHashes: IStringDictionary, currentPluginNativeHashes: IStringDictionary) => !!opts.hasChangesInShasums,
+				generateHashes: async (files: string[]): Promise<IStringDictionary> => pluginHashes
+			});
+
+			unitTestsInjector.register("fs", {
+				exists: (file: string) => {
+					if (file.indexOf(PLUGINS_BUILD_DATA_FILENAME) !== -1) {
+						return !!opts.buildDataFileExists;
+					}
+
+					if (file.indexOf("platforms") !== -1) {
+						return !!opts.hasPluginPlatformsDir;
+					}
+
+					return true;
+				},
+				readJson: (file: string) => ({
+					[pluginData.name]: pluginHashes
+				}),
+				writeJson: (file: string, json: any) => { testData.dataPassedToWriteJson = json; },
+				enumerateFilesInDirectorySync: (): string[] => ["some_file"]
+			});
+
+			unitTestsInjector.register("npm", {});
+			unitTestsInjector.register("options", {});
+			unitTestsInjector.register("logger", {});
+			unitTestsInjector.register("errors", {});
+			unitTestsInjector.register("injector", unitTestsInjector);
+
+			const pluginsService: PluginsService = unitTestsInjector.resolve(PluginsService);
+			testData.pluginsService = pluginsService;
+			testData.pluginData = pluginData;
+			return testData;
+		};
+
+		const platform = "platform";
+		const projectData: IProjectData = <any>{};
+
+		it("does not prepare the files when plugin does not have platforms dir", async () => {
+			const testData = setupTest({ hasPluginPlatformsDir: false });
+			await testData.pluginsService.preparePluginNativeCode(testData.pluginData, platform, projectData);
+			assert.isFalse(testData.isPreparePluginNativeCodeCalled);
+		});
+
+		it("prepares the files when plugin has platforms dir and has not been built before", async () => {
+			const newPluginHashes = { "file": "hash" };
+			const testData = setupTest({ newPluginHashes, hasPluginPlatformsDir: true });
+			await testData.pluginsService.preparePluginNativeCode(testData.pluginData, platform, projectData);
+			assert.isTrue(testData.isPreparePluginNativeCodeCalled);
+			assert.deepEqual(testData.dataPassedToWriteJson, { [testData.pluginData.name]: newPluginHashes });
+		});
+
+		it("does not prepare the files when plugin has platforms dir and files have not changed since then", async () => {
+			const testData = setupTest({ hasChangesInShasums: false, buildDataFileExists: true, hasPluginPlatformsDir: true });
+			await testData.pluginsService.preparePluginNativeCode(testData.pluginData, platform, projectData);
+			assert.isFalse(testData.isPreparePluginNativeCodeCalled);
+		});
+	});
 });

test/project-changes-service.ts

@@ -6,7 +6,7 @@ import { PlatformsData } from "../lib/platforms-data";
 import { ProjectChangesService } from "../lib/services/project-changes-service";
 import * as Constants from "../lib/constants";
 import { FileSystem } from "../lib/common/file-system";
-import { HooksServiceStub } from "./stubs";
+import { HooksServiceStub, LoggerStub } from "./stubs";
 
 // start tracking temporary folders/files
 temp.track();
@@ -34,9 +34,7 @@ class ProjectChangesServiceTest extends BaseServiceTest {
 		this.injector.register("filesHashService", {
 			generateHashes: () => Promise.resolve({})
 		});
-		this.injector.register("logger", {
-			warn: () => ({})
-		});
+		this.injector.register("logger", LoggerStub);
 		this.injector.register("hooksService", HooksServiceStub);
 
 		const fs = this.injector.resolve<IFileSystem>("fs");