diff --git a/api/ENVIRONMENT.md b/api/ENVIRONMENT.md index c7d0e0369..1fee46869 100644 --- a/api/ENVIRONMENT.md +++ b/api/ENVIRONMENT.md @@ -75,6 +75,7 @@ To perform the unit tests against an Azure server: To enable generating diffs for releases: - `ENABLE_PACKAGE_DIFFING`: Set to 'true' +- `DIFF_PACKAGE_COUNT`: Set to the number of releases to generate diffs for. Defaults to 5 if unspecified. To enable KeyVault credential resolution, set: @@ -82,3 +83,7 @@ To enable KeyVault credential resolution, set: - `CLIENT_ID`: The client ID of an Active Directory app that has access to your KeyVault account - `CERTIFICATE_THUMBPRINT`: The thumbprint of the certificate associated with your Active Directory app (for which a .pfx has been uploaded to your certificate store) - `REFRESH_CREDENTIALS_INTERVAL` (Optional): The frequency, in milliseconds, to re-retrieve credentials from Key Vault (defaults to one day, currently only storage keys are supported) + +To proxy update check URLs through a caching or proxy service: + +- `UPDATE_CHECK_PROXY_URL`: The base URL to use for proxying update check responses. For example, if set to "https://yourcdn.com/" Azure blob storage URLs will be transformed to use this domain instead. diff --git a/api/script/environment.ts b/api/script/environment.ts index 4ab5f0418..715784706 100644 --- a/api/script/environment.ts +++ b/api/script/environment.ts @@ -6,3 +6,7 @@ import * as os from 'os'; export function getTempDirectory(): string { return process.env.TEMP || process.env.TMPDIR || os.tmpdir(); } + +export function getUpdateCheckProxyUrl(): string { + return process.env.UPDATE_CHECK_PROXY_URL; +} diff --git a/api/script/routes/acquisition.ts b/api/script/routes/acquisition.ts index 3fc841078..25dfc3400 100644 --- a/api/script/routes/acquisition.ts +++ b/api/script/routes/acquisition.ts @@ -29,6 +29,7 @@ export interface AcquisitionConfig { function getUrlKey(originalUrl: string): string { const obj: any = URL.parse(originalUrl, /*parseQueryString*/ true); delete obj.query.clientUniqueId; + delete obj.query.client_unique_id; return obj.pathname + "?" + queryString.stringify(obj.query); } diff --git a/api/script/routes/management.ts b/api/script/routes/management.ts index 53a287e2f..ddc4d2e0a 100644 --- a/api/script/routes/management.ts +++ b/api/script/routes/management.ts @@ -722,9 +722,9 @@ export function getManagementRouter(config: ManagementConfig): Router { const newRolloutValue: number = info.rollout; if (validationUtils.isDefined(newRolloutValue)) { let errorMessage: string; - if (!isUnfinishedRollout(packageToUpdate.rollout)) { + if (!isUnfinishedRollout(packageToUpdate.rollout) && newRolloutValue !== 100) { errorMessage = "Cannot update rollout value for a completed rollout release."; - } else if (packageToUpdate.rollout >= newRolloutValue) { + } else if (packageToUpdate.rollout > newRolloutValue) { errorMessage = `Rollout value must be greater than "${packageToUpdate.rollout}", the existing value.`; } @@ -1261,54 +1261,75 @@ export function getManagementRouter(config: ManagementConfig): Router { appPackage: storageTypes.Package, diffPackageMap: storageTypes.PackageHashToBlobInfoMap ) { - let updateHistory: boolean = false; - - return storage - .getApp(accountId, appId) - .then((storageApp: storageTypes.App) => { - throwIfInvalidPermissions(storageApp, storageTypes.Permissions.Collaborator); - return storage.getPackageHistory(accountId, appId, deploymentId); - }) - .then((history: storageTypes.Package[]) => { - if (history) { - for (let i = history.length - 1; i >= 0; i--) { - if (history[i].label === appPackage.label && !history[i].diffPackageMap) { - history[i].diffPackageMap = diffPackageMap; - updateHistory = true; + console.log(`[addDiffInfoForPackage] Starting for package ${appPackage.label}, hash: ${appPackage.packageHash}`); + console.log(`[addDiffInfoForPackage] diffPackageMap exists: ${!!diffPackageMap}, entries: ${diffPackageMap ? Object.keys(diffPackageMap).length : 0}`); + + // Add diff info the package + if (diffPackageMap) { + return storage + .getPackageHistory(accountId, appId, deploymentId) + .then((history: storageTypes.Package[]) => { + console.log(`[addDiffInfoForPackage] Got history with ${history.length} packages`); + + // Update the diff info for the current package we deployed + for (const updatedPackage of history) { + if (updatedPackage.packageHash === appPackage.packageHash) { + updatedPackage.diffPackageMap = diffPackageMap; + console.log(`[addDiffInfoForPackage] Updated diff package map for package hash: ${updatedPackage.packageHash}`); break; } } - - if (updateHistory) { - return storage.updatePackageHistory(accountId, appId, deploymentId, history); - } - } - }) - .then(() => { - if (updateHistory) { + + console.log(`[addDiffInfoForPackage] Saving updated package history (${history.length} packages)`); + return storage.updatePackageHistory(accountId, appId, deploymentId, history); + }) + .then(() => { + console.log(`[addDiffInfoForPackage] Successfully updated package history`); return storage.getDeployment(accountId, appId, deploymentId).then((deployment: storageTypes.Deployment) => { + console.log(`[addDiffInfoForPackage] Invalidating cache for key ${deployment.key}`); return invalidateCachedPackage(deployment.key); }); - } - }) - .catch(diffErrorUtils.diffErrorHandler); + }); + } else { + console.log(`[addDiffInfoForPackage] No diff package map available, skipping update`); + return q(null); + } } function processDiff(accountId: string, appId: string, deploymentId: string, appPackage: storageTypes.Package): q.Promise { - if (!appPackage.manifestBlobUrl || process.env.ENABLE_PACKAGE_DIFFING) { + console.log(`[processDiff] Starting for package ${appPackage.label}, manifestBlobUrl exists: ${!!appPackage.manifestBlobUrl}`); + console.log(`[processDiff] ENABLE_PACKAGE_DIFFING env var: ${process.env.ENABLE_PACKAGE_DIFFING}`); + + if (!appPackage.manifestBlobUrl || !process.env.ENABLE_PACKAGE_DIFFING) { // No need to process diff because either: // 1. The release just contains a single file. // 2. Diffing disabled. + console.log(`[processDiff] Skipping diff generation: ${!appPackage.manifestBlobUrl ? 'No manifest URL' : 'Diffing disabled by env var'}`); return q(null); } - console.log(`Processing package: ${appPackage.label}`); + console.log(`[processDiff] Processing package: ${appPackage.label}, hash: ${appPackage.packageHash}`); return packageDiffing .generateDiffPackageMap(accountId, appId, deploymentId, appPackage) .then((diffPackageMap: storageTypes.PackageHashToBlobInfoMap) => { - console.log(`Package processed, adding diff info`); - addDiffInfoForPackage(accountId, appId, deploymentId, appPackage, diffPackageMap); + console.log(`[processDiff] Package processed, diffPackageMap exists: ${!!diffPackageMap}`); + if (diffPackageMap) { + console.log(`[processDiff] diffPackageMap has ${Object.keys(diffPackageMap).length} entries`); + Object.keys(diffPackageMap).forEach(key => { + console.log(`[processDiff] - Entry: ${key}, size: ${diffPackageMap[key].size} bytes`); + }); + } + + console.log(`[processDiff] Adding diff info to package`); + return addDiffInfoForPackage(accountId, appId, deploymentId, appPackage, diffPackageMap); + }) + .then(() => { + console.log(`[processDiff] Successfully completed diff processing`); + }) + .catch((error) => { + console.error(`[processDiff] Error during diff processing: ${error.message || error}`); + return diffErrorUtils.diffErrorHandler(error); }); } diff --git a/api/script/utils/acquisition.ts b/api/script/utils/acquisition.ts index bf8a7bce4..3d69a45e2 100644 --- a/api/script/utils/acquisition.ts +++ b/api/script/utils/acquisition.ts @@ -5,6 +5,27 @@ import * as semver from "semver"; import { UpdateCheckCacheResponse, UpdateCheckRequest, UpdateCheckResponse } from "../types/rest-definitions"; import { Package } from "../storage/storage"; import { isUnfinishedRollout } from "./rollout-selector"; +import * as env from "../environment"; +import * as URL from "url"; + +function proxyBlobUrl(azureUrl: string): string { + try { + const proxyUrl = env.getUpdateCheckProxyUrl(); + if (!proxyUrl) { + return azureUrl; + } + + const parsedUrl = new URL.URL(azureUrl); + const newUrl = new URL.URL(proxyUrl); + newUrl.pathname = parsedUrl.pathname; + newUrl.search = parsedUrl.search; + + return newUrl.toString(); + } catch (error) { + console.warn('Failed to proxy blob URL:', error); + return azureUrl; + } +} interface UpdatePackage { response: UpdateCheckResponse; @@ -114,10 +135,10 @@ function getUpdatePackage(packageHistory: Package[], request: UpdateCheckRequest latestSatisfyingEnabledPackage.diffPackageMap && latestSatisfyingEnabledPackage.diffPackageMap[request.packageHash] ) { - updateDetails.downloadURL = latestSatisfyingEnabledPackage.diffPackageMap[request.packageHash].url; + updateDetails.downloadURL = proxyBlobUrl(latestSatisfyingEnabledPackage.diffPackageMap[request.packageHash].url); updateDetails.packageSize = latestSatisfyingEnabledPackage.diffPackageMap[request.packageHash].size; } else { - updateDetails.downloadURL = latestSatisfyingEnabledPackage.blobUrl; + updateDetails.downloadURL = proxyBlobUrl(latestSatisfyingEnabledPackage.blobUrl); updateDetails.packageSize = latestSatisfyingEnabledPackage.size; } diff --git a/api/script/utils/package-diffing.ts b/api/script/utils/package-diffing.ts index 385c201e0..1fff0b39f 100644 --- a/api/script/utils/package-diffing.ts +++ b/api/script/utils/package-diffing.ts @@ -32,7 +32,6 @@ interface DiffBlobInfo { export class PackageDiffer { private static MANIFEST_FILE_NAME: string = "hotcodepush.json"; private static WORK_DIRECTORY_PATH: string = env.getTempDirectory(); - private static IS_WORK_DIRECTORY_CREATED: boolean = false; private _storage: storageTypes.Storage; private _maxPackagesToDiff: number; @@ -49,29 +48,51 @@ export class PackageDiffer { newPackage: storageTypes.Package ): Promise { if (!newPackage || !newPackage.blobUrl || !newPackage.manifestBlobUrl) { + console.info(`[generateDiffPackageMap] Missing required package info. blobUrl: ${!!newPackage?.blobUrl}, manifestBlobUrl: ${!!newPackage?.manifestBlobUrl}`); return q.reject( diffErrorUtils.diffError(diffErrorUtils.ErrorCode.InvalidArguments, "Package information missing") ); } + console.info(`[generateDiffPackageMap] Starting for package ${newPackage.label}, hash: ${newPackage.packageHash}`); + console.info(`[generateDiffPackageMap] Manifest URL: ${newPackage.manifestBlobUrl}`); + const manifestPromise: Promise = this.getManifest(newPackage); const historyPromise: Promise = this._storage.getPackageHistory(accountId, appId, deploymentId); const newReleaseFilePromise: Promise = this.downloadArchiveFromUrl(newPackage.blobUrl); let newFilePath: string; + console.info(`[generateDiffPackageMap] Promises created for manifest, history, and download`); + return q .all([manifestPromise, historyPromise, newReleaseFilePromise]) .spread((newManifest: PackageManifest, history: storageTypes.Package[], downloadedArchiveFile: string) => { newFilePath = downloadedArchiveFile; + + console.info(`[generateDiffPackageMap] Got manifest: ${newManifest ? 'yes' : 'no'}, history: ${history?.length || 0} packages, downloaded file: ${!!downloadedArchiveFile}`); + + if (!newManifest) { + console.info(`[generateDiffPackageMap] No manifest available, cannot generate diffs`); + return []; + } + + const fileMap = newManifest.toMap(); + const fileCount = Object.keys(fileMap || {}).length; + console.info(`[generateDiffPackageMap] Manifest contains ${fileCount} files`); + const packagesToDiff: storageTypes.Package[] = this.getPackagesToDiff( history, newPackage.appVersion, newPackage.packageHash, newPackage.label ); + + console.info(`[generateDiffPackageMap] Found ${packagesToDiff?.length || 0} packages to diff against`); + const diffBlobInfoPromises: Promise[] = []; if (packagesToDiff) { packagesToDiff.forEach((appPackage: storageTypes.Package) => { + console.info(`[generateDiffPackageMap] Adding diff task for package ${appPackage.label}, hash: ${appPackage.packageHash}`); diffBlobInfoPromises.push( this.uploadAndGetDiffBlobInfo(accountId, appPackage, newPackage.packageHash, newManifest, newFilePath) ); @@ -84,117 +105,133 @@ export class PackageDiffer { // all done, delete the downloaded archive file. fs.unlinkSync(newFilePath); + console.info(`[generateDiffPackageMap] Processed ${diffBlobInfoList?.length || 0} diffs`); + if (diffBlobInfoList && diffBlobInfoList.length) { let diffPackageMap: storageTypes.PackageHashToBlobInfoMap = null; diffBlobInfoList.forEach((diffBlobInfo: DiffBlobInfo) => { if (diffBlobInfo && diffBlobInfo.blobInfo) { diffPackageMap = diffPackageMap || {}; diffPackageMap[diffBlobInfo.packageHash] = diffBlobInfo.blobInfo; + console.info(`[generateDiffPackageMap] Added diff for package hash ${diffBlobInfo.packageHash}, size: ${diffBlobInfo.blobInfo.size} bytes`); } }); + console.info(`[generateDiffPackageMap] Returning diff package map with ${Object.keys(diffPackageMap || {}).length} entries`); return diffPackageMap; } else { + console.info(`[generateDiffPackageMap] No diff packages were generated`); return q(null); } }) - .catch(diffErrorUtils.diffErrorHandler); + .catch((error) => { + console.error(`[generateDiffPackageMap] Error: ${error.message || error}`); + return diffErrorUtils.diffErrorHandler(error); + }); } public generateDiffArchive(oldManifest: PackageManifest, newManifest: PackageManifest, newArchiveFilePath: string): Promise { return Promise( (resolve: (value?: string | Promise) => void, reject: (reason: any) => void, notify: (progress: any) => void): void => { + console.info(`[generateDiffArchive] Starting generation`); + if (!oldManifest || !newManifest) { + console.info(`[generateDiffArchive] Missing manifests - oldManifest: ${!!oldManifest}, newManifest: ${!!newManifest}`); resolve(null); return; } const diff: IArchiveDiff = PackageDiffer.generateDiff(oldManifest.toMap(), newManifest.toMap()); + console.info(`[generateDiffArchive] Generated diff - deletedFiles: ${diff.deletedFiles.length}, newOrUpdatedFiles: ${diff.newOrUpdatedEntries.size}`); if (diff.deletedFiles.length === 0 && diff.newOrUpdatedEntries.size === 0) { + console.info(`[generateDiffArchive] No differences found, skipping archive creation`); resolve(null); return; } PackageDiffer.ensureWorkDirectoryExists(); + console.info(`[generateDiffArchive] Work directory exists`); const diffFilePath = path.join(PackageDiffer.WORK_DIRECTORY_PATH, "diff_" + PackageDiffer.randomString(20) + ".zip"); + console.info(`[generateDiffArchive] Will create diff archive at: ${diffFilePath}`); + const writeStream: stream.Writable = fs.createWriteStream(diffFilePath); const diffFile = new yazl.ZipFile(); diffFile.outputStream.pipe(writeStream).on("close", (): void => { + console.info(`[generateDiffArchive] Successfully created diff archive: ${diffFilePath}`); resolve(diffFilePath); }); const json: string = JSON.stringify({ deletedFiles: diff.deletedFiles }); + console.info(`[generateDiffArchive] Added manifest with ${diff.deletedFiles.length} deleted files`); const readStream: stream.Readable = streamifier.createReadStream(json); diffFile.addReadStream(readStream, PackageDiffer.MANIFEST_FILE_NAME); - if (diff.newOrUpdatedEntries.size > 0) { - yauzl.open(newArchiveFilePath, (error?: any, zipFile?: yauzl.ZipFile): void => { - if (error) { - reject(error); - return; - } + // Iterate through the diff entries to avoid memory overrun when dealing + // with large files + console.info(`[generateDiffArchive] Opening source archive: ${newArchiveFilePath}`); + yauzl.open(newArchiveFilePath, { lazyEntries: true }, (err?: Error, zipFile?: any): void => { + if (err) { + console.error(`[generateDiffArchive] Error opening source archive: ${err.message}`); + reject(err); + return; + } - zipFile - .on("error", (error: any): void => { - reject(error); - }) - .on("entry", (entry: yauzl.IEntry): void => { - if ( - !PackageDiffer.isEntryInMap(entry.fileName, /*hash*/ null, diff.newOrUpdatedEntries, /*requireContentMatch*/ false) - ) { - return; - } else if (/\/$/.test(entry.fileName)) { - // this is a directory - diffFile.addEmptyDirectory(entry.fileName); + zipFile.readEntry(); + zipFile + .on("entry", (entry: any): void => { + // Skip processing non-file entries. + if (entry.fileName.endsWith("/")) { + zipFile.readEntry(); + return; + } + + const fileName: string = PackageManifest.normalizePath(entry.fileName); + // Skip processing files that don't match certain conditions. + if (PackageManifest.isIgnored(fileName)) { + zipFile.readEntry(); + return; + } + + // Add an updated file to the diff zip file if it's new or changed + for (const [fileNameHash, fileHash] of diff.newOrUpdatedEntries) { + if (fileNameHash === fileName) { + zipFile.openReadStream(entry, (entryErr?: Error, readStream?: stream.Readable): void => { + if (entryErr) { + console.error(`[generateDiffArchive] Error opening read stream for entry ${fileName}: ${entryErr.message}`); + reject(entryErr); + return; + } + + console.info(`[generateDiffArchive] Adding updated/new file to diff: ${fileName}`); + diffFile.addReadStream(readStream, fileName); + diff.newOrUpdatedEntries.delete(fileNameHash); + zipFile.readEntry(); + }); return; } + } - let readStreamCounter = 0; // Counter to track the number of read streams - let readStreamError = null; // Error flag for read streams - - zipFile.openReadStream(entry, (error?: any, readStream?: stream.Readable): void => { - if (error) { - reject(error); - return; - } - - readStreamCounter++; - - readStream - .on("error", (error: any): void => { - readStreamError = error; - reject(error); - }) - .on("end", (): void => { - readStreamCounter--; - if (readStreamCounter === 0 && !readStreamError) { - // All read streams have completed successfully - resolve(); - } - }); - - diffFile.addReadStream(readStream, entry.fileName); - }); - - zipFile.on("close", (): void => { - if (readStreamCounter === 0) { - // All read streams have completed, no need to wait - if (readStreamError) { - reject(readStreamError); - } else { - diffFile.end(); - resolve(); - } - } - }); - }); - }); - } else { - diffFile.end(); - } + zipFile.readEntry(); + }) + .on("end", (): void => { + if (diff.newOrUpdatedEntries.size > 0) { + console.warn(`[generateDiffArchive] Warning: ${diff.newOrUpdatedEntries.size} files in diff were not found in the archive`); + for (const [fileName, _] of diff.newOrUpdatedEntries) { + console.warn(`[generateDiffArchive] - Missing file: ${fileName}`); + } + } + + console.info(`[generateDiffArchive] Finalizing diff archive`); + diffFile.end(); + }) + .on("error", (zipErr?: Error): void => { + console.error(`[generateDiffArchive] Error processing zip: ${zipErr.message}`); + reject(zipErr); + }); + }); } ); } @@ -246,52 +283,109 @@ export class PackageDiffer { newManifest: PackageManifest, newFilePath: string ): Promise { + console.info(`[uploadAndGetDiffBlobInfo] Starting for package ${appPackage.label}, hash: ${appPackage.packageHash}`); + if (!appPackage || appPackage.packageHash === newPackageHash) { // If the packageHash matches, no need to calculate diff, its the same package. + console.info(`[uploadAndGetDiffBlobInfo] Skipping - ${!appPackage ? 'no package' : 'same hash'}`); return q(null); } + console.info(`[uploadAndGetDiffBlobInfo] Getting manifest for package ${appPackage.label}`); return this.getManifest(appPackage) .then((existingManifest?: PackageManifest) => { + console.info(`[uploadAndGetDiffBlobInfo] Got manifest for old package: ${existingManifest ? 'yes' : 'no'}`); + if (existingManifest) { + const fileCount = Object.keys(existingManifest.toMap() || {}).length; + console.info(`[uploadAndGetDiffBlobInfo] Old manifest contains ${fileCount} files`); + } + + console.info(`[uploadAndGetDiffBlobInfo] Generating diff archive`); return this.generateDiffArchive(existingManifest, newManifest, newFilePath); }) .then((diffArchiveFilePath?: string): Promise => { + console.info(`[uploadAndGetDiffBlobInfo] Diff archive generated: ${diffArchiveFilePath ? 'yes' : 'no'}`); + if (diffArchiveFilePath) { + console.info(`[uploadAndGetDiffBlobInfo] Uploading diff archive blob`); return this.uploadDiffArchiveBlob(security.generateSecureKey(accountId), diffArchiveFilePath); } + console.info(`[uploadAndGetDiffBlobInfo] No diff archive to upload`); return q(null); }) .then((blobInfo: storageTypes.BlobInfo) => { if (blobInfo) { + console.info(`[uploadAndGetDiffBlobInfo] Uploaded blob, size: ${blobInfo.size} bytes`); return { packageHash: appPackage.packageHash, blobInfo: blobInfo }; } else { + console.info(`[uploadAndGetDiffBlobInfo] No blob info available`); return q(null); } + }) + .catch((error) => { + console.error(`[uploadAndGetDiffBlobInfo] Error: ${error.message || error}`); + return null; }); } private getManifest(appPackage: storageTypes.Package): Promise { return Promise( (resolve: (manifest: PackageManifest) => void, reject: (error: any) => void, notify: (progress: any) => void): void => { + console.info(`[getManifest] Starting for package ${appPackage?.label || 'unknown'}`); + if (!appPackage || !appPackage.manifestBlobUrl) { + console.info(`[getManifest] No package or manifest URL, returning null`); resolve(null); return; } - const req: superagent.Request = superagent.get(appPackage.manifestBlobUrl); - const writeStream = new stream.Writable(); - let json = ""; + console.info(`[getManifest] Downloading manifest from URL: ${appPackage.manifestBlobUrl}`); + + const req: superagent.Request = superagent + .get(appPackage.manifestBlobUrl) + .buffer(true) + .parse(superagent.parse.text); - writeStream._write = (data: string | Buffer, encoding: string, callback: Function): void => { - json += (data).toString("utf8"); - callback(); - }; + req.end((err, res) => { + if (err) { + console.error(`[getManifest] Error downloading manifest: ${err.message}`); + resolve(null); + return; + } - req.pipe(writeStream).on("finish", () => { - const manifest: PackageManifest = PackageManifest.deserialize(json); + if (!res.text) { + console.error(`[getManifest] Manifest download succeeded but no text content received`); + console.info(`[getManifest] Response status: ${res.status}, type: ${res.type}, headers:`, res.headers); + resolve(null); + return; + } - resolve(manifest); + try { + console.info(`[getManifest] Downloaded manifest (${res.text.length} bytes), content-type: ${res.type}`); + console.info(`[getManifest] Manifest first 200 chars: ${res.text.substring(0, 200)}...`); + + const manifest = PackageManifest.deserialize(res.text); + if (manifest) { + const fileMap = manifest.toMap(); + const fileCount = Object.keys(fileMap || {}).length; + console.info(`[getManifest] Successfully parsed manifest with ${fileCount} files`); + if (fileCount > 0) { + const sampleFiles = Object.keys(fileMap).slice(0, 3); + console.info(`[getManifest] Sample files: ${sampleFiles.join(', ')}${fileCount > 3 ? '...' : ''}`); + } else { + console.warn(`[getManifest] Warning: Manifest contains zero files`); + } + } else { + console.error(`[getManifest] Manifest deserialization returned null`); + } + + resolve(manifest); + } catch (e) { + console.error(`[getManifest] Error parsing manifest: ${e.message}`); + console.info(`[getManifest] Raw content that failed parsing: ${res.text.substring(0, 500)}...`); + resolve(null); + } }); } ); @@ -322,32 +416,51 @@ export class PackageDiffer { newPackageHash: string, newPackageLabel: string ): storageTypes.Package[] { + console.info(`[getPackagesToDiff] Starting with ${history?.length || 0} packages in history`); + console.info(`[getPackagesToDiff] App version: ${appVersion}, new hash: ${newPackageHash}, new label: ${newPackageLabel}`); + if (!history || !history.length) { + console.info(`[getPackagesToDiff] No history available`); return null; } - // We assume that the new package has been released and already is in history. - // Only pick the packages that are released before the new package to generate diffs. - let foundNewPackageInHistory: boolean = false; - const validPackages: storageTypes.Package[] = []; - for (let i = history.length - 1; i >= 0; i--) { - if (!foundNewPackageInHistory) { - foundNewPackageInHistory = history[i].label === newPackageLabel; - continue; + // Only diff with packages of the same app version. + // Ignore already processed diffs. + const matchingAppVersionPackages: storageTypes.Package[] = history.filter((item: storageTypes.Package) => { + const matchingVersion = PackageDiffer.isMatchingAppVersion(item.appVersion, appVersion); + const alreadyHasDiff = item.diffPackageMap && item.diffPackageMap[newPackageHash]; + + if (!matchingVersion) { + console.info(`[getPackagesToDiff] Skipping package ${item.label} - app version mismatch: ${item.appVersion} vs ${appVersion}`); } - - if (validPackages.length === this._maxPackagesToDiff) { - break; + if (alreadyHasDiff) { + console.info(`[getPackagesToDiff] Skipping package ${item.label} - already has diff for hash ${newPackageHash}`); } + + return matchingVersion && !alreadyHasDiff; + }); - const isMatchingAppVersion: boolean = PackageDiffer.isMatchingAppVersion(appVersion, history[i].appVersion); - if (isMatchingAppVersion && history[i].packageHash !== newPackageHash) { - validPackages.push(history[i]); - } + console.info(`[getPackagesToDiff] Found ${matchingAppVersionPackages.length} packages with matching app version`); + + if (matchingAppVersionPackages.length) { + // Sort packages by uploadTime in descending order to get newest packages first + const sortedPackages = matchingAppVersionPackages.sort((a, b) => { + return b.uploadTime - a.uploadTime; + }); + + const maxPackagesToDiff = Math.min(this._maxPackagesToDiff, sortedPackages.length); + const packagesToProcess = sortedPackages.slice(0, maxPackagesToDiff); + + console.info(`[getPackagesToDiff] Will diff against ${packagesToProcess.length} packages (max limit: ${this._maxPackagesToDiff})`); + packagesToProcess.forEach(pkg => { + console.info(`[getPackagesToDiff] - Package to diff: ${pkg.label}, hash: ${pkg.packageHash}`); + }); + + return packagesToProcess; } - // maintain the order of release. - return validPackages.reverse(); + console.info(`[getPackagesToDiff] No suitable packages found for diffing`); + return null; } private static generateDiff(oldFileHashes: Map, newFileHashes: Map): IArchiveDiff { @@ -388,13 +501,8 @@ export class PackageDiffer { } private static ensureWorkDirectoryExists(): void { - if (!PackageDiffer.IS_WORK_DIRECTORY_CREATED) { - if (!fs.existsSync(PackageDiffer.WORK_DIRECTORY_PATH)) { - fs.mkdirSync(PackageDiffer.WORK_DIRECTORY_PATH); - } - - // Memoize this check to avoid unnecessary file system access. - PackageDiffer.IS_WORK_DIRECTORY_CREATED = true; + if (!fs.existsSync(PackageDiffer.WORK_DIRECTORY_PATH)) { + fs.mkdirSync(PackageDiffer.WORK_DIRECTORY_PATH); } }