How can I optimize this ASP.NET Core + EF Core + PostgreSQL CRUD API?

Hi @Csaba Faragó ,

Thanks for sharing the details.

I would suggest a few improvements below:

1. GetAllVariables and the group lookup

The current projection:

_dbContext.groups
    .Where(y => y.groupId == x.groupId)
    .Select(y => y.name)
    .First()

is not ideal, but I would not describe it as a guaranteed N+1 problem. In EF Core, this kind of expression is still translated into a single SQL query. The issue is that it is harder to read and maintain than modeling the relationship explicitly.

A cleaner approach is to add a navigation property and project through it:

public class Variable
{
    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public int variableId { get; set; }

    public string title { get; set; }
    public string director { get; set; }
    public int releaseYear { get; set; }

    public int groupId { get; set; }
    public Group Group { get; set; }
}

Then query like this:

return await _dbContext.variables
    .AsNoTracking()
    .Select(x => new GetAllVariablesDto
    {
        VariableId = x.variableId,
        Title = x.title,
        Director = x.director,
        ReleaseYear = x.releaseYear,
        GroupId = x.groupId,
        GroupName = x.Group.name
    })
    .ToListAsync();

If you are projecting into a DTO, Include(...) is usually not needed. EF Core can translate the navigation access into the appropriate SQL.

2. AddTransient vs AddScoped

Since VariableModel depends on VariableDbContext, AddScoped<VariableModel>() is the better fit for a typical web API:

builder.Services.AddScoped<VariableModel>();

Using AddTransient is not necessarily broken, but scoped lifetime is usually the better semantic match for a request-oriented service that works with a scoped DbContext.

3. Manual transactions

For a single write operation followed by a single SaveChanges() call, you generally do not need to start a transaction manually. EF Core already wraps SaveChanges() in a transaction when needed.

So if your method is doing one unit of work, this is enough:

await _dbContext.SaveChangesAsync();

Manual transactions are more appropriate when:

• You need multiple SaveChanges() calls to succeed or fail together
• You are combining EF Core work with raw SQL or other database operations

Also, if SaveChanges() and Commit() are outside the using block, that is a correctness issue and should be fixed regardless.

4. Async methods with no await

If a method is marked async, it should normally contain real asynchronous database calls such as ToListAsync(), FirstOrDefaultAsync(), AnyAsync(), or SaveChangesAsync().

So you have two valid choices:

• Make the methods truly async
• Remove async and make them synchronous

For ASP.NET Core APIs, async database access is usually the better choice for scalability and consistency.

5. Where(...) plus null checks

This pattern is incorrect:

var data = _dbContext.variables.Where(x => x.variableId == id);
if (data == null)
    throw new KeyNotFoundException();

Where(...) returns a query, not an entity, so that null check is not meaningful.

Use something like:

var data = await _dbContext.variables
    .FirstOrDefaultAsync(x => x.variableId == id);

if (data == null)
    throw new KeyNotFoundException();

If this is a primary key lookup, FindAsync(id) is also a good option.

6. Avoid loading data into memory for existence checks

This pattern is inefficient:

_dbContext.groups.Select(x => x.name).ToList().Contains(data.GroupName)

because it loads all matching values into memory before checking.

Prefer letting the database do the existence check:

var exists = await _dbContext.groups
    .AnyAsync(x => x.name == data.GroupName);

That said, I would also review whether GroupName is really the correct lookup key. If the relationship is based on groupId, using the foreign key directly is usually better. If group names are meant to be unique identifiers, that should be enforced in the database.

7. Update and delete cleanup

The issues you identified there are valid:

• data.groupId == null is suspicious if groupId is a non-nullable int
• .First() followed by a null check is inconsistent, because .First() already throws if nothing is found
• duplicate validation conditions should be removed

FirstOrDefaultAsync() plus an explicit not-found check is clearer than relying on exceptions from .First().

8. Controller error handling

The general direction is good, but I would avoid returning raw exception details directly from controllers for unexpected failures.

A better pattern is:

• 400 for invalid input or validation issues
• 404 for missing resources
• 500 for unexpected errors

For larger APIs, a global exception handler with ProblemDetails keeps controllers much cleaner and gives you more consistent responses.

9. Additional improvements

A few other improvements worth considering:

• Use AsNoTracking() for read-only queries
• Use conventional REST routes such as GET api/variables and GET api/variables/{id}
• Re-enable HTTPS redirection outside local development
• Verify that appropriate indexes and foreign key constraints exist in PostgreSQL
• If you have a real relationship between Variable and Group, configure it explicitly in EF Core

Hope this helps! If my answer was helpful, I would greatly appreciate it if you could follow the instructions here so others with the same problem can benefit as well.