Merge pull request #3983 from NativeScript/fatme/fix-android-hashes

Fatme · web-flow · commit 47a2c8693e73 · 2018-10-10T09:14:23.000+03:00
fix: don't update local hashes on livesync
diff --git a/lib/common/definitions/mobile.d.ts b/lib/common/definitions/mobile.d.ts
@@ -118,6 +118,11 @@ declare module Mobile {
 	interface IAndroidDevice extends IDevice {
 		adb: Mobile.IDeviceAndroidDebugBridge;
 		init(): Promise<void>;
+		fileSystem: Mobile.IAndroidDeviceFileSystem;
+	}
+
+	interface IAndroidDeviceFileSystem extends IDeviceFileSystem {
+		getDeviceHashService(appIdentifier: string): Mobile.IAndroidDeviceHashService;
 	}
 
 	interface IiOSSimulator extends IDevice { }
diff --git a/lib/common/mobile/android/android-device-file-system.ts b/lib/common/mobile/android/android-device-file-system.ts
@@ -147,16 +147,16 @@ export class AndroidDeviceFileSystem implements Mobile.IDeviceFileSystem {
 		await deviceHashService.uploadHashFileToDevice(hashes);
 	}
 
-	private getTempDir(): string {
-		temp.track();
-		return temp.mkdirSync("application-");
-	}
-
-	private getDeviceHashService(appIdentifier: string): Mobile.IAndroidDeviceHashService {
+	public getDeviceHashService(appIdentifier: string): Mobile.IAndroidDeviceHashService {
 		if (!this._deviceHashServices[appIdentifier]) {
 			this._deviceHashServices[appIdentifier] = this.$injector.resolve(AndroidDeviceHashService, { adb: this.adb, appIdentifier });
 		}
 
 		return this._deviceHashServices[appIdentifier];
 	}
+
+	private getTempDir(): string {
+		temp.track();
+		return temp.mkdirSync("application-");
+	}
 }
diff --git a/lib/common/mobile/android/android-device.ts b/lib/common/mobile/android/android-device.ts
@@ -19,7 +19,7 @@ interface IAdbDeviceStatusInfo {
 export class AndroidDevice implements Mobile.IAndroidDevice {
 	public adb: Mobile.IDeviceAndroidDebugBridge;
 	public applicationManager: Mobile.IDeviceApplicationManager;
-	public fileSystem: Mobile.IDeviceFileSystem;
+	public fileSystem: Mobile.IAndroidDeviceFileSystem;
 	public deviceInfo: Mobile.IDeviceInfo;
 
 	// http://stackoverflow.com/questions/31178195/what-does-adb-device-status-mean
diff --git a/lib/common/test/unit-tests/mobile/android-device-file-system.ts b/lib/common/test/unit-tests/mobile/android-device-file-system.ts
@@ -12,7 +12,7 @@ import { LiveSyncPaths } from "../../../constants";
 const myTestAppIdentifier = "org.nativescript.myApp";
 let isAdbPushExecuted = false;
 let isAdbPushAppDirCalled = false;
-let androidDeviceFileSystem: Mobile.IDeviceFileSystem;
+let androidDeviceFileSystem: Mobile.IAndroidDeviceFileSystem;
 
 class AndroidDebugBridgeMock {
 	public executeCommand(args: string[]) {
@@ -138,10 +138,14 @@ function setup(options?: {
 	return {
 		localToDevicePaths,
 		deviceAppData,
-		projectRoot
+		projectRoot,
+		injector
 	};
 }
 
+let resolveParams: any[] = [];
+const appIdentifier = "testAppIdentifier";
+
 describe("AndroidDeviceFileSystem", () => {
 	describe("transferDirectory", () => {
 		it("pushes the whole directory when hash file doesn't exist on device", async () => {
@@ -186,4 +190,52 @@ describe("AndroidDeviceFileSystem", () => {
 			assert.isFalse(isAdbPushAppDirCalled);
 		});
 	});
+
+	describe("getDeviceHashService", () => {
+		beforeEach(() => {
+			resolveParams = [];
+			const { injector } = setup();
+			injector.resolve = (service: any, args: string[]) => resolveParams.push({ service, args });
+		});
+		it("should resolve AndroidDeviceHashService when the key is not stored in dictionary", () => {
+			androidDeviceFileSystem.getDeviceHashService(appIdentifier);
+			assert.equal(resolveParams.length, 1);
+			assert.isFunction(resolveParams[0].service);
+			assert.isDefined(resolveParams[0].args.adb);
+			assert.equal(resolveParams[0].args.appIdentifier, appIdentifier);
+		});
+		it("should return already stored value when the method is called for second time with the same deviceIdentifier and appIdentifier", () => {
+			androidDeviceFileSystem.getDeviceHashService(appIdentifier);
+			assert.equal(resolveParams.length, 1);
+			assert.isFunction(resolveParams[0].service);
+			assert.isDefined(resolveParams[0].args.adb);
+			assert.equal(resolveParams[0].args.appIdentifier, appIdentifier);
+
+			androidDeviceFileSystem.getDeviceHashService(appIdentifier);
+			assert.equal(resolveParams.length, 1);
+			assert.isFunction(resolveParams[0].service);
+			assert.isDefined(resolveParams[0].args.adb);
+			assert.equal(resolveParams[0].args.appIdentifier, appIdentifier);
+		});
+		it("should return AndroidDeviceHashService when the method is called for second time with different appIdentifier and same deviceIdentifier", () => {
+			androidDeviceFileSystem.getDeviceHashService(appIdentifier);
+			assert.equal(resolveParams.length, 1);
+			assert.isFunction(resolveParams[0].service);
+			assert.isDefined(resolveParams[0].args.adb);
+			assert.equal(resolveParams[0].args.appIdentifier, appIdentifier);
+
+			androidDeviceFileSystem.getDeviceHashService(appIdentifier);
+			assert.equal(resolveParams.length, 1);
+			assert.isFunction(resolveParams[0].service);
+			assert.isDefined(resolveParams[0].args.adb);
+			assert.equal(resolveParams[0].args.appIdentifier, appIdentifier);
+
+			const newAppIdentifier = "myNewAppIdentifier";
+			androidDeviceFileSystem.getDeviceHashService(newAppIdentifier);
+			assert.equal(resolveParams.length, 2);
+			assert.isFunction(resolveParams[1].service);
+			assert.isDefined(resolveParams[1].args.adb);
+			assert.equal(resolveParams[1].args.appIdentifier, newAppIdentifier);
+		});
+	});
 });
diff --git a/lib/definitions/livesync.d.ts b/lib/definitions/livesync.d.ts
@@ -414,13 +414,6 @@ interface INativeScriptDeviceLiveSyncService extends IDeviceLiveSyncServiceBase
 }
 
 interface IAndroidNativeScriptDeviceLiveSyncService extends INativeScriptDeviceLiveSyncService {
-	/**
-	 * Retrieves the android device's hash service.
-	 * @param  {string} appIdentifier Application identifier.
-	 * @return {Promise<Mobile.IAndroidDeviceHashService>} The hash service
-	 */
-	getDeviceHashService(appIdentifier: string): Mobile.IAndroidDeviceHashService;
-
 	/**
 	 * Guarantees all remove/update operations have finished
 	 * @param  {ILiveSyncResultInfo} liveSyncInfo Describes the LiveSync operation - for which project directory is the operation and other settings.
diff --git a/lib/device-path-provider.ts b/lib/device-path-provider.ts
@@ -1,12 +1,10 @@
 import { fromWindowsRelativePathToUnix } from "./common/helpers";
 import { APP_FOLDER_NAME } from "./constants";
 import { LiveSyncPaths } from "./common/constants";
-import { AndroidDeviceLiveSyncService } from "./services/livesync/android-device-livesync-service";
 import * as path from "path";
 
 export class DevicePathProvider implements IDevicePathProvider {
 	constructor(private $mobileHelper: Mobile.IMobileHelper,
-		private $injector: IInjector,
 		private $iOSSimResolver: Mobile.IiOSSimResolver,
 		private $errors: IErrors) {
 	}
@@ -26,8 +24,7 @@ export class DevicePathProvider implements IDevicePathProvider {
 		} else if (this.$mobileHelper.isAndroidPlatform(device.deviceInfo.platform)) {
 			projectRoot = `${LiveSyncPaths.ANDROID_TMP_DIR_NAME}/${options.appIdentifier}`;
 			if (!options.getDirname) {
-				const deviceLiveSyncService = this.$injector.resolve<AndroidDeviceLiveSyncService>(AndroidDeviceLiveSyncService, { device });
-				const hashService = deviceLiveSyncService.getDeviceHashService(options.appIdentifier);
+				const hashService = (<Mobile.IAndroidDevice>device).fileSystem.getDeviceHashService(options.appIdentifier);
 				const hashFile = options.syncAllFiles ? null : await hashService.doesShasumFileExistsOnDevice();
 				const syncFolderName = options.watch || hashFile ? LiveSyncPaths.SYNC_DIR_NAME : LiveSyncPaths.FULLSYNC_DIR_NAME;
 				projectRoot = path.join(projectRoot, syncFolderName);
diff --git a/lib/services/livesync/android-device-livesync-service-base.ts b/lib/services/livesync/android-device-livesync-service-base.ts
@@ -1,42 +1,28 @@
 import { DeviceLiveSyncServiceBase } from './device-livesync-service-base';
-import { AndroidDeviceHashService } from "../../common/mobile/android/android-device-hash-service";
 
 export abstract class AndroidDeviceLiveSyncServiceBase extends DeviceLiveSyncServiceBase {
-	private deviceHashServices: IDictionary<Mobile.IAndroidDeviceHashService>;
-
 	constructor(protected $injector: IInjector,
 		protected $platformsData: IPlatformsData,
 		protected $filesHashService: IFilesHashService,
 		protected $logger: ILogger,
 		protected device: Mobile.IAndroidDevice) {
 			super($platformsData, device);
-			this.deviceHashServices = {};
 	}
 
 	public abstract async transferFilesOnDevice(deviceAppData: Mobile.IDeviceAppData, localToDevicePaths: Mobile.ILocalToDevicePathData[]): Promise<void>;
 	public abstract async transferDirectoryOnDevice(deviceAppData: Mobile.IDeviceAppData, localToDevicePaths: Mobile.ILocalToDevicePathData[], projectFilesPath: string): Promise<void>;
 
-	public getDeviceHashService(appIdentifier: string): Mobile.IAndroidDeviceHashService {
-		const key = `${this.device.deviceInfo.identifier}${appIdentifier}`;
-		if (!this.deviceHashServices[key]) {
-			const deviceHashService = this.$injector.resolve(AndroidDeviceHashService, { adb: this.device.adb, appIdentifier });
-			this.deviceHashServices[key] = deviceHashService;
-		}
-
-		return this.deviceHashServices[key];
-	}
-
 	public async transferFiles(deviceAppData: Mobile.IDeviceAppData, localToDevicePaths: Mobile.ILocalToDevicePathData[], projectFilesPath: string, projectData: IProjectData, liveSyncDeviceInfo: ILiveSyncDeviceInfo, options: ITransferFilesOptions): Promise<Mobile.ILocalToDevicePathData[]> {
-		const deviceHashService = this.getDeviceHashService(deviceAppData.appIdentifier);
+		const deviceHashService = this.device.fileSystem.getDeviceHashService(deviceAppData.appIdentifier);
 		const currentHashes = await deviceHashService.generateHashesFromLocalToDevicePaths(localToDevicePaths);
 		const transferredFiles = await this.transferFilesCore(deviceAppData, localToDevicePaths, projectFilesPath, currentHashes, options);
-		await this.updateHashes(deviceAppData, currentHashes, projectData, liveSyncDeviceInfo);
+		await this.device.fileSystem.updateHashesOnDevice(currentHashes, deviceAppData.appIdentifier);
 		return transferredFiles;
 	}
 
 	private async transferFilesCore(deviceAppData: Mobile.IDeviceAppData, localToDevicePaths: Mobile.ILocalToDevicePathData[], projectFilesPath: string, currentHashes: IStringDictionary, options: ITransferFilesOptions): Promise<Mobile.ILocalToDevicePathData[]> {
 		if (options.force && options.isFullSync) {
-			const hashFileDevicePath = this.getDeviceHashService(deviceAppData.appIdentifier).hashFileDevicePath;
+			const hashFileDevicePath = this.device.fileSystem.getDeviceHashService(deviceAppData.appIdentifier).hashFileDevicePath;
 			await this.device.fileSystem.deleteFile(hashFileDevicePath, deviceAppData.appIdentifier);
 			this.$logger.trace("Before transfer directory on device ", localToDevicePaths);
 			await this.transferDirectoryOnDevice(deviceAppData, localToDevicePaths, projectFilesPath);
@@ -59,27 +45,11 @@ export abstract class AndroidDeviceLiveSyncServiceBase extends DeviceLiveSyncSer
 	}
 
 	private async getChangedLocalToDevicePaths(appIdentifier: string, localToDevicePaths: Mobile.ILocalToDevicePathData[], currentHashes: IStringDictionary): Promise<Mobile.ILocalToDevicePathData[]> {
-		const deviceHashService = this.getDeviceHashService(appIdentifier);
+		const deviceHashService = this.device.fileSystem.getDeviceHashService(appIdentifier);
 		const oldHashes = (await deviceHashService.getShasumsFromDevice()) || {};
 		const changedHashes = deviceHashService.getChangedShasums(oldHashes, currentHashes);
 		const changedFiles = _.keys(changedHashes);
 		const changedLocalToDevicePaths = localToDevicePaths.filter(localToDevicePathData => changedFiles.indexOf(localToDevicePathData.getLocalPath()) >= 0);
 		return changedLocalToDevicePaths;
 	}
-
-	private async updateHashes(deviceAppData: Mobile.IDeviceAppData, currentHashes: IStringDictionary, projectData: IProjectData, liveSyncDeviceInfo: ILiveSyncDeviceInfo): Promise<void> {
-		const hashes = await this.updateHashesOnDevice(deviceAppData, currentHashes, projectData, liveSyncDeviceInfo);
-		this.updateLocalHashes(hashes, deviceAppData, projectData, liveSyncDeviceInfo);
-	}
-
-	private async updateHashesOnDevice(deviceAppData: Mobile.IDeviceAppData, currentHashes: IStringDictionary, projectData: IProjectData, liveSyncDeviceInfo: ILiveSyncDeviceInfo): Promise<IStringDictionary> {
-		const deviceHashService = this.getDeviceHashService(deviceAppData.appIdentifier);
-		await deviceHashService.uploadHashFileToDevice(currentHashes);
-		return currentHashes;
-	}
-
-	private updateLocalHashes(hashes: IStringDictionary, deviceAppData: Mobile.IDeviceAppData, projectData: IProjectData, liveSyncDeviceInfo: ILiveSyncDeviceInfo): void {
-		const hashFilePath = liveSyncDeviceInfo.outputPath || this.$platformsData.getPlatformData(deviceAppData.platform, projectData).deviceBuildOutputPath;
-		this.$filesHashService.saveHashes(hashes, hashFilePath);
-	}
 }
diff --git a/lib/services/livesync/android-device-livesync-service.ts b/lib/services/livesync/android-device-livesync-service.ts
@@ -119,7 +119,8 @@ export class AndroidDeviceLiveSyncService extends AndroidDeviceLiveSyncServiceBa
 			await this.device.adb.executeShellCommand(["mkdir", "-p", path.dirname(deviceFilePath), " && ", "touch", deviceFilePath]);
 		}
 
-		await this.getDeviceHashService(deviceAppData.appIdentifier).removeHashes(localToDevicePaths);
+		const deviceHashService = this.device.fileSystem.getDeviceHashService(deviceAppData.appIdentifier);
+		await deviceHashService.removeHashes(localToDevicePaths);
 	}
 
 	private async awaitRuntimeReloadSuccessMessage(): Promise<boolean> {
diff --git a/lib/services/livesync/android-device-livesync-sockets-service.ts b/lib/services/livesync/android-device-livesync-sockets-service.ts
@@ -101,7 +101,7 @@ export class AndroidDeviceSocketsLiveSyncService extends AndroidDeviceLiveSyncSe
 
 	public async removeFiles(deviceAppData: Mobile.IDeviceAppData, localToDevicePaths: Mobile.ILocalToDevicePathData[], projectFilesPath: string): Promise<void> {
 		await this.livesyncTool.removeFiles(_.map(localToDevicePaths, (element: any) => element.filePath));
-		const deviceHashService = this.getDeviceHashService(deviceAppData.appIdentifier);
+		const deviceHashService = this.device.fileSystem.getDeviceHashService(deviceAppData.appIdentifier);
 		await deviceHashService.removeHashes(localToDevicePaths);
 	}
 
diff --git a/test/services/livesync/android-device-livesync-service-base.ts b/test/services/livesync/android-device-livesync-service-base.ts

Original file line number	Diff line number	Diff line change
`@@ -147,16 +147,16 @@ export class AndroidDeviceFileSystem implements Mobile.IDeviceFileSystem {`
`147`	`147`	`await deviceHashService.uploadHashFileToDevice(hashes);`
`148`	`148`	`}`
`149`	`149`
`150`		`- private getTempDir(): string {`
`151`		`- temp.track();`
`152`		`- return temp.mkdirSync("application-");`
`153`		`- }`
`154`		`-`
`155`		`- private getDeviceHashService(appIdentifier: string): Mobile.IAndroidDeviceHashService {`
	`150`	`+ public getDeviceHashService(appIdentifier: string): Mobile.IAndroidDeviceHashService {`
`156`	`151`	`if (!this._deviceHashServices[appIdentifier]) {`
`157`	`152`	`this._deviceHashServices[appIdentifier] = this.$injector.resolve(AndroidDeviceHashService, { adb: this.adb, appIdentifier });`
`158`	`153`	`}`
`159`	`154`
`160`	`155`	`return this._deviceHashServices[appIdentifier];`
`161`	`156`	`}`
	`157`	`+`
	`158`	`+ private getTempDir(): string {`
	`159`	`+ temp.track();`
	`160`	`+ return temp.mkdirSync("application-");`
	`161`	`+ }`
`162`	`162`	`}`
Original file line number	Diff line number	Diff line change
`@@ -119,7 +119,8 @@ export class AndroidDeviceLiveSyncService extends AndroidDeviceLiveSyncServiceBa`
`119`	`119`	`await this.device.adb.executeShellCommand(["mkdir", "-p", path.dirname(deviceFilePath), " && ", "touch", deviceFilePath]);`
`120`	`120`	`}`
`121`	`121`
`122`		`- await this.getDeviceHashService(deviceAppData.appIdentifier).removeHashes(localToDevicePaths);`
	`122`	`+ const deviceHashService = this.device.fileSystem.getDeviceHashService(deviceAppData.appIdentifier);`
	`123`	`+ await deviceHashService.removeHashes(localToDevicePaths);`
`123`	`124`	`}`
`124`	`125`
`125`	`126`	`private async awaitRuntimeReloadSuccessMessage(): Promise<boolean> {`
Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ export class AndroidDeviceSocketsLiveSyncService extends AndroidDeviceLiveSyncSe`
`101`	`101`
`102`	`102`	`public async removeFiles(deviceAppData: Mobile.IDeviceAppData, localToDevicePaths: Mobile.ILocalToDevicePathData[], projectFilesPath: string): Promise<void> {`
`103`	`103`	`await this.livesyncTool.removeFiles(_.map(localToDevicePaths, (element: any) => element.filePath));`
`104`		`- const deviceHashService = this.getDeviceHashService(deviceAppData.appIdentifier);`
	`104`	`+ const deviceHashService = this.device.fileSystem.getDeviceHashService(deviceAppData.appIdentifier);`
`105`	`105`	`await deviceHashService.removeHashes(localToDevicePaths);`
`106`	`106`	`}`
`107`	`107`