Merge pull request #5121 from NativeScript/vladimirov/fix-values-filtering

fix: values-v<sdk> directories are not prepared correctly

Author: rosen-vladimirov

lib/services/android-project-service.ts

@@ -142,11 +142,23 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
 
 		this.copy(this.getPlatformData(projectData).projectRoot, frameworkDir, "*", "-R");
 
+		// TODO: Check if we actually need this and if it should be targetSdk or compileSdk
 		this.cleanResValues(targetSdkVersion, projectData);
 	}
 
+	private getResDestinationDir(projectData: IProjectData): string {
+		const appResourcesDirStructureHasMigrated = this.$androidResourcesMigrationService.hasMigrated(projectData.getAppResourcesDirectoryPath());
+
+		if (appResourcesDirStructureHasMigrated) {
+			const appResourcesDestinationPath = this.getUpdatedAppResourcesDestinationDirPath(projectData);
+			return path.join(appResourcesDestinationPath, constants.MAIN_DIR, constants.RESOURCES_DIR);
+		} else {
+			return this.getLegacyAppResourcesDestinationDirPath(projectData);
+		}
+	}
+
 	private cleanResValues(targetSdkVersion: number, projectData: IProjectData): void {
-		const resDestinationDir = this.getAppResourcesDestinationDirectoryPath(projectData);
+		const resDestinationDir = this.getResDestinationDir(projectData);
 		const directoriesInResFolder = this.$fs.readDirectory(resDestinationDir);
 		const directoriesToClean = directoriesInResFolder
 			.map(dir => {
@@ -283,7 +295,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
 		const projectAppResourcesPath = projectData.getAppResourcesDirectoryPath(projectData.projectDir);
 		const platformsAppResourcesPath = this.getAppResourcesDestinationDirectoryPath(projectData);
 
-		this.cleanUpPreparedResources(projectAppResourcesPath, projectData);
+		this.cleanUpPreparedResources(projectData);
 
 		this.$fs.ensureDirectoryExists(platformsAppResourcesPath);
 
@@ -296,6 +308,10 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
 			// App_Resources/Android/libs is reserved to user's aars and jars, but they should not be copied as resources
 			this.$fs.deleteDirectory(path.join(platformsAppResourcesPath, "libs"));
 		}
+
+		const androidToolsInfo = this.$androidToolsInfo.getToolsInfo({ projectDir: projectData.projectDir });
+		const compileSdkVersion = androidToolsInfo && androidToolsInfo.compileSdkVersion;
+		this.cleanResValues(compileSdkVersion, projectData);
 	}
 
 	public async preparePluginNativeCode(pluginData: IPluginData, projectData: IProjectData): Promise<void> {
@@ -431,18 +447,31 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
 		return path.join(this.getPlatformData(projectData).projectRoot, ...resourcePath);
 	}
 
-	private cleanUpPreparedResources(appResourcesDirectoryPath: string, projectData: IProjectData): void {
-		let resourcesDirPath = path.join(appResourcesDirectoryPath, this.getPlatformData(projectData).normalizedPlatformName);
+	/**
+	 * The purpose of this method is to delete the previously prepared user resources.
+	 * The content of the `<platforms>/android/.../res` directory is based on user's resources and gradle project template from android-runtime.
+	 * During preparation of the `<path to user's App_Resources>/Android` we want to clean all the users files from previous preparation,
+	 * but keep the ones that were introduced during `platform add` of the android-runtime.
+	 * Currently the Gradle project template contains resources only in values and values-v21 directories.
+	 * So the current logic of the method is cleaning al resources from `<platforms>/android/.../res` that are not in `values.*` directories
+	 * and that exist in the `<path to user's App_Resources>/Android/.../res` directory
+	 * This means that if user has a resource file in values-v29 for example, builds the project and then deletes this resource,
+	 * it will be kept in platforms directory. Reference issue: `https://github.com/NativeScript/nativescript-cli/issues/5083`
+	 * Same is valid for files in `drawable-<resolution>` directories - in case in user's resources there's drawable-hdpi directory,
+	 * which is deleted after the first build of app, it will remain in platforms directory.
+	 */
+	private cleanUpPreparedResources(projectData: IProjectData): void {
+		let resourcesDirPath = path.join(projectData.appResourcesDirectoryPath, this.getPlatformData(projectData).normalizedPlatformName);
 		if (this.$androidResourcesMigrationService.hasMigrated(projectData.appResourcesDirectoryPath)) {
-			resourcesDirPath = path.join(resourcesDirPath, constants.MAIN_DIR, constants.RESOURCES_DIR);
+			resourcesDirPath = path.join(resourcesDirPath, constants.SRC_DIR, constants.MAIN_DIR, constants.RESOURCES_DIR);
 		}
 
 		const valuesDirRegExp = /^values/;
 		if (this.$fs.exists(resourcesDirPath)) {
 			const resourcesDirs = this.$fs.readDirectory(resourcesDirPath).filter(resDir => !resDir.match(valuesDirRegExp));
-			const appResourcesDestinationDirectoryPath = this.getAppResourcesDestinationDirectoryPath(projectData);
-			_.each(resourcesDirs, resourceDir => {
-				this.$fs.deleteDirectory(path.join(appResourcesDestinationDirectoryPath, resourceDir));
+			const resDestinationDir = this.getResDestinationDir(projectData);
+			_.each(resourcesDirs, currentResource => {
+				this.$fs.deleteDirectory(path.join(resDestinationDir, currentResource));
 			});
 		}
 	}

test/services/android-project-service.ts

@@ -3,6 +3,7 @@ import { Yok } from "../../lib/common/yok";
 import * as stubs from "../stubs";
 import { assert } from "chai";
 import * as sinon from "sinon";
+import * as path from "path";
 import { GradleCommandService } from "../../lib/services/android/gradle-command-service";
 import { GradleBuildService } from "../../lib/services/android/gradle-build-service";
 import { GradleBuildArgsService } from "../../lib/services/android/gradle-build-args-service";
@@ -42,7 +43,7 @@ const createTestInjector = (): IInjector => {
 	testInjector.register("gradleBuildService", GradleBuildService);
 	testInjector.register("gradleBuildArgsService", GradleBuildArgsService);
 	testInjector.register("analyticsService", stubs.AnalyticsService);
-	testInjector.register("staticConfig", {TRACK_FEATURE_USAGE_SETTING_NAME: "TrackFeatureUsage"});
+	testInjector.register("staticConfig", { TRACK_FEATURE_USAGE_SETTING_NAME: "TrackFeatureUsage" });
 	return testInjector;
 };
 
@@ -59,7 +60,7 @@ const getDefautlBuildConfig = (): IBuildConfig => {
 	};
 };
 
-describe("androidDeviceDebugService", () => {
+describe("androidProjectService", () => {
 	let injector: IInjector;
 	let androidProjectService: IPlatformProjectService;
 	let sandbox: sinon.SinonSandbox = null;
@@ -74,7 +75,7 @@ describe("androidDeviceDebugService", () => {
 		sandbox.restore();
 	});
 
-	describe("buildPlatform", () => {
+	describe("buildProject", () => {
 		let projectData: IProjectData;
 		let childProcess: stubs.ChildProcessStub;
 
@@ -138,4 +139,182 @@ describe("androidDeviceDebugService", () => {
 			assert.include(childProcess.lastCommandArgs, "bundleDebug");
 		});
 	});
+
+	describe("prepareAppResources", () => {
+		const projectDir = "testDir";
+		const pathToAppResourcesDir = path.join(projectDir, "app", "App_Resources");
+		const pathToAppResourcesAndroid = path.join(pathToAppResourcesDir, "Android");
+		const pathToPlatformsAndroid = path.join(projectDir, "platforms", "android");
+		const pathToResDirInPlatforms = path.join(pathToPlatformsAndroid, "app", "src", "main", "res");
+		const valuesV27Path = path.join(pathToResDirInPlatforms, "values-v27");
+		const valuesV28Path = path.join(pathToResDirInPlatforms, "values-v28");
+		const libsPath = path.join(pathToResDirInPlatforms, "libs");
+		const drawableHdpiPath = path.join(pathToResDirInPlatforms, "drawable-hdpi");
+		const drawableLdpiPath = path.join(pathToResDirInPlatforms, "drawable-ldpi");
+		let deletedDirs: string[] = [];
+		let copiedFiles: { sourceFileName: string, destinationFileName: string }[] = [];
+		let readDirectoryResults: IDictionary<string[]> = {};
+		let fs: IFileSystem = null;
+		let projectData: IProjectData = null;
+		let compileSdkVersion = 29;
+
+		beforeEach(() => {
+			projectData = injector.resolve("projectData");
+			projectData.projectDir = projectDir;
+			projectData.appResourcesDirectoryPath = pathToAppResourcesDir;
+
+			deletedDirs = [];
+			copiedFiles = [];
+			readDirectoryResults = {};
+
+			fs = injector.resolve<IFileSystem>("fs");
+			fs.deleteDirectory = (directory: string): void => {
+				deletedDirs.push(directory);
+			};
+			fs.copyFile = (sourceFileName: string, destinationFileName: string): void => {
+				copiedFiles.push({ sourceFileName, destinationFileName });
+			};
+			fs.readDirectory = (dir: string): string[] => {
+				return readDirectoryResults[dir] || [];
+			};
+
+			compileSdkVersion = 29;
+
+			const androidToolsInfo = injector.resolve<IAndroidToolsInfo>("androidToolsInfo");
+			androidToolsInfo.getToolsInfo = (config?: IProjectDir): IAndroidToolsInfoData => {
+				return <any>{
+					compileSdkVersion
+				};
+			};
+
+		});
+
+		describe("when new Android App_Resources structure is detected (post {N} 4.0 structure)", () => {
+			const pathToSrcDirInAppResources = path.join(pathToAppResourcesAndroid, "src");
+			beforeEach(() => {
+				const androidResourcesMigrationService = injector.resolve<IAndroidResourcesMigrationService>("androidResourcesMigrationService");
+				androidResourcesMigrationService.hasMigrated = () => true;
+			});
+
+			it("copies everything from App_Resources/Android/src to correct location in platforms", async () => {
+				await androidProjectService.prepareAppResources(projectData);
+
+				assert.deepEqual(copiedFiles, [{ sourceFileName: path.join(pathToSrcDirInAppResources, "*"), destinationFileName: path.join(projectData.projectDir, "platforms", "android", "app", "src") }]);
+			});
+
+			it("deletes correct values-<sdk> directories based on the compileSdk", async () => {
+				readDirectoryResults = {
+					[`${pathToResDirInPlatforms}`]: [
+						"values",
+						"values-v21",
+						"values-v26",
+						"values-v27",
+						"values-v28"
+					]
+				};
+
+				compileSdkVersion = 26;
+				await androidProjectService.prepareAppResources(projectData);
+				assert.deepEqual(deletedDirs, [
+					valuesV27Path,
+					valuesV28Path
+				]);
+			});
+
+			it("deletes drawable directories when they've been previously prepared", async () => {
+				readDirectoryResults = {
+					[path.join(pathToSrcDirInAppResources, "main", "res")]: [
+						"drawable-hdpi",
+						"drawable-ldpi",
+						"values",
+						"values-v21",
+						"values-v29"
+					],
+					[`${pathToResDirInPlatforms}`]: [
+						"drawable-hdpi",
+						"drawable-ldpi",
+						"drawable-mdpi",
+						"values",
+						"values-v21",
+						"values-v29"
+					]
+				};
+
+				await androidProjectService.prepareAppResources(projectData);
+
+				// NOTE: Currently the drawable-mdpi directory is not deleted from prepared App_Resources as it does not exist in the currently prepared App_Resources
+				// This is not correct behavior and it should be fixed in a later point.
+				assert.deepEqual(deletedDirs, [
+					drawableHdpiPath,
+					drawableLdpiPath
+				]);
+			});
+		});
+
+		describe("when old Android App_Resources structure is detected (post {N} 4.0 structure)", () => {
+			beforeEach(() => {
+				const androidResourcesMigrationService = injector.resolve<IAndroidResourcesMigrationService>("androidResourcesMigrationService");
+				androidResourcesMigrationService.hasMigrated = () => false;
+			});
+
+			it("copies everything from App_Resources/Android to correct location in platforms", async () => {
+				await androidProjectService.prepareAppResources(projectData);
+
+				assert.deepEqual(copiedFiles, [{ sourceFileName: path.join(pathToAppResourcesAndroid, "*"), destinationFileName: pathToResDirInPlatforms }]);
+			});
+
+			it("deletes correct values-<sdk> directories based on the compileSdk", async () => {
+				readDirectoryResults = {
+					[`${pathToResDirInPlatforms}`]: [
+						"values",
+						"values-v21",
+						"values-v26",
+						"values-v27",
+						"values-v28"
+					]
+				};
+
+				compileSdkVersion = 26;
+
+				await androidProjectService.prepareAppResources(projectData);
+
+				// During preparation of old App_Resources, CLI copies all of them in platforms and after that deletes the libs directory.
+				assert.deepEqual(deletedDirs, [
+					libsPath,
+					valuesV27Path,
+					valuesV28Path
+				]);
+			});
+
+			it("deletes drawable directories when they've been previously prepared", async () => {
+				readDirectoryResults = {
+					[`${pathToAppResourcesAndroid}`]: [
+						"drawable-hdpi",
+						"drawable-ldpi",
+						"values",
+						"values-v21",
+						"values-v29"
+					],
+					[`${pathToResDirInPlatforms}`]: [
+						"drawable-hdpi",
+						"drawable-ldpi",
+						"drawable-mdpi",
+						"values",
+						"values-v21",
+						"values-v29"
+					]
+				};
+
+				await androidProjectService.prepareAppResources(projectData);
+				// NOTE: Currently the drawable-mdpi directory is not deleted from prepared App_Resources as it does not exist in the currently prepared App_Resources
+				// This is not correct behavior and it should be fixed in a later point.
+				// During preparation of old App_Resources, CLI copies all of them in platforms and after that deletes the libs directory.
+				assert.deepEqual(deletedDirs, [
+					drawableHdpiPath,
+					drawableLdpiPath,
+					libsPath
+				]);
+			});
+		});
+	});
 });