2
\$\begingroup\$

I'm constructing a homework from my Spring Boot Course and so far I did the JPA layer and want to have some feedback, before start the business rules, about if the I could improve something or change a type of relationship in my app. Any suggestions will be welcome.

Here is the exercise:

"Assume we have a big legacy system and one of the parts is withdrawal processing (the process that allows to transfer money from company to employee accounts). Now we have chance to completely rewrite the system, including API change (endpoints, DTOs etc). As a techical challenge we suggest you to take it. You can do whathever you want following the acceptance criteria:

  • Use any architecture you are comfortable with
  • Use modern Java or Kotlin
  • Use Spring boot
  • Use any database SQL/NoSQL (please use embedded)
  • The code must be tested
  • The service should be easy to run (e.q. docker-compose)

Here are some business rules of the processing:

  • We have a list of users (/v1/users endpoint)
  • A user has several payment methods
  • A user can execute a withdrawal request using one of a payment methods
  • A withdrawal can be executed as soon as possible or be scheduled to execute later
  • After the service receives a request it stores a withdrawal object in our DB and sends a request to a payment provider async. Note: for this task we don't care about a transaction completion
  • We MUST 100% send notifications regarding any withdrawal status (event, email etc)"

That's my solution for the Models with JPA:

  • USER

     @Entity(name = "User")
     @Table(name = "user", uniqueConstraints = {@UniqueConstraint(name= "user_email_unique", columnNames = "email")})
     public class User {
    
     @Id
     @GeneratedValue(strategy = GenerationType.IDENTITY)
     @Column(name = "user_id", updatable=false)
     private Long id;
    
     @OneToMany(mappedBy="user")
     @Column(name = "payment_methods")
     private List<PaymentMethod> paymentMethods;
    
     @OneToOne(mappedBy = "user")
     @JoinColumn(name = "account_id", nullable = false)
     private Account account;
    
     @Column(name = "first_name", nullable = false, columnDefinition = "TEXT")
     private String firstName;
    
     @Column(name = "last_name", nullable = false, columnDefinition = "TEXT")
     private String lastName;
    
     @Column(name = "email", nullable = false)
     private String email;
    
     @Column(name = "max_withdrawal_amount", nullable = false, columnDefinition = "DOUBLE")
     private Double maxWithdrawalAmount;
    
     } 
    
  • ACCOUNT

     @Entity(name = "Account")
     @Table(name = "account", uniqueConstraints = {
     @UniqueConstraint(name = "account_account_number_unique", columnNames = "account_number") })
     public class Account {
    
     @Id
     @GeneratedValue(strategy = GenerationType.IDENTITY)
     @Column(name = "account_id")
     private Long id;
    
     @OneToOne(fetch = FetchType.LAZY)
     @MapsId
     private User user;
    
     @OneToMany(mappedBy = "account", cascade = CascadeType.ALL, fetch = FetchType.EAGER)
     private List<Transaction> transactionList = new ArrayList<>();
    
     @Column(name = "account_number", nullable = false)
     private Integer accountNumber;
    
     @Column(name = "balance", nullable = false)
     private double balance;
    
     }
    
  • PAYMENT METHOD

     @Entity(name = "PaymentMethod")
     @Table(name = "payment_methods")
     public class PaymentMethod {
    
     @Id
     @GeneratedValue(strategy = GenerationType.IDENTITY)
     @Column(name = "payment_methods_id")
     private Long id;
    
     @ManyToOne
     @JsonIgnore
     @JoinColumn(name = "user_id", nullable = false)
     private User user;
    
     @Column(name = "payment_name", nullable = false, columnDefinition = "TEXT")
     private String paymentName;
     }
    
  • TRANSACTION

    @Entity(name = "Transaction")
    @Table(name = "transaction")
    public class Transaction {
    
    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column(name = "transaction_id", updatable = false)
    private Long id;
    
    @Column(name = "comment", nullable = false, columnDefinition = "TEXT")
    private String comment;
    
    @ManyToOne
    @JoinColumn(name = "accountNumber")
    private Account account;
    
    }
    
  • WITHDRAWAL

    @Entity(name = "Withdrawal")
    @Table(name = "withdrawal")
    public class Withdrawal {
    
    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column(name = "withdrawals_id")
    private Long id;
    
    @OneToOne(fetch = FetchType.LAZY)
    @MapsId
    private Transaction transaction;
    
    @ManyToOne
    @JsonIgnore
    @JoinColumn(name = "payment_methods_id", nullable = false)
    private PaymentMethod paymentMethod;
    
    @Column(name = "amount", nullable = false, columnDefinition = "DOUBLE")
    private Double amount;
    
    @Column(name = "created_at", nullable = false, columnDefinition = "DATE DEFAULT CURRENT_DATE")
    private Instant createdAt;
    
    @Column(name = "execute_at", nullable = false, columnDefinition = "DATE DEFAULT CURRENT_DATE")
    private Instant executeAt;
    
    @Enumerated(EnumType.STRING)
    @Column(name = "withdrawal_status", nullable = false, columnDefinition = "TEXT")
    private WithdrawalStatus status;
    }
    
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Mast May 24 at 11:46
3
\$\begingroup\$

Welcome to code review! I have some comments about the data structure from banking point of view.

  • Users usually have more than one account.
  • E-mail is not a unique identifier.
  • The assumption that users have a first and last name is wrong. :)
  • Maximum withdrawal amount is usually a property of the account. Each account can have a different withdrawal limit. Usually there are even separate limits for internet payments and ATM withdrawals, but that may be beyond the scope of the exercise.
  • Fetech type on account transactions should never be eager. There may be many thousands of transactions and loading the account would thus fetch them all. In fact, providing a list access to the complete transaction history is not feasible at all. Due to the volume, transaction history should be provided through a separate interface that supports paging and searching.
  • Money should never be described as floting points.
  • What is the purpose of payment method in your system? Usually it means something like "debit card purchase", "ATM withdrawal", "online bank transfer" etc. All in all it probably should be an enumeration of values that is not associated to any specific user.
  • A transaction involves two accounts: credit account for the recipient and debit account for the payer.
  • Money withdrawal is a form of transaction. A transaction is identified as a money withdrawal by the transaction's type-attribute. The payment method may be related to this.
\$\endgroup\$
1
  • \$\begingroup\$ Hi @TorbenPutkonen. Many thanks in advance for all the help you gave me here. I was refining my JPA Structure as you advised and so far I made some improvements. =) I'll go point by point: code. \$\endgroup\$ – Gabriel Vendramini May 24 at 11:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.