From 2aadfa1b49b797c4a2fcf13039a38a691f651b54 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 15 Oct 2025 17:27:29 +0100 Subject: [PATCH] Bug 38489: (follow-up) Improve idempotency of update We needed better idempotency around checking for fields before attempting to use them during migration and then dropping them. --- installer/data/mysql/db_revs/250600022.pl | 151 +++++++++++----------- 1 file changed, 79 insertions(+), 72 deletions(-) diff --git a/installer/data/mysql/db_revs/250600022.pl b/installer/data/mysql/db_revs/250600022.pl index 18ec9ac2573..057d95f93fd 100755 --- a/installer/data/mysql/db_revs/250600022.pl +++ b/installer/data/mysql/db_revs/250600022.pl @@ -41,10 +41,17 @@ return { "Added 'local' transport type to file_transports table" ); + # Migrate then drop the old transport-related columns from vendor_edi_accounts + my @columns_to_drop = + qw(host username password upload_port download_port upload_directory download_directory transport); + + my $all_exist = @columns_to_drop == grep { column_exists( 'vendor_edi_accounts', "$_" ) } @columns_to_drop; + # Migrate existing EDI transport configurations to file_transports - my $migration_count = 0; - my $edi_accounts_sth = $dbh->prepare( - q{ + if ($all_exist) { + my $migration_count = 0; + my $edi_accounts_sth = $dbh->prepare( + q{ SELECT id, description, host, username, password, upload_port, download_port, upload_directory, download_directory, transport FROM vendor_edi_accounts @@ -52,91 +59,91 @@ return { AND host IS NOT NULL AND host != '' } - ); - $edi_accounts_sth->execute(); + ); + $edi_accounts_sth->execute(); - my $insert_transport_sth = $dbh->prepare( - q{ + my $insert_transport_sth = $dbh->prepare( + q{ INSERT INTO file_transports (name, host, port, transport, user_name, password, upload_directory, download_directory, auth_mode, passive, debug) VALUES (?, ?, ?, ?, ?, ?, ?, ?, 'password', 1, 0) } - ); + ); - my $update_edi_account_sth = $dbh->prepare( - q{ + my $update_edi_account_sth = $dbh->prepare( + q{ UPDATE vendor_edi_accounts SET file_transport_id = ? WHERE id = ? } - ); + ); - while ( my $row = $edi_accounts_sth->fetchrow_hashref ) { - - # Determine appropriate port (default to SFTP=22, FTP=21, Local=NULL) - my $port = $row->{upload_port} || $row->{download_port}; - unless ($port) { - if ( uc( $row->{transport} ) eq 'SFTP' ) { - $port = 22; - } elsif ( uc( $row->{transport} ) eq 'FILE' ) { - $port = undef; # Local transport doesn't use ports - } else { - $port = 21; # FTP default + while ( my $row = $edi_accounts_sth->fetchrow_hashref ) { + + # Determine appropriate port + # (default to SFTP=22, FTP=21, Local=NULL) + my $port = $row->{upload_port} || $row->{download_port}; + unless ($port) { + if ( uc( $row->{transport} ) eq 'SFTP' ) { + $port = 22; + } elsif ( uc( $row->{transport} ) eq 'FILE' ) { + $port = undef; # Local transport doesn't use ports + } else { + $port = 21; # FTP default + } } - } - # Map transport type (normalize case and handle FILE -> local) - my $transport_type = lc( $row->{transport} ); - $transport_type = 'local' if $transport_type eq 'file'; - $transport_type = 'ftp' - unless $transport_type =~ /^(sftp|local)$/; - - # Create transport name from EDI account description - my $transport_name = sprintf( "EDI Transport for %s", $row->{description} ); - - # Handle host for local transport - my $host = $row->{host}; - $host = 'localhost' if $transport_type eq 'local' && !$host; - - # Insert new file transport - $insert_transport_sth->execute( - $transport_name, - $host, - $port, - $transport_type, - $row->{username}, - $row->{password}, # Password is already encrypted in EDI accounts - $row->{upload_directory}, - $row->{download_directory} - ); - - my $transport_id = $dbh->last_insert_id( - undef, undef, 'file_transports', - 'file_transport_id' - ); + # Map transport type + # (normalize case and handle FILE -> local) + my $transport_type = lc( $row->{transport} ); + $transport_type = 'local' if $transport_type eq 'file'; + $transport_type = 'ftp' + unless $transport_type =~ /^(sftp|local)$/; + + # Create transport name from EDI account description + my $transport_name = sprintf( "EDI Transport for %s", $row->{description} ); + + # Handle host for local transport + my $host = $row->{host}; + $host = 'localhost' if $transport_type eq 'local' && !$host; + + # Insert new file transport + $insert_transport_sth->execute( + $transport_name, + $host, + $port, + $transport_type, + $row->{username}, + $row->{password}, # Password is already encrypted in EDI accounts + $row->{upload_directory}, + $row->{download_directory} + ); - # Update EDI account to reference the new transport - $update_edi_account_sth->execute( $transport_id, $row->{id} ); + my $transport_id = $dbh->last_insert_id( + undef, undef, 'file_transports', + 'file_transport_id' + ); - $migration_count++; - } + # Update EDI account to reference the new transport + $update_edi_account_sth->execute( + $transport_id, + $row->{id} + ); - if ( $migration_count > 0 ) { - say_success( - $out, - "Successfully migrated $migration_count EDI transport configurations to file_transports" - ); - } else { - say_info( - $out, - "No EDI transport configurations found to migrate" - ); - } + $migration_count++; + } - # Drop the old transport-related columns from vendor_edi_accounts - my @columns_to_drop = - qw(host username password upload_port download_port upload_directory download_directory transport); + if ( $migration_count > 0 ) { + say_success( + $out, + "Successfully migrated $migration_count EDI transport configurations to file_transports" + ); + } else { + say_info( + $out, + "No EDI transport configurations found to migrate" + ); + } - for my $column (@columns_to_drop) { - if ( column_exists( 'vendor_edi_accounts', $column ) ) { + for my $column (@columns_to_drop) { $dbh->do("ALTER TABLE vendor_edi_accounts DROP COLUMN $column"); say_success( $out, -- 2.51.0