Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why the database data is not being updated but object did and without error?

I have this bank ATM mock-up app which implements some Domain-Driven Design architecture and Unit of Work pattern.

This app have 3 basic functions:

  • Check balance
  • Deposit
  • Withdraw

These are the project layers:

ATM.Model (Domain model entity layer)

namespace ATM.Model
{
public class BankAccount
{
    public int Id { get; set; }
    public string AccountName { get; set; }
    public decimal Balance { get; set; }

    public decimal CheckBalance()
    {
        return Balance;
    }

    public void Deposit(int amount)
    {
        // Domain logic
        Balance += amount;
    }

    public void Withdraw(int amount)
    {
        // Domain logic
        //if(amount > Balance)
        //{
        //    throw new Exception("Withdraw amount exceed account balance.");
        //}

        Balance -= amount;
    }
}
}

namespace ATM.Model
{
public class Transaction
{
    public int Id { get; set; }
    public int BankAccountId { get; set; }
    public DateTime TransactionDateTime { get; set; }
    public TransactionType TransactionType { get; set; }
    public decimal Amount { get; set; }
}

public enum TransactionType
{
    Deposit, Withdraw
}
}

ATM.Persistence (Persistence Layer)

namespace ATM.Persistence.Context
{
public class AppDbContext : DbContext
{        
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"[connstring]");
    }

    public DbSet<BankAccount> BankAccounts { get; set; }
    public DbSet<Transaction> Transactions { get; set; }
}
}

namespace ATM.Persistence.Repository
{
public class RepositoryBankAccount
{
    public AppDbContext context { get; }

    public RepositoryBankAccount()
    {
        context = new AppDbContext();
    }

    public BankAccount FindById(int bankAccountId)
    {
        return context.BankAccounts.Find(bankAccountId);
    }

    public void AddBankAccount(BankAccount account)
    {
        context.BankAccounts.Add(account);
    }

    public void UpdateBankAccount(BankAccount account)
    {
        context.Entry(account).State = EntityState.Modified;
    }
}
}

namespace ATM.Persistence.Repository
{
public class RepositoryTransaction
{
    private readonly AppDbContext context;

    public RepositoryTransaction()
    {
        context = new AppDbContext();
    }

    public void AddTransaction(Transaction transaction)
    {
        context.Transactions.Add(transaction);
    }
}
}

namespace ATM.Persistence.UnitOfWork
{
public class UnitOfWork : IUnitOfWork
{
    private readonly AppDbContext db;
    public UnitOfWork()
    {
        db = new AppDbContext();
    }

    private RepositoryBankAccount _BankAccounts;
    public RepositoryBankAccount BankAccounts
    {
        get
        {
            if (_BankAccounts == null)
            {
                _BankAccounts = new RepositoryBankAccount();
            }
            return _BankAccounts;
        }
    }

    private RepositoryTransaction _Transactions;
    public RepositoryTransaction Transactions
    {
        get
        {
            if (_Transactions == null)
            {
                _Transactions = new RepositoryTransaction();
            }
            return _Transactions;
        }
    }

    public void Dispose()
    {
        db.Dispose();
    }

    public int Commit()
    {
        return db.SaveChanges();
    }

    public void Rollback()
    {
        db
        .ChangeTracker
        .Entries()
        .ToList()
        .ForEach(x => x.Reload());
    }
}
}

ATM.ApplicationService (Application layer)

namespace ATM.ApplicationService
{
public class AccountService
{        
    private readonly UnitOfWork uow;

    public AccountService()
    {            
        uow = new UnitOfWork();
    }

    public void DepositAmount(BankAccount bankAccount, int amount)
    {            
        bankAccount.Deposit(amount);
        uow.BankAccounts.UpdateBankAccount(bankAccount);

        var transaction = new Transaction()
        {
            BankAccountId = bankAccount.Id,
            Amount = amount,
            TransactionDateTime = DateTime.Now,
            TransactionType = TransactionType.Deposit
        };

        uow.Transactions.AddTransaction(transaction);

        try
        {
            uow.Commit();
        }
        catch
        {
            uow.Rollback();
        }
        finally
        {
            uow.Dispose();
        }
    }

    public void WithdrawAmount(BankAccount bankAccount, int amount)
    {            
        bankAccount.Withdraw(amount);
        uow.BankAccounts.UpdateBankAccount(bankAccount);
        //repoBankAccount.UpdateBankAccount(bankAccount);

        var transaction = new Transaction()
        {
            BankAccountId = bankAccount.Id,
            Amount = amount,
            TransactionDateTime = DateTime.Now,
            TransactionType = TransactionType.Withdraw
        };

        uow.Transactions.AddTransaction(transaction);

        try
        {
            uow.Commit();
        }
        catch
        {
            uow.Rollback();
        }
        finally
        {
            uow.Dispose();
        }
    }

    public decimal CheckBalanceAmount(int bankAccountId)
    {
        BankAccount bankAccount = uow.BankAccounts.FindById(bankAccountId);

        return bankAccount.CheckBalance();
    }
}
}

ATM.ConsoleUICore

namespace ATM.ConsoleUICore
{
class Program
{
    static void Main()
    {
        AccountService accountService = new AccountService();
        RepositoryBankAccount repoBankAccount = new RepositoryBankAccount();

        var bankAccount = repoBankAccount.FindById(2);

        Console.WriteLine("1. Check balance");
        Console.WriteLine("2. Deposit");
        Console.WriteLine("3. Withdraw");
        Console.WriteLine("Enter option: ");
        string opt = Console.ReadLine();
        switch (opt)
        {
            case "1":
                Console.WriteLine($"Your balance is ${bankAccount.CheckBalance()}");
                break;
            case "2":
                // User to input amount.
                // Data validation to make sure amount is greater than zero.
                // Pass the input amount to Application layer.

                accountService.DepositAmount(bankAccount, 50);

                // After getting the operation status from Application service layer.
                // Print operation status here: Either success or fail
                Console.WriteLine("Deposit successfully");
                break;
            case "3":            
                break;
            default:
                break;
        }

    }
}
}

I could check balance successfully. For option 2, I can execute "Deposit" option without any error. But in the database, my balance balance is not being updated. Transaction is also not added into the db.

If I put back context.SaveChanges(); in UpdateBankAccount method, it works. It returns 1. But, I use UoW to perform SaveChanges(). The SaveChanges() did executed in UoW Commit method but the database didn't reflect its changes. The UoW Commit method SaveChanges returns 0.

Complete code can be found on Github repository.

like image 782
Steve Avatar asked Aug 19 '19 10:08

Steve


2 Answers

The core of the problem here is that, two instances of AppDbContext are being created to conduct one single action. Changes are made in one instance and SaveChanges is being called on other instance. Obviously, it is not being reflected in underlying database.

We will now go through your code step by step from bottom to top.

In ATM.ConsoleUICore.Program.Main() method, note the following code:

AccountService accountService = new AccountService();
...
...
...
accountService.DepositAmount(bankAccount, 50);

You are creating an instance of AccountService. In constructor of AccountService, you are creating an instance of UnitOfWork as below:

private readonly UnitOfWork uow;
public AccountService()
{            
    uow = new UnitOfWork();
}

In constructor of UnitOfWork, you are creating an instance of AppDbContext (which is derived from DbContext).
You also have BankAccounts property there which is an instance of RepositoryBankAccount as below:

private readonly AppDbContext db;
public UnitOfWork()
{
    db = new AppDbContext();
}
...
...
...
private RepositoryBankAccount _BankAccounts;
public RepositoryBankAccount BankAccounts
{
    get
    {
        if (_BankAccounts == null)
        {
            _BankAccounts = new RepositoryBankAccount();
        }
        return _BankAccounts;
    }
}

Now the problem...

In constructor of RepositoryBankAccount, you are again creating an instance of AppDbContext as below:

public AppDbContext context { get; }
public RepositoryBankAccount()
{
    context = new AppDbContext();
}

Actually, you are pretending that your actions under one UnitOfWork instance are being executed as one database transaction. But, as you are creating different instance of AppDbContext in repository, this is not the case. Your unit of work is detached from repository. You have to connect them. It should be same instance of AppDbContext everywhere.

So, what is the solution?

Do NOT create an instance of AppDbContext in any repository. Instead, inject the existing instance from unit of work.

public AppDbContext context { get; }

public RepositoryBankAccount(AppDbContext appDbContext)//<==Inject the AppDbContext
{
    context = appDbContext;//<==Do NOT create new instance here; assign the injected instance.
}

Then, in your UnitOfWork class, change the property BankAccounts as below:

private RepositoryBankAccount _BankAccounts;
public RepositoryBankAccount BankAccounts
{
    get
    {
        if (_BankAccounts == null)
        {
            _BankAccounts = new RepositoryBankAccount(db);//<==Note that `db` means `AppDbContext` is injected
        }
        return _BankAccounts;
    }
}

By the way, avoid all these unnecessary wrappers over wrappers.

Have a look at this answer that explains why such wrappers are not needed.

Just in case you decide to go on with your existing design, I have already suggested a solution above.

Additionally, I will suggest your one unit of work should be one database transaction. So, your database transaction starts when you create an instance of unit of work and ends (commit or rollback) when you dispose it. Either everything flushes to database or none. Everything that happens in between this should be part of one database transaction. In case of exception, rollback the unit of work all together.

like image 161
Amit Joshi Avatar answered Oct 20 '22 01:10

Amit Joshi


try this format for any db transactions. creating multiple instance of

public RepositoryTransaction()
    {
        context = new AppDbContext();
    }

just mean that you create instance everytime and it does not get saved to database.

  using(AppDbContext db = new AppDbContext())
{
   var transaction = new Transaction()
        {
            BankAccountId = bankAccount.Id,
            Amount = amount,
            TransactionDateTime = DateTime.Now,
            TransactionType = TransactionType.Deposit
        };

    // save them back to the database
    //Add new Employee to database
         db.Transactions.InsertOnSubmit(transaction);

         //Save changes to Database.
         db.SubmitChanges();
}


     using(AppDbContext db = new AppDbContext())
    {
        // get the record
        Transaction dbProduct = db.Transactions.Single(p => p.BankAccountId == 1);

    // set new values
    dbProduct.TransactionDateTime = DateTime.Now; 
    dbProduct.TransactionType =  TransactionType.Deposit;

    // save them back to the database
    db.SubmitChanges();
}
like image 27
Jin Thakur Avatar answered Oct 19 '22 23:10

Jin Thakur