Bugzilla – Attachment 187925 Details for
Bug 38489
EDI should be updated to use the new FTP/SFTP Servers management page
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 38489: (follow-up) Improve idempotency of update
Bug-38489-follow-up-Improve-idempotency-of-update.patch (text/plain), 8.65 KB, created by
Martin Renvoize (ashimema)
on 2025-10-15 16:28:45 UTC
(
hide
)
Description:
Bug 38489: (follow-up) Improve idempotency of update
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-10-15 16:28:45 UTC
Size:
8.65 KB
patch
obsolete
>From 2aadfa1b49b797c4a2fcf13039a38a691f651b54 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 38489
:
175204
|
175205
|
175206
|
175207
|
176118
|
176119
|
176161
|
185324
|
186819
|
186820
|
186845
|
186846
|
186847
|
186848
|
186849
|
186850
|
186851
|
186852
|
186854
|
187080
|
187081
|
187082
|
187083
|
187084
|
187085
|
187086
|
187087
|
187088
|
187089
|
187108
|
187147
|
187148
|
187149
|
187150
|
187151
|
187152
|
187153
|
187154
|
187155
|
187156
|
187157
|
187158
|
187159
|
187270
|
187271
|
187272
|
187273
|
187274
|
187275
|
187276
|
187277
|
187278
|
187279
|
187280
|
187281
|
187283
|
187284
|
187285
|
187286
|
187287
|
187288
|
187289
|
187290
|
187291
|
187292
|
187293
|
187294
|
187861
|
187862
|
187863
|
187864
|
187865
|
187866
|
187867
|
187868
|
187869
|
187870
|
187871
|
187872
|
187873
|
187897
|
187916
|
187918
|
187923
|
187924
| 187925 |
187926