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

Structural Refactoring - .NET 6.0 & SqlServer #45

Merged
merged 20 commits into from
Dec 21, 2021

Conversation

enisn
Copy link
Contributor

@enisn enisn commented Dec 7, 2021

Summary

Why do you need to create DbContext and models for each EF Provider? They're just providers, the project abstraction shouldn't know anything about a provider. So I removed all the provider-specified codes and merged them into a single abstraction layer.

As I can see, migrations are managed by the library. Between EF Providers, the only different thing is migrations. Only migrations are separated in the ideal scenario. You can see, provider specified projects have only migrations but now any of EF providers can be used via generating migrations


What brings that structure?

That structure has a couple of benefits like I presented below.

Database Independency

Firstly, database dependency is completely removed. That means developers can use any database as they wish with EntityFramework. They need to manage their own migrations for database providers which we don't provide migrations.

  • New usage of a database that ironhook support (PostgreSql).
    If we provide migrations, it can be defined via calling UseIronHook{databasename}Migrations() method.
            services.AddIronHook(options =>
            {
                options.UseNpgsql(
                Configuration.GetConnectionString("Default"), 
                opts => opts.UseIronHookNpgsqlMigrations());
            });

I just replaced UseIronHook() method with MigrateIronHook() method since it doesn't add a middleware, it just performs a migration operation.

app.MigrateIronHook();
  • New usage of Non-supported databases
options.UseMySql(Configuration.GetConnectionString("Default"));

Migrations aren't provided by IronHook, but MigrateIronHook can still be used, it'll create a database from the current state of DbContext with https://github.com/FowApps/IronHook/pull/45/files#diff-48af72c016bfcfaede0f855ae54a6f1b88fceeb48b7977aea5bb3bc412f376edR41-R48

app.MigrateIronHook();

Managing own migrations by developers is highly recommended for unsupported databases but it's not mandatory

Multi-Targeting

Now, the project can support both framework versions at the same time. Projects are targeting .NET 5 and .NET6 right now.

In the future, .Net 5 support can be dropped partially.

Common DbContext

I just have removed DB provider-specific DbContexes since there is no difference with CoreDbContext. Using single DbContext is much more maintainable.


@enisn
Copy link
Contributor Author

enisn commented Dec 7, 2021

Hi @codeclimate, all of problems you found are auto-generated files (migrations) 😅

@furkandeveloper furkandeveloper self-requested a review December 8, 2021 09:47
@furkandeveloper
Copy link
Contributor

Thanks for the contribution, @enisn we are reviewing it as a team.

@furkandeveloper
Copy link
Contributor

Update codeclimate.yml file on root and exclude SqlServer migration files. @enisn

@furkandeveloper furkandeveloper self-assigned this Dec 8, 2021
@furkandeveloper furkandeveloper added backlog An issue as an idea that hasn't planned, approved or estimated yet change-request Process used to identitfy, document and remediate change codeclimate dependencies Pull requests that update a dependency file dev-ops dev-ops relaed operations documentation Improvements or additions to documentation effort-12 effort-8 enhancement New feature or request github_actions Pull requests that update Github_actions code infrastructure Infrastructure priority:high High Priority Task priority:medium Medium Priority Task quality labels Dec 8, 2021
@furkandeveloper
Copy link
Contributor

Codeclimate treats files with the same name as duplications. For this reason, we must make the names of the IronHookCoreDbContextFactory classes provider dependent.

for example;

IronHookPostgreSqlCoreDbContextFactory instead of IronHookCoreDbContextFactory
we can use

@enisn

@codeclimate
Copy link

codeclimate bot commented Dec 9, 2021

Code Climate has analyzed commit c66a794 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 4

View more on Code Climate.

@enisn
Copy link
Contributor Author

enisn commented Dec 11, 2021

Any updates?

@furkandeveloper
Copy link
Contributor

Any updates?

Not for now. Tests continue. PR can be merged on weekdays. @enisn

Copy link
Contributor

@furkandeveloper furkandeveloper left a comment

Choose a reason for hiding this comment

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

:lgtm:

@furkandeveloper furkandeveloper merged commit 8d26a1a into FowApps:master Dec 21, 2021
@furkandeveloper furkandeveloper linked an issue Dec 21, 2021 that may be closed by this pull request
@enisn
Copy link
Contributor Author

enisn commented Dec 21, 2021

I can't update the wiki section, Can you update documentation too?

@furkandeveloper
Copy link
Contributor

I updated wiki documentation. Thanks @enisn. Developing with you is so much fun. 🎉🎉🎉🎉

@furkandeveloper
Copy link
Contributor

@all-contributors please add @enisn for infrastructure, tests and code

@allcontributors
Copy link
Contributor

@furkandeveloper

I've put up a pull request to add @enisn! 🎉

@enisn enisn deleted the feature/structural-refactoring branch October 17, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog An issue as an idea that hasn't planned, approved or estimated yet change-request Process used to identitfy, document and remediate change codeclimate dependencies Pull requests that update a dependency file dev-ops dev-ops relaed operations documentation Improvements or additions to documentation effort-8 effort-12 enhancement New feature or request github_actions Pull requests that update Github_actions code infrastructure Infrastructure priority:high High Priority Task priority:medium Medium Priority Task quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📌 [CUSTOM] Net6 support 📌 [CUSTOM] Updated package versions ✨ [FEATURE] SQL Server Implementation
2 participants