Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PostgreSQL Support #367

Merged
merged 23 commits into from
Apr 26, 2023
Merged

PostgreSQL Support #367

merged 23 commits into from
Apr 26, 2023

Conversation

Bram1903
Copy link
Collaborator

@Bram1903 Bram1903 commented Apr 25, 2023

PostgreSQL Support

Features included in this PR:

  • PostgreSQL Support Additional database engine support #359
  • Improved search functionality on both the Users and Roles pages
  • Preventing an administrator from removing its own account (could lead to zero administrator accounts)

I added PostgreSQL support and tried to do it as generic as possible. To achieve support for PostgreSQL I added an additional setting within the appsettings.json.

Due to the nature of the PostgreSQL connection string being built up slightly differently than MS SQL Server, especially when not using any authentication I decided to create an additional connection string.

Updated appsettings.json:
image

Testing

I have tested this code both on a traditional x64 processor and on an Arm (M2 Max) processor.

Tested OS Versions:

  • Windows 11 (22H2)
  • MacOS (Ventura 13.3.1)

Tested Database Versions:

  • In memory database
  • MS SQL Server (Latest)
  • PostgreSQL (Latest)

Important Side Note

Since the migrations look different depending on the active database provider, migrations need to be made separately for both MS SQL Server and PostgreSQL.

I currently achieve this by setting Blazor.Server.UIas migration assembly, when using the PostgreSQL provider. This ends up creating migrations in a Migrations folder on the Blazor.Server.UI project. Obviously, this isn't considered "Clean Architecture".

Currently, I create migrations by executing dotnet ef migrations add migrationsName in the Blazor.Server.UI directory. I tried to create a new migration for MS SQL Server, in your traditional way, but I couldn't figure out how to create one. 😅
Most probably I looked over something, but if you could let me know how you normally create your migrations that would be great! :-)

I hope you could look into the best and cleanest way of storing two separate migrations for two different providers!

Even though the changes may look small, I had to spend quite some time researching how I could implement another database provider, without changing too much of the codebase. Luckily the PostgreSQL provider supports

AppContext.SetSwitch("Npgsql.EnableLegacyTimestampBehavior", true);

which handles all the datetime conversions between the MS SQL Server standard to the PostgreSQL standard.

If you have any questions feel free to ask them!

@neozhu
Copy link
Owner

neozhu commented Apr 25, 2023

@Bram1903
Thank you very much for participating in this project. I have reviewed the PR you submitted. I have an idea, is it possible to participate in this project https://github.com/fullstackhero/dotnet-webapi-boilerplate/tree/main/src/Migrators to separate the Migration of different database types and create a separate Project separately? I think this would be better. What do you think?

@neozhu
Copy link
Owner

neozhu commented Apr 26, 2023

I have done some refactoring in the PR #367. I haven't had the time to test it yet due to my unpredictable schedule. Thank you very much for your contribution. Let's work together to complete this feature.

@Bram1903
Copy link
Collaborator Author

Bram1903 commented Apr 26, 2023

I have done some refactoring in the PR #367. I haven't had the time to test it yet due to my unpredictable schedule. Thank you very much for your contribution. Let's work together to complete this feature.

I'm going to get some breakfast, and I'll take a look afterward. What still needs to be done, and how do you normally create migrations, as that might come in handy while working on this feature?

@neozhu
Copy link
Owner

neozhu commented Apr 26, 2023

Please test whether this project can run using a PostgreSQL database. Currently, I don't have the necessary database environment set up, but you can easily modify the parameters in the DatabaseSettings to support different databases. Good luck with the testing, and let me know if you have any questions about how to approach the migration process!

@Bram1903
Copy link
Collaborator Author

Bram1903 commented Apr 26, 2023

Sure. I have the testing environment setup anyways. I'll start testing with the current PR branch, and return feedback when needed. How do you normally create a migration?

@neozhu
Copy link
Owner

neozhu commented Apr 26, 2023

image

@Bram1903
Copy link
Collaborator Author

Bram1903 commented Apr 26, 2023

image

Yesterday I added the following line of code to solve this issue. Ideally, we only want to enable Npgsql.EnableLegacyTimestampBehavior when using PostgreSQL, or we could make all DateTimes UTC based rather than local. Obviously enabling Npgsql.EnableLegacyTimestampBehavior would be the easiest option. More information can be found in this part of the documentation, and this part!

AppContext.SetSwitch("Npgsql.EnableLegacyTimestampBehavior", true);

@neozhu
Copy link
Owner

neozhu commented Apr 26, 2023

Ok,
I applied for a free PostgreSQL database for testing
image

Removed: Unnecessary usings
Changed: Reduced nesting by inverting two if statements.
Copy link
Owner

@neozhu neozhu left a comment

Choose a reason for hiding this comment

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

nice

@Bram1903
Copy link
Collaborator Author

Bram1903 commented Apr 26, 2023

Everything seems to work with PostgreSQL. I enabled Npgsql.EnableLegacyTimestampBehavior when using PostgreSQL as a database provider, even though we might want to use UTC as a timezone, rather than local at a later point. The migrations are being applied correctly, and the database is correctly being seeded as well!

I did notice that when trying to use the connection string provided by you with the external database the website doesn't seem to launch correctly, but I'm uncertain what this might have caused. When using my own running instance everything works perfectly fine, just as it did yesterday without this new clean approach!

@neozhu
Copy link
Owner

neozhu commented Apr 26, 2023

Yes, connecting to the locally running PostgreSQL database works fine.
Thank you.

@neozhu neozhu merged commit 40444fd into neozhu:main Apr 26, 2023
@Bram1903
Copy link
Collaborator Author

Bram1903 commented Apr 26, 2023

I think this PR is a great addition to the already existing codebase because many people will benefit from being able to use another Database! Besides that adding additional support for a new Database has also become significantly easier.

Great collaboration!

@Bram1903 Bram1903 self-assigned this May 5, 2023
@Bram1903 Bram1903 added Documentation Improvements or additions to documentation Enhancement New feature or request Help Wanted Extra attention is needed labels May 5, 2023
@Bram1903 Bram1903 linked an issue May 5, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation Enhancement New feature or request Help Wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Additional database engine support
3 participants