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

Allow extending ConfigureConventions and OnModelCreating #19236

Merged
merged 5 commits into from
Mar 8, 2024

Conversation

salihozkara
Copy link
Member

@salihozkara salihozkara commented Mar 7, 2024

Description

Users will need to replace these methods on a per-DbContext basis when they require them. Instead of replacement, methods can be extended using options. In our case, we utilized citext to ensure case-insensitivity for strings in Postgres SQL. However, this change was effective only when applied to abp.io's own DbContext; even if other DbContexts used citext in the database, case sensitivity was not addressed. Subsequently, after replacing all DbContexts and configuring this change within each, our issue was resolved. Given the potential workload of replacing and maintaining all DbContexts, employing such a method would be more ideal.

Example usage:

Configure<AbpDbContextOptions>(options =>
{
    options.UseNpgsql();
    options.ConfigureConventions((builder, dbContext) =>
    {
        builder.Properties<string>().HaveColumnType("citext");
    });
});

Checklist

  • I fully tested it as developer / designer and created unit / integration tests
  • I documented it (or no need to document or I will create a separate documentation issue)

@salihozkara salihozkara added this to the 8.2-preview milestone Mar 7, 2024
@salihozkara salihozkara requested review from hikalkan and maliming March 7, 2024 11:26
Copy link

codecov bot commented Mar 7, 2024

Codecov Report

Attention: Patch coverage is 24.05063% with 60 lines in your changes are missing coverage. Please review.

Project coverage is 51.53%. Comparing base (436b00d) to head (1761019).
Report is 47 commits behind head on dev.

Files Patch % Lines
...kCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs 33.33% 28 Missing and 2 partials ⚠️
...olo/Abp/EntityFrameworkCore/AbpDbContextOptions.cs 11.76% 30 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #19236      +/-   ##
==========================================
- Coverage   51.55%   51.53%   -0.03%     
==========================================
  Files        3097     3097              
  Lines       98420    98499      +79     
  Branches     7862     7880      +18     
==========================================
+ Hits        50741    50760      +19     
- Misses      46120    46178      +58     
- Partials     1559     1561       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maliming
Copy link
Member

maliming commented Mar 8, 2024

hi @salihozkara

I added a new commit. What do you think?

#19241
98e64b8

@salihozkara
Copy link
Member Author

salihozkara commented Mar 8, 2024

hi @salihozkara

I added a new commit. What do you think?

#19241 98e64b8

Hi @maliming
I am contemplating whether we need to add multiple actions without specifying a DbContext. For instance, if we add an option to the PostgreSQL package for using citext and include these actions when the option is set to true, if the user changes the default, our added actions will be removed. I believe it should be possible to add multiple actions without specifying a DbContext.

@maliming
Copy link
Member

maliming commented Mar 8, 2024

hi

I am contemplating whether we need to add multiple actions without specifying a DbContext.

The DefaultConventionAction and DefaultOnModelCreatingAction apply for all dbcontexts.

@salihozkara
Copy link
Member Author

hi

I am contemplating whether we need to add multiple actions without specifying a DbContext.

The DefaultConventionAction and DefaultOnModelCreatingAction apply for all dbcontexts.

The user will need to override it like this:

var oldAction = options.DefaultConventionAction;
options.DefaultConventionAction = (dbContext, builder) =>
  {
      oldAction?.Invoke(dbContext, builder);
      // my code
  };

If you find it suitable, it's not a problem for me. My other question is about the 'order' that was present before. Is there a specific reason for removing it?

@maliming
Copy link
Member

maliming commented Mar 8, 2024

DefaultConventionAction It should be used in the template project, It is similar to UseNpgsql/UseSqkServer, so it should be called once in a start-up project.

The actions will be in the order in the List<>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants