Skip to content

Studio: Handle duplicate SQL files in tar.gz archives - #793

Merged
ivan-ottinger merged 2 commits into
trunkfrom
fix/duplicate-file-paths-generated-in-listfiles
Jan 9, 2025
Merged

Studio: Handle duplicate SQL files in tar.gz archives#793
ivan-ottinger merged 2 commits into
trunkfrom
fix/duplicate-file-paths-generated-in-listfiles

Conversation

@ivan-ottinger

@ivan-ottinger ivan-ottinger commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

Related issues

Proposed Changes

When importing tar.gz archives that contain duplicate SQL files, we will now ensure each file is processed only once by using a Set for deduplication. During extraction, tar's natural behavior will use the last occurrence of each file.

Details can be found in the following comment: https://github.com/Automattic/dotcom-forge/issues/9029#issuecomment-2578098289.

  • ensure array returned by listFiles does not include any file path duplicates

Testing Instructions

  1. Try to reproduce the https://github.com/Automattic/dotcom-forge/issues/9029 issue:
  2. Head over to the Import tab and try to import the following tar.gz archive: https://drive.google.com/file/d/1XgU0NZFUeuCLXnT9Zr-CAbArQpAkDQIh/view?usp=drive_link.
  3. The process should finish correctly and the imported site should work as expected.
  4. There should be no regressions when importing any other files (e.g. zip exports).

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@ivan-ottinger ivan-ottinger self-assigned this Jan 9, 2025
@ivan-ottinger ivan-ottinger changed the title Studio: Ensure array returned by listFiles does not include any file path duplicates Jan 9, 2025
@ivan-ottinger
ivan-ottinger requested a review from a team January 9, 2025 09:49

@fredrikekelund fredrikekelund left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! LGTM and works as expected with the tarball you provided 👍

@ivan-ottinger
ivan-ottinger merged commit f8c5420 into trunk Jan 9, 2025
@ivan-ottinger
ivan-ottinger deleted the fix/duplicate-file-paths-generated-in-listfiles branch January 9, 2025 13:29
export class BackupHandlerTarGz extends EventEmitter implements BackupHandler {
async listFiles( backup: BackupArchiveInfo ): Promise< string[] > {
const files: string[] = [];
const filesSet = new Set< string >();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nice use of the built-in Set object to ensure entries are unique!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Wojtek! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants