saturdaymp / Migrator

Migrate database changes.
https://saturdaymp.com
MIT License
1 stars 0 forks source link

Update SMO and .NET framework #2

Closed mrbiggred closed 5 months ago

mrbiggred commented 5 months ago

Updated the SMO objects to the latest version and the .NET Framework for 4.7.2. This matches the version of the framework a client is using so hopefully I can use this to run migrations.

sweep-ai[bot] commented 5 months ago

Sweep: PR Review

Source/Migrator.sln

The changes remove the "Solution Items" project and add a new "SolutionFolder" project that includes a LICENSE and README.md file.


Source/Migrator/App.config

The pull request adds a new App.config file with binding redirects for multiple assemblies to ensure consistent version usage.


Source/Migrator/Migration.cs

The code changes replace the System.Data.SqlClient library with the Microsoft.Data.SqlClient library for SQL Server operations.


Source/Migrator/Migrator.csproj

The changes update the target framework, replace and add several assembly references, include necessary configuration files, and add build process safeguards to ensure required NuGet packages are available.


Source/Migrator/Schema.cs

The change replaces the System.Data.SqlClient namespace with the Microsoft.Data.SqlClient namespace to use a newer SQL client library.

Sweep Found These Issues

  • Switching to Microsoft.Data.SqlClient may introduce compatibility issues if there are any dependencies or specific behaviors tied to System.Data.SqlClient.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigrator%2FSchema.cs#L3-L4 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-5d0bb9287d9e17c755ca25f56c848f1c12acb4cea27157f8647d8d766679faa5R3-R4)

Source/Migrator/packages.config

A new packages.config file was added to manage project dependencies.


Source/MigratorConsole/MigratorConsole.csproj

The target framework version was updated from .NET Framework 4.6 to .NET Framework 4.7.2.

Potential Issues

Sweep isn't 100% sure if the following are issues or not but they may be worth taking a look at.

  • Updating the target framework to 4.7.2 may cause compatibility issues if the deployment environment does not support .NET Framework 4.7.2.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigratorConsole%2FMigratorConsole.csproj#L13 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-29c90b821a9ae3babc2f995002b0476818403d17476da546400a0d1b1f7a4a0cR13)

Source/MigratorConsoleLib/MigratorConsoleLib.csproj

The project's target framework was updated from .NET Framework 4.6 to .NET Framework 4.7.2.


Source/MigratorConsoleLibTests/MigratorConsoleLibTests.csproj

The target framework version was updated from .NET Framework 4.6 to .NET Framework 4.7.2.


Source/MigratorGUI/App.config

The changes update the connection string to use explicit credentials, correct a key value, and add numerous assembly binding redirects.

Sweep Found These Issues

  • The connection string now includes hardcoded credentials (Uid=sa;Pwd=Password1234!), which introduces a security vulnerability by exposing sensitive information.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigratorGUI%2FApp.config#L4 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-b391d7e77476dd8aceef3ea40607008c2b7b8ddc87894b1950f2a739ea654e6fR4)
Potential Issues

Sweep isn't 100% sure if the following are issues or not but they may be worth taking a look at.

  • The connection string now includes hardcoded credentials (Uid=sa;Pwd=Password1234!), which is a security vulnerability.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigratorGUI%2FApp.config#L4 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-b391d7e77476dd8aceef3ea40607008c2b7b8ddc87894b1950f2a739ea654e6fR4)

Source/MigratorGUI/MigratorGUI.csproj

The changes update the target framework to 4.7.2, add numerous library references, include the packages.config file, and ensure necessary NuGet packages are available during the build process.

Potential Issues

Sweep isn't 100% sure if the following are issues or not but they may be worth taking a look at.

  • The inclusion of the Microsoft.Data.SqlClient.SNI.targets file and the EnsureNuGetPackageBuildImports target may cause build failures if the required NuGet packages are not restored properly.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigratorGUI%2FMigratorGUI.csproj#L208-L213 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-f1ada1478a86ef816702233004c2edd867002b657d10d0aded7cde8da04ddb44R208-R213)

Source/MigratorGUI/packages.config

The pull request adds a new packages.config file specifying dependencies on various Azure, Microsoft Identity, and system libraries targeting .NET Framework 4.7.2.


Source/MigratorTests/App.config

The pull request modifies the database connection string to use explicit credentials and adds binding redirects for multiple assemblies to manage dependencies.

Sweep Found These Issues

  • The new connection string includes hardcoded credentials (username and password), which is a security vulnerability.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigratorTests%2FApp.config#L4 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-036adf66c426c9f25d7dc0dae81ddd7c6dea789d8d4f827dad23843c4291221eR4)

Source/MigratorTests/MigratorTests.csproj

The changes update the target framework to v4.7.2, replace older SQL Server library references with newer versions from NuGet, add new references to various Azure and Microsoft libraries, include a new reference to the "WindowsBase" assembly, and add a target to ensure all necessary NuGet packages are present before building.


Source/MigratorTests/Schema.sql

The changes include cosmetic adjustments, the addition of a block to create the AddressView view if it does not exist, the removal and relocation of the block to create the SchemaMigrations table, and minor formatting changes within stored procedures.

Potential Issues

Sweep isn't 100% sure if the following are issues or not but they may be worth taking a look at.

  • The removal of the block of code that checks for the existence of the AddressView view and creates it if it does not exist will prevent the AddressView view from being created, affecting any functionality relying on this view.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigratorTests%2FSchema.sql#L60-L72 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-cb29c050af8f0e6b1ba13c4732e58efbfd32d4b228a1b4176021ffa4363c0f83R60-R72)
  • The removal of the block of code that checks for the existence of the SchemaMigrations table and creates it if it does not exist will prevent the SchemaMigrations table from being created, affecting any functionality relying on this table.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigratorTests%2FSchema.sql#L60-L72 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-cb29c050af8f0e6b1ba13c4732e58efbfd32d4b228a1b4176021ffa4363c0f83R60-R72)

Source/MigratorTests/SchemaWriteSchemaToFileTests.cs

The regular expressions for removing blank lines were changed to only remove completely empty lines, and two unnecessary blank lines were removed.

Sweep Found These Issues

  • The new regular expression @"^\r?\n" may not remove lines that contain only whitespace characters, potentially causing schema comparison failures if such lines exist.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigratorTests%2FSchemaWriteSchemaToFileTests.cs#L97-L98 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-fbaedfa53e2a88a4a01a4c36022bfef36f8d06b44fe4f10805e64b4981383ff4R97-R98)

Source/MigratorTests/TestBase.cs

The code changes switch the SQL client library from System.Data.SqlClient to Microsoft.Data.SqlClient by commenting out the old directives and adding the new one.


Source/MigratorTests/packages.config

The pull request adds numerous new package dependencies targeting the net472 framework, enhancing the project's capabilities in SQL Server management, Azure functionalities, identity management, and other system utilities.

Potential Issues

Sweep isn't 100% sure if the following are issues or not but they may be worth taking a look at.

  • The addition of numerous new packages increases the complexity and potential for dependency conflicts, which could introduce runtime errors or compatibility issues.
  • https://github.com/saturdaymp/Migrator/blob/88c8e0173183131af7676eaaa1ae7298f66fdf11/Source%2FMigratorTests%2Fpackages.config#L3-L45 [View Diff](https://github.com/saturdaymp/Migrator/pull/2/files#diff-103caf533a8605b7f3fc649548d7d02d5385fe07518d279ef2c46a790b0cf6baR3-R45)