fix short uuid and status race issues

idaholab · Oct 21, 2024 · b179d19 · b179d19
1 parent 758f064
commit b179d19
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 41 deletions.
diff --git a/server/src/data_access_layer/mappers/data_warehouse/data/file_mapper.ts b/server/src/data_access_layer/mappers/data_warehouse/data/file_mapper.ts
@@ -355,10 +355,11 @@ export default class FileMapper extends Mapper {
         };
     }
 
-    // fetch the fully qualified file path complete with file name and short uuid
+    // fetch the fully qualified file path complete with file name and short uuid (if present)
     private filePathMetadataStatement(fileIDs: string[]): QueryConfig {
         const text = `SELECT id,
-                        short_uuid || file_name AS file_name,
+                        CASE WHEN short_uuid IS NULL THEN file_name
+	                        ELSE short_uuid || file_name END AS file_name,
                         TRIM('/\\' FROM adapter_file_path) AS access_path
                         FROM files
                         WHERE id IN (%L)`;

diff --git a/server/src/data_access_layer/repositories/data_warehouse/data/report_query_repository.ts b/server/src/data_access_layer/repositories/data_warehouse/data/report_query_repository.ts
@@ -141,6 +141,14 @@ export default class ReportQueryRepository extends Repository implements Reposit
             return Promise.resolve(Result.Failure(`error: unsupported or unimplemented file storage method being used`));
         }
 
+        // set report status to "processing"
+        let statusSet = await this.#reportRepo.setStatus(
+            reportID, 'processing',
+            `executing query ${queryID}: "${reportQuery.query}" as part of report ${reportID}`
+        );
+        if (statusSet.isError) {return Promise.resolve(Result.Failure(`unable to set report status`))}
+
+        // kick off the describe or query process
         if (describe) {
             this.processDescribe(reportID, request.query!, storageConnection, files as FileMetadata[]);
         } else {
@@ -154,13 +162,6 @@ export default class ReportQueryRepository extends Repository implements Reposit
             );
         }
 
-        // set report status to "processing"
-        let statusSet = await this.#reportRepo.setStatus(
-            reportID, 'processing',
-            `executing query ${queryID}: "${reportQuery.query}" as part of report ${reportID}`
-        );
-        if (statusSet.isError) {return Promise.resolve(Result.Failure(`unable to set report status`))}
-
         // return report ID to the user so they can poll for results
         return Promise.resolve(Result.Success(reportID));
     }

diff --git a/server/src/services/blob_storage/azure_blob_impl.ts b/server/src/services/blob_storage/azure_blob_impl.ts
@@ -214,25 +214,30 @@ export default class AzureBlobImpl implements BlobStorage {
     }
 
     async renameFile(f: File): Promise<Result<boolean>> {
-        const newFileClient = this._ContainerClient.getBlockBlobClient(`${f.adapter_file_path}${f.short_uuid}${f.file_name}`);
-        const oldFileClient = this._ContainerClient.getBlockBlobClient(`${f.adapter_file_path}${f.file_name}${f.short_uuid}`);
+        // only perform the rename if uuid is present; otherwise, file should already be accessible
+        if (!f.short_uuid) {
+            return Promise.resolve(Result.Success(true));
+        } else {
+            const newFileClient = this._ContainerClient.getBlockBlobClient(`${f.adapter_file_path}${f.short_uuid}${f.file_name}`);
+            const oldFileClient = this._ContainerClient.getBlockBlobClient(`${f.adapter_file_path}${f.file_name}${f.short_uuid}`);
 
-        try {
-            const copyPoller = await newFileClient.beginCopyFromURL(oldFileClient.url);
-            const copy_res = await copyPoller.pollUntilDone();
+            try {
+                const copyPoller = await newFileClient.beginCopyFromURL(oldFileClient.url);
+                const copy_res = await copyPoller.pollUntilDone();
 
-            if (copy_res._response.status === 201 || copy_res._response.status === 202) {
-                const delete_res = await oldFileClient.delete();
+                if (copy_res._response.status === 201 || copy_res._response.status === 202) {
+                    const delete_res = await oldFileClient.delete();
 
-                if (delete_res._response.status === 201 || delete_res._response.status === 202) {
-                    return Promise.resolve(Result.Success(true));
+                    if (delete_res._response.status === 201 || delete_res._response.status === 202) {
+                        return Promise.resolve(Result.Success(true));
+                    }
                 }
+            } catch (e) {
+                Logger.error(`azure rename blob error: ${e}`);
+                return Promise.resolve(Result.Success(false));
             }
-        } catch (e) {
-            Logger.error(`azure rename blob error: ${e}`);
+
             return Promise.resolve(Result.Success(false));
         }
-
-        return Promise.resolve(Result.Success(false));
     }
 }
diff --git a/server/src/services/blob_storage/filesystem_impl.ts b/server/src/services/blob_storage/filesystem_impl.ts
@@ -172,27 +172,32 @@ export default class Filesystem implements BlobStorage {
     }
 
     renameFile(f: File): Promise<Result<boolean>> {
-        try {
-            /* eslint-disable-next-line security/detect-non-literal-fs-filename --
-             * TypeScript wants to guard against malicious file renaming,
-             * but since the rename is generated server-side and not by the end user,
-             * there is no security risk
-            **/
-            const rename_res = fs.rename(
-                `${f.adapter_file_path}${f.file_name}${f.short_uuid}`,
-                `${f.adapter_file_path}${f.short_uuid}${f.file_name}`,
-                (err) => {
-                    if (err) throw err;
-            });
-
-            if (rename_res === null || rename_res === undefined) {
-                return Promise.resolve(Result.Success(true));
+        // only rename file if short uuid is present; otherwise, file should already be accessible
+        if (!f.short_uuid) {
+            return Promise.resolve(Result.Success(true));
+        } else {
+            try {
+                /* eslint-disable-next-line security/detect-non-literal-fs-filename --
+                * TypeScript wants to guard against malicious file renaming,
+                * but since the rename is generated server-side and not by the end user,
+                * there is no security risk
+                **/
+                const rename_res = fs.rename(
+                    `${f.adapter_file_path}${f.file_name}${f.short_uuid}`,
+                    `${f.adapter_file_path}${f.short_uuid}${f.file_name}`,
+                    (err) => {
+                        if (err) throw err;
+                });
+
+                if (rename_res === null || rename_res === undefined) {
+                    return Promise.resolve(Result.Success(true));
+                }
+            } catch (e) {
+                Logger.error(`filesystem rename error: ${e}`);
+                return Promise.resolve(Result.Success(false));
             }
-        } catch (e) {
-            Logger.error(`filesystem rename error: ${e}`);
+
             return Promise.resolve(Result.Success(false));
         }
-
-        return Promise.resolve(Result.Success(false));
     }
 }