[desktop] Finalize zip handling (#1576)

And other fixes. Getting close to a stable desktop build.

desktop/src/main/services/dir.ts

@@ -17,8 +17,11 @@ export const selectDirectory = async () => {
  * For example, on macOS this'll open {@link dirPath} in Finder.
  */
 export const openDirectory = async (dirPath: string) => {
+    // We need to use `path.normalize` because `shell.openPath; does not support
+    // POSIX path, it needs to be a platform specific path:
+    // https://github.com/electron/electron/issues/28831#issuecomment-826370589
     const res = await shell.openPath(path.normalize(dirPath));
-    // shell.openPath resolves with a string containing the error message
+    // `shell.openPath` resolves with a string containing the error message
     // corresponding to the failure if a failure occurred, otherwise "".
     if (res) throw new Error(`Failed to open directory ${dirPath}: res`);
 };

desktop/src/main/services/upload.ts

@@ -14,7 +14,7 @@ export const listZipItems = async (zipPath: string): Promise<ZipItem[]> => {
     for (const entry of Object.values(entries)) {
         const basename = path.basename(entry.name);
         // Ignore "hidden" files (files whose names begins with a dot).
-        if (entry.isFile && basename.startsWith(".")) {
+        if (entry.isFile && !basename.startsWith(".")) {
             // `entry.name` is the path within the zip.
             entryNames.push(entry.name);
         }

desktop/src/main/services/watch.ts

@@ -35,8 +35,8 @@ export const createWatcher = (mainWindow: BrowserWindow) => {
     return watcher;
 };
 
-const eventData = (path: string): [string, FolderWatch] => {
-    path = posixPath(path);
+const eventData = (platformPath: string): [string, FolderWatch] => {
+    const path = posixPath(platformPath);
 
     const watch = folderWatches().find((watch) =>
         path.startsWith(watch.folderPath + "/"),

desktop/src/main/stores/upload-status.ts

@@ -9,15 +9,10 @@ export interface UploadStatusStore {
     collectionName?: string;
     /**
      * Paths to regular files that are pending upload.
-     *
-     * This should generally be present, albeit empty, but it is marked optional
-     * in sympathy with its siblings.
      */
     filePaths?: string[];
     /**
      * Each item is the path to a zip file and the name of an entry within it.
-     *
-     * This is marked optional since legacy stores will not have it.
      */
     zipItems?: [zipPath: string, entryName: string][];
     /**

desktop/src/main/stream.ts

@@ -48,7 +48,8 @@ export const registerStreamProtocol = () => {
         const { host, pathname, hash } = new URL(url);
         // Convert e.g. "%20" to spaces.
         const path = decodeURIComponent(pathname);
-        const hashPath = decodeURIComponent(hash);
+        // `hash` begins with a "#", slice that off.
+        const hashPath = decodeURIComponent(hash.slice(1));
         switch (host) {
             case "read":
                 return handleRead(path);
@@ -116,7 +117,6 @@ const handleReadZip = async (zipPath: string, entryName: string) => {
             webReadableStreamAny as ReadableStream<Uint8Array>;
 
         // Close the zip handle when the underlying stream closes.
-        // TODO(MR): Verify
         stream.on("end", () => void zip.close());
 
         return new Response(webReadableStream, {

desktop/src/main/utils/electron.ts

@@ -9,11 +9,26 @@ import log from "../log";
 export const isDev = !app.isPackaged;
 
 /**
- * Convert a file system {@link filePath} that uses the local system specific
- * path separators into a path that uses POSIX file separators.
+ * Convert a file system {@link platformPath} that uses the local system
+ * specific path separators into a path that uses POSIX file separators.
+ *
+ * For all paths that we persist or pass over the IPC boundary, we always use
+ * POSIX paths, even on Windows.
+ *
+ * Windows recognizes both forward and backslashes. This also works with drive
+ * names. c:\foo\bar and c:/foo/bar are both valid.
+ *
+ * > Almost all paths passed to Windows APIs are normalized. During
+ * > normalization, Windows performs the following steps: ... All forward
+ * > slashes (/) are converted into the standard Windows separator, the back
+ * > slash (\).
+ * >
+ * > https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats
  */
-export const posixPath = (filePath: string) =>
-    filePath.split(path.sep).join(path.posix.sep);
+export const posixPath = (platformPath: string) =>
+    path.sep == path.posix.sep
+        ? platformPath
+        : platformPath.split(path.sep).join(path.posix.sep);
 
 /**
  * Run a shell command asynchronously.

web/apps/photos/src/components/PhotoFrame.tsx

@@ -308,7 +308,7 @@ const PhotoFrame = ({
         item: EnteFile,
     ) => {
         log.info(
-            `[${item.id}] getSlideData called for thumbnail: ${!!item.msrc} sourceLoaded: ${item.isSourceLoaded} fetching:${fetching[item.id]}`,
+            `[${item.id}] getSlideData called for thumbnail: ${!!item.msrc} sourceLoaded: ${!!item.isSourceLoaded} fetching: ${!!fetching[item.id]}`,
         );
 
         if (!item.msrc) {

web/apps/photos/src/components/Upload/Uploader.tsx

@@ -261,7 +261,9 @@ export default function Uploader({
 
                 const { collectionName, filePaths, zipItems } = pending;
 
-                log.info("Resuming pending upload", pending);
+                log.info(
+                    `Resuming pending of upload of ${filePaths.length + zipItems.length} items${collectionName ? " to collection " + collectionName : ""}`,
+                );
                 isPendingDesktopUpload.current = true;
                 pendingDesktopUploadCollectionName.current = collectionName;
                 setDesktopFilePaths(filePaths);
@@ -323,13 +325,26 @@ export default function Uploader({
 
     // Trigger an upload when any of the dependencies change.
     useEffect(() => {
+        // Re the paths:
+        //
+        // - These are not necessarily the full paths. In particular, when
+        //   running on the browser they'll be the relative paths (at best) or
+        //   just the file-name otherwise.
+        //
+        // - All the paths use POSIX separators. See inline comments.
         const allItemAndPaths = [
             // See: [Note: webkitRelativePath]. In particular, they use POSIX
             // separators.
             webFiles.map((f) => [f, f.webkitRelativePath ?? f.name]),
+            // The paths we get from the desktop app all eventually come either
+            // from electron.selectDirectory or electron.pathForFile, both of
+            // which return POSIX paths.
             desktopFiles.map((fp) => [fp, fp.path]),
             desktopFilePaths.map((p) => [p, p]),
-            // ze[1], the entry name, uses POSIX separators.
+            // The first path, that of the zip file itself, is POSIX like the
+            // other paths we get over the IPC boundary. And the second path,
+            // ze[1], the entry name, uses POSIX separators because that is what
+            // the ZIP format uses.
             desktopZipItems.map((ze) => [ze, ze[1]]),
         ].flat() as [UploadItem, string][];
 
@@ -792,10 +807,7 @@ async function waitAndRun(
     await task();
 }
 
-const desktopFilesAndZipItems = async (
-    electron: Electron,
-    files: File[],
-): Promise<{ fileAndPaths: FileAndPath[]; zipItems: ZipItem[] }> => {
+const desktopFilesAndZipItems = async (electron: Electron, files: File[]) => {
     const fileAndPaths: FileAndPath[] = [];
     let zipItems: ZipItem[] = [];
 

web/apps/photos/src/services/exif.ts

@@ -167,14 +167,7 @@ function parseExifData(exifData: RawEXIFData): ParsedEXIFData {
             parsedExif.imageWidth = ImageWidth;
             parsedExif.imageHeight = ImageHeight;
         } else {
-            log.error(
-                `Image dimension parsing failed - ImageWidth or ImageHeight is not a number ${JSON.stringify(
-                    {
-                        ImageWidth,
-                        ImageHeight,
-                    },
-                )}`,
-            );
+            log.warn("EXIF: Ignoring non-numeric ImageWidth or ImageHeight");
         }
     } else if (ExifImageWidth && ExifImageHeight) {
         if (
@@ -184,13 +177,8 @@ function parseExifData(exifData: RawEXIFData): ParsedEXIFData {
             parsedExif.imageWidth = ExifImageWidth;
             parsedExif.imageHeight = ExifImageHeight;
         } else {
-            log.error(
-                `Image dimension parsing failed - ExifImageWidth or ExifImageHeight is not a number ${JSON.stringify(
-                    {
-                        ExifImageWidth,
-                        ExifImageHeight,
-                    },
-                )}`,
+            log.warn(
+                "EXIF: Ignoring non-numeric ExifImageWidth or ExifImageHeight",
             );
         }
     } else if (PixelXDimension && PixelYDimension) {
@@ -201,13 +189,8 @@ function parseExifData(exifData: RawEXIFData): ParsedEXIFData {
             parsedExif.imageWidth = PixelXDimension;
             parsedExif.imageHeight = PixelYDimension;
         } else {
-            log.error(
-                `Image dimension parsing failed - PixelXDimension or PixelYDimension is not a number ${JSON.stringify(
-                    {
-                        PixelXDimension,
-                        PixelYDimension,
-                    },
-                )}`,
+            log.warn(
+                "EXIF: Ignoring non-numeric PixelXDimension or PixelYDimension",
             );
         }
     }
@@ -302,15 +285,13 @@ export function parseEXIFLocation(
         );
         return { latitude, longitude };
     } catch (e) {
-        log.error(
-            `Failed to parseEXIFLocation ${JSON.stringify({
-                gpsLatitude,
-                gpsLatitudeRef,
-                gpsLongitude,
-                gpsLongitudeRef,
-            })}`,
-            e,
-        );
+        const p = {
+            gpsLatitude,
+            gpsLatitudeRef,
+            gpsLongitude,
+            gpsLongitudeRef,
+        };
+        log.error(`Failed to parse EXIF location ${JSON.stringify(p)}`, e);
         return { ...NULL_LOCATION };
     }
 }

web/apps/photos/src/services/export/index.ts

@@ -547,6 +547,9 @@ class ExportService {
         isCanceled: CancellationStatus,
     ) {
         const fs = ensureElectron().fs;
+        const rmdirIfExists = async (dirPath: string) => {
+            if (await fs.exists(dirPath)) await fs.rmdir(dirPath);
+        };
         try {
             const exportRecord = await this.getExportRecord(exportFolder);
             const collectionIDPathMap =
@@ -581,11 +584,11 @@ class ExportService {
                     );
                     try {
                         // delete the collection metadata folder
-                        await fs.rmdir(
+                        await rmdirIfExists(
                             getMetadataFolderExportPath(collectionExportPath),
                         );
                         // delete the collection folder
-                        await fs.rmdir(collectionExportPath);
+                        await rmdirIfExists(collectionExportPath);
                     } catch (e) {
                         await this.addCollectionExportedRecord(
                             exportFolder,
@@ -1398,17 +1401,19 @@ const moveToTrash = async (
 
     if (await fs.exists(filePath)) {
         await fs.mkdirIfNeeded(trashDir);
-        const trashFilePath = await safeFileName(trashDir, fileName, fs.exists);
+        const trashFileName = await safeFileName(trashDir, fileName, fs.exists);
+        const trashFilePath = `${trashDir}/${trashFileName}`;
         await fs.rename(filePath, trashFilePath);
     }
 
     if (await fs.exists(metadataFilePath)) {
         await fs.mkdirIfNeeded(metadataTrashDir);
-        const metadataTrashFilePath = await safeFileName(
+        const metadataTrashFileName = await safeFileName(
             metadataTrashDir,
             metadataFileName,
             fs.exists,
         );
-        await fs.rename(filePath, metadataTrashFilePath);
+        const metadataTrashFilePath = `${metadataTrashDir}/${metadataTrashFileName}`;
+        await fs.rename(metadataFilePath, metadataTrashFilePath);
     }
 };

web/apps/photos/src/services/upload/uploadService.ts

@@ -1021,7 +1021,7 @@ const withThumbnail = async (
                 fileTypeInfo,
             );
         } catch (e) {
-            if (e.message == CustomErrorMessage.NotAvailable) {
+            if (e.message.endsWith(CustomErrorMessage.NotAvailable)) {
                 moduleState.isNativeImageThumbnailGenerationNotAvailable = true;
             } else {
                 log.error("Native thumbnail generation failed", e);

web/apps/photos/src/utils/file/index.ts

@@ -301,7 +301,8 @@ export const getRenderableImage = async (fileName: string, imageBlob: Blob) => {
         const tempFile = new File([imageBlob], fileName);
         const fileTypeInfo = await detectFileTypeInfo(tempFile);
         log.debug(
-            () => `Need renderable image for ${JSON.stringify(fileTypeInfo)}`,
+            () =>
+                `Need renderable image for ${JSON.stringify({ fileName, ...fileTypeInfo })}`,
         );
         const { extension } = fileTypeInfo;
 
@@ -318,7 +319,7 @@ export const getRenderableImage = async (fileName: string, imageBlob: Blob) => {
             try {
                 return await nativeConvertToJPEG(imageBlob);
             } catch (e) {
-                if (e.message == CustomErrorMessage.NotAvailable) {
+                if (e.message.endsWith(CustomErrorMessage.NotAvailable)) {
                     moduleState.isNativeJPEGConversionNotAvailable = true;
                 } else {
                     log.error("Native conversion to JPEG failed", e);

web/apps/photos/src/utils/native-stream.ts

@@ -42,7 +42,7 @@ export const readStream = async (
         url = new URL(`stream://read${pathOrZipItem}`);
     } else {
         const [zipPath, entryName] = pathOrZipItem;
-        url = new URL(`stream://read${zipPath}`);
+        url = new URL(`stream://read-zip${zipPath}`);
         url.hash = entryName;
     }
 

web/packages/next/types/ipc.ts

@@ -53,6 +53,8 @@ export interface Electron {
      * Ask the user to select a directory on their local file system, and return
      * it path.
      *
+     * The returned path is guaranteed to use POSIX separators ('/').
+     *
      * We don't strictly need IPC for this, we can use a hidden <input> element
      * and trigger its click for the same behaviour (as we do for the
      * `useFileInput` hook that we use for uploads). However, it's a bit