Skip to content

Commit d8e1dfb

Browse files
author
Fatme
authored
Merge pull request #3768 from NativeScript/vladimirov/checkForChanges-logging
fix: Skip preparation of plugins native files in case they are not changed
2 parents a397430 + d159572 commit d8e1dfb

6 files changed

+166
-11
lines changed

‎lib/constants.ts

+1
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,4 @@ export class AddPlaformErrors {
219219
}
220220

221221
export const PLUGIN_BUILD_DATA_FILENAME = "plugin-data.json";
222+
export const PLUGINS_BUILD_DATA_FILENAME = ".ns-plugins-build-data.json";

‎lib/services/plugins-service.ts

+45-3
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export class PluginsService implements IPluginsService {
3636
private $options: IOptions,
3737
private $logger: ILogger,
3838
private $errors: IErrors,
39+
private $filesHashService: IFilesHashService,
3940
private $injector: IInjector) { }
4041

4142
public async add(plugin: string, projectData: IProjectData): Promise<void> {
@@ -107,7 +108,7 @@ export class PluginsService implements IPluginsService {
107108
return await platformData.platformProjectService.validatePlugins(projectData);
108109
}
109110

110-
public async prepare(dependencyData: IDependencyData, platform: string, projectData: IProjectData, projectFilesConfig: IProjectFilesConfig): Promise<void> {
111+
public async prepare(dependencyData: IDependencyData, platform: string, projectData: IProjectData, projectFilesConfig: IProjectFilesConfig): Promise<void> {
111112
platform = platform.toLowerCase();
112113
const platformData = this.$platformsData.getPlatformData(platform, projectData);
113114
const pluginData = this.convertToPluginData(dependencyData, projectData.projectDir);
@@ -141,9 +142,26 @@ export class PluginsService implements IPluginsService {
141142

142143
public async preparePluginNativeCode(pluginData: IPluginData, platform: string, projectData: IProjectData): Promise<void> {
143144
const platformData = this.$platformsData.getPlatformData(platform, projectData);
144-
145145
pluginData.pluginPlatformsFolderPath = (_platform: string) => path.join(pluginData.fullPath, "platforms", _platform);
146-
await platformData.platformProjectService.preparePluginNativeCode(pluginData, projectData);
146+
147+
const pluginPlatformsFolderPath = pluginData.pluginPlatformsFolderPath(platform);
148+
if (this.$fs.exists(pluginPlatformsFolderPath)) {
149+
const pathToPluginsBuildFile = path.join(platformData.projectRoot, constants.PLUGINS_BUILD_DATA_FILENAME);
150+
151+
const allPluginsNativeHashes = this.getAllPluginsNativeHashes(pathToPluginsBuildFile);
152+
const oldPluginNativeHashes = allPluginsNativeHashes[pluginData.name];
153+
const currentPluginNativeHashes = await this.getPluginNativeHashes(pluginPlatformsFolderPath);
154+
155+
if (!oldPluginNativeHashes || this.$filesHashService.hasChangesInShasums(oldPluginNativeHashes, currentPluginNativeHashes)) {
156+
await platformData.platformProjectService.preparePluginNativeCode(pluginData, projectData);
157+
this.setPluginNativeHashes({
158+
pathToPluginsBuildFile,
159+
pluginData,
160+
currentPluginNativeHashes,
161+
allPluginsNativeHashes
162+
});
163+
}
164+
}
147165
}
148166

149167
public async ensureAllDependenciesAreInstalled(projectData: IProjectData): Promise<void> {
@@ -307,6 +325,30 @@ export class PluginsService implements IPluginsService {
307325

308326
return isValid;
309327
}
328+
329+
private async getPluginNativeHashes(pluginPlatformsDir: string): Promise<IStringDictionary> {
330+
let data: IStringDictionary = {};
331+
if (this.$fs.exists(pluginPlatformsDir)) {
332+
const pluginNativeDataFiles = this.$fs.enumerateFilesInDirectorySync(pluginPlatformsDir);
333+
data = await this.$filesHashService.generateHashes(pluginNativeDataFiles);
334+
}
335+
336+
return data;
337+
}
338+
339+
private getAllPluginsNativeHashes(pathToPluginsBuildFile: string): IDictionary<IStringDictionary> {
340+
let data: IDictionary<IStringDictionary> = {};
341+
if (this.$fs.exists(pathToPluginsBuildFile)) {
342+
data = this.$fs.readJson(pathToPluginsBuildFile);
343+
}
344+
345+
return data;
346+
}
347+
348+
private setPluginNativeHashes(opts: { pathToPluginsBuildFile: string, pluginData: IPluginData, currentPluginNativeHashes: IStringDictionary, allPluginsNativeHashes: IDictionary<IStringDictionary> }): void {
349+
opts.allPluginsNativeHashes[opts.pluginData.name] = opts.currentPluginNativeHashes;
350+
this.$fs.writeJson(opts.pathToPluginsBuildFile, opts.allPluginsNativeHashes);
351+
}
310352
}
311353

312354
$injector.register("pluginsService", PluginsService);

‎lib/services/project-changes-service.ts

+23-4
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export class ProjectChangesService implements IProjectChangesService {
5454
private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
5555
private $fs: IFileSystem,
5656
private $filesHashService: IFilesHashService,
57+
private $logger: ILogger,
5758
private $injector: IInjector) {
5859
}
5960

@@ -83,9 +84,13 @@ export class ProjectChangesService implements IProjectChangesService {
8384
projectData,
8485
this.fileChangeRequiresBuild);
8586

87+
this.$logger.trace(`Set nativeChanged to ${this._changesInfo.nativeChanged}. skipModulesNativeCheck is: ${projectChangesOptions.skipModulesNativeCheck}`);
88+
8689
if (this._newFiles > 0 || this._changesInfo.nativeChanged) {
90+
this.$logger.trace(`Setting modulesChanged to true, newFiles: ${this._newFiles}, nativeChanged: ${this._changesInfo.nativeChanged}`);
8791
this._changesInfo.modulesChanged = true;
8892
}
93+
8994
if (platform === this.$devicePlatformsConstants.iOS.toLowerCase()) {
9095
this._changesInfo.configChanged = this.filesChanged([path.join(platformResourcesDir, platformData.configurationFileName),
9196
path.join(platformResourcesDir, "LaunchScreen.storyboard"),
@@ -97,12 +102,15 @@ export class ProjectChangesService implements IProjectChangesService {
97102
path.join(platformResourcesDir, APP_GRADLE_FILE_NAME)
98103
]);
99104
}
105+
106+
this.$logger.trace(`Set value of configChanged to ${this._changesInfo.configChanged}`);
100107
}
101108

102109
const projectService = platformData.platformProjectService;
103110
await projectService.checkForChanges(this._changesInfo, projectChangesOptions, projectData);
104111

105112
if (projectChangesOptions.bundle !== this._prepareInfo.bundle || projectChangesOptions.release !== this._prepareInfo.release) {
113+
this.$logger.trace(`Setting all setting to true. Current options are: `, projectChangesOptions, " old prepare info is: ", this._prepareInfo);
106114
this._changesInfo.appFilesChanged = true;
107115
this._changesInfo.appResourcesChanged = true;
108116
this._changesInfo.modulesChanged = true;
@@ -112,9 +120,11 @@ export class ProjectChangesService implements IProjectChangesService {
112120
this._prepareInfo.bundle = projectChangesOptions.bundle;
113121
}
114122
if (this._changesInfo.packageChanged) {
123+
this.$logger.trace("Set modulesChanged to true as packageChanged is true");
115124
this._changesInfo.modulesChanged = true;
116125
}
117126
if (this._changesInfo.modulesChanged || this._changesInfo.appResourcesChanged) {
127+
this.$logger.trace(`Set configChanged to true, current value of moduleChanged is: ${this._changesInfo.modulesChanged}, appResourcesChanged is: ${this._changesInfo.appResourcesChanged}`);
118128
this._changesInfo.configChanged = true;
119129
}
120130
if (this._changesInfo.hasChanges) {
@@ -129,6 +139,7 @@ export class ProjectChangesService implements IProjectChangesService {
129139

130140
this._changesInfo.nativePlatformStatus = this._prepareInfo.nativePlatformStatus;
131141

142+
this.$logger.trace("checkForChanges returns", this._changesInfo);
132143
return this._changesInfo;
133144
}
134145

@@ -234,14 +245,16 @@ export class ProjectChangesService implements IProjectChangesService {
234245
}
235246

236247
private containsNewerFiles(dir: string, skipDir: string, projectData: IProjectData, processFunc?: (filePath: string, projectData: IProjectData) => boolean): boolean {
237-
238248
const dirName = path.basename(dir);
249+
this.$logger.trace(`containsNewerFiles will check ${dir}`);
239250
if (_.startsWith(dirName, '.')) {
251+
this.$logger.trace(`containsNewerFiles returns false for ${dir} as its name starts with dot (.) .`);
240252
return false;
241253
}
242254

243255
const dirFileStat = this.$fs.getFsStats(dir);
244256
if (this.isFileModified(dirFileStat, dir)) {
257+
this.$logger.trace(`containsNewerFiles returns true for ${dir} as the dir itself has been modified.`);
245258
return true;
246259
}
247260

@@ -256,24 +269,30 @@ export class ProjectChangesService implements IProjectChangesService {
256269
const changed = this.isFileModified(fileStats, filePath);
257270

258271
if (changed) {
272+
this.$logger.trace(`File ${filePath} has been changed.`);
259273
if (processFunc) {
260274
this._newFiles++;
275+
this.$logger.trace(`Incremented the newFiles counter. Current value is ${this._newFiles}`);
261276
const filePathRelative = path.relative(projectData.projectDir, filePath);
262277
if (processFunc.call(this, filePathRelative, projectData)) {
278+
this.$logger.trace(`containsNewerFiles returns true for ${dir}. The modified file is ${filePath}`);
263279
return true;
264280
}
265281
} else {
282+
this.$logger.trace(`containsNewerFiles returns true for ${dir}. The modified file is ${filePath}`);
266283
return true;
267284
}
268285
}
269286

270287
if (fileStats.isDirectory()) {
271288
if (this.containsNewerFiles(filePath, skipDir, projectData, processFunc)) {
289+
this.$logger.trace(`containsNewerFiles returns true for ${dir}.`);
272290
return true;
273291
}
274292
}
275-
276293
}
294+
295+
this.$logger.trace(`containsNewerFiles returns false for ${dir}.`);
277296
return false;
278297
}
279298

@@ -291,7 +310,7 @@ export class ProjectChangesService implements IProjectChangesService {
291310
}
292311

293312
private fileChangeRequiresBuild(file: string, projectData: IProjectData) {
294-
if (path.basename(file) === "package.json") {
313+
if (path.basename(file) === PACKAGE_JSON_FILE_NAME) {
295314
return true;
296315
}
297316
const projectDir = projectData.projectDir;
@@ -302,7 +321,7 @@ export class ProjectChangesService implements IProjectChangesService {
302321
let filePath = file;
303322
while (filePath !== NODE_MODULES_FOLDER_NAME) {
304323
filePath = path.dirname(filePath);
305-
const fullFilePath = path.join(projectDir, path.join(filePath, "package.json"));
324+
const fullFilePath = path.join(projectDir, path.join(filePath, PACKAGE_JSON_FILE_NAME));
306325
if (this.$fs.exists(fullFilePath)) {
307326
const json = this.$fs.readJson(fullFilePath);
308327
if (json["nativescript"] && _.startsWith(file, path.join(filePath, "platforms"))) {

‎test/ios-project-service.ts

+4
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@ function createTestInjector(projectPath: string, projectName: string, xcode?: IX
126126
on: () => ({})
127127
});
128128
testInjector.register("emulatorHelper", {});
129+
testInjector.register("filesHashService", {
130+
hasChangesInShasums: (oldPluginNativeHashes: IStringDictionary, currentPluginNativeHashes: IStringDictionary) => true,
131+
generateHashes: async (files: string[]): Promise<IStringDictionary> => ({})
132+
});
129133

130134
return testInjector;
131135
}

‎test/plugins-service.ts

+91
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { SettingsService } from "../lib/common/test/unit-tests/stubs";
3434
import StaticConfigLib = require("../lib/config");
3535
import * as path from "path";
3636
import * as temp from "temp";
37+
import { PLUGINS_BUILD_DATA_FILENAME } from '../lib/constants';
3738
temp.track();
3839

3940
let isErrorThrown = false;
@@ -119,6 +120,10 @@ function createTestInjector() {
119120
testInjector.register("androidResourcesMigrationService", stubs.AndroidResourcesMigrationServiceStub);
120121

121122
testInjector.register("platformEnvironmentRequirements", {});
123+
testInjector.register("filesHashService", {
124+
hasChangesInShasums: (oldPluginNativeHashes: IStringDictionary, currentPluginNativeHashes: IStringDictionary) => true,
125+
generateHashes: async (files: string[]): Promise<IStringDictionary> => ({})
126+
});
122127
return testInjector;
123128
}
124129

@@ -541,4 +546,90 @@ describe("Plugins service", () => {
541546
await pluginsService.prepare(pluginJsonData, "android", projectData, {});
542547
});
543548
});
549+
550+
describe("preparePluginNativeCode", () => {
551+
const setupTest = (opts: { hasChangesInShasums?: boolean, newPluginHashes?: IStringDictionary, buildDataFileExists?: boolean, hasPluginPlatformsDir?: boolean }): any => {
552+
const testData: any = {
553+
pluginsService: null,
554+
isPreparePluginNativeCodeCalled: false,
555+
dataPassedToWriteJson: null
556+
};
557+
558+
const unitTestsInjector = new Yok();
559+
unitTestsInjector.register("platformsData", {
560+
getPlatformData: (platform: string, projectData: IProjectData) => ({
561+
projectRoot: "projectRoot",
562+
platformProjectService: {
563+
preparePluginNativeCode: async (pluginData: IPluginData, projData: IProjectData) => {
564+
testData.isPreparePluginNativeCodeCalled = true;
565+
}
566+
}
567+
})
568+
});
569+
570+
const pluginHashes = opts.newPluginHashes || { "file1": "hash1" };
571+
const pluginData: IPluginData = <any>{
572+
fullPath: "plugin_full_path",
573+
name: "plugin_name"
574+
};
575+
576+
unitTestsInjector.register("filesHashService", {
577+
hasChangesInShasums: (oldPluginNativeHashes: IStringDictionary, currentPluginNativeHashes: IStringDictionary) => !!opts.hasChangesInShasums,
578+
generateHashes: async (files: string[]): Promise<IStringDictionary> => pluginHashes
579+
});
580+
581+
unitTestsInjector.register("fs", {
582+
exists: (file: string) => {
583+
if (file.indexOf(PLUGINS_BUILD_DATA_FILENAME) !== -1) {
584+
return !!opts.buildDataFileExists;
585+
}
586+
587+
if (file.indexOf("platforms") !== -1) {
588+
return !!opts.hasPluginPlatformsDir;
589+
}
590+
591+
return true;
592+
},
593+
readJson: (file: string) => ({
594+
[pluginData.name]: pluginHashes
595+
}),
596+
writeJson: (file: string, json: any) => { testData.dataPassedToWriteJson = json; },
597+
enumerateFilesInDirectorySync: (): string[] => ["some_file"]
598+
});
599+
600+
unitTestsInjector.register("npm", {});
601+
unitTestsInjector.register("options", {});
602+
unitTestsInjector.register("logger", {});
603+
unitTestsInjector.register("errors", {});
604+
unitTestsInjector.register("injector", unitTestsInjector);
605+
606+
const pluginsService: PluginsService = unitTestsInjector.resolve(PluginsService);
607+
testData.pluginsService = pluginsService;
608+
testData.pluginData = pluginData;
609+
return testData;
610+
};
611+
612+
const platform = "platform";
613+
const projectData: IProjectData = <any>{};
614+
615+
it("does not prepare the files when plugin does not have platforms dir", async () => {
616+
const testData = setupTest({ hasPluginPlatformsDir: false });
617+
await testData.pluginsService.preparePluginNativeCode(testData.pluginData, platform, projectData);
618+
assert.isFalse(testData.isPreparePluginNativeCodeCalled);
619+
});
620+
621+
it("prepares the files when plugin has platforms dir and has not been built before", async () => {
622+
const newPluginHashes = { "file": "hash" };
623+
const testData = setupTest({ newPluginHashes, hasPluginPlatformsDir: true });
624+
await testData.pluginsService.preparePluginNativeCode(testData.pluginData, platform, projectData);
625+
assert.isTrue(testData.isPreparePluginNativeCodeCalled);
626+
assert.deepEqual(testData.dataPassedToWriteJson, { [testData.pluginData.name]: newPluginHashes });
627+
});
628+
629+
it("does not prepare the files when plugin has platforms dir and files have not changed since then", async () => {
630+
const testData = setupTest({ hasChangesInShasums: false, buildDataFileExists: true, hasPluginPlatformsDir: true });
631+
await testData.pluginsService.preparePluginNativeCode(testData.pluginData, platform, projectData);
632+
assert.isFalse(testData.isPreparePluginNativeCodeCalled);
633+
});
634+
});
544635
});

‎test/project-changes-service.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { PlatformsData } from "../lib/platforms-data";
66
import { ProjectChangesService } from "../lib/services/project-changes-service";
77
import * as Constants from "../lib/constants";
88
import { FileSystem } from "../lib/common/file-system";
9-
import { HooksServiceStub } from "./stubs";
9+
import { HooksServiceStub, LoggerStub } from "./stubs";
1010

1111
// start tracking temporary folders/files
1212
temp.track();
@@ -34,9 +34,7 @@ class ProjectChangesServiceTest extends BaseServiceTest {
3434
this.injector.register("filesHashService", {
3535
generateHashes: () => Promise.resolve({})
3636
});
37-
this.injector.register("logger", {
38-
warn: () => ({})
39-
});
37+
this.injector.register("logger", LoggerStub);
4038
this.injector.register("hooksService", HooksServiceStub);
4139

4240
const fs = this.injector.resolve<IFileSystem>("fs");

0 commit comments

Comments
 (0)