Spring Mvc RESTful app with MySQL

Question

I recently write a Spring MVC RESTful app and this is the REST controller that I have worked on. I haven't work with Spring RESTful for a while and hence, provided, the code for suggestions of how to improve. I can provedotehr part of the app if requested.

@RestController
@RequestMapping("/rest")
@Produces({"text/plain", "application/xml", "application/json"})
public class WalletRestController {

    @Autowired
    private WalletService walletService;

    @Autowired
    private UserService userService;

    // get all the wallets as the JSON arrays
    // curl -G http://localhost:8080/rest/wallets | json
    @RequestMapping(value = "/wallets", method = RequestMethod.GET)
    public ResponseEntity<List<WalletInfo>> getAllWalletInfo() {

        List<WalletInfo> walletInfos = walletService.getAllWallets();

        if (Objects.isNull(walletInfos)) {
            return new ResponseEntity<List<WalletInfo>>(HttpStatus.NO_CONTENT);
        }
        return new ResponseEntity<List<WalletInfo>>(walletInfos, HttpStatus.OK);
    }


    // get all the users
    // curl -G http://localhost:8080/rest/users | json
    @GetMapping(value = "/users")
    public ResponseEntity<List<User>> getAllUsers() {

        List<User> users = userService.getAllUsers();

        if (Objects.isNull(users)) {
            return new ResponseEntity<List<User>>(HttpStatus.NO_CONTENT);
        }
        return new ResponseEntity<List<User>>(users, HttpStatus.OK);
    }


    // get the info of the specific wallet

    // list of the requests
    // curl -i -H "Accept: application/json" http://localhost:8080/rest/wallets/1 | json
    // curl -G "Accept: application/json" http://localhost:8080/rest/wallets/1 | json
    // // curl -X GET "Accept: application/json" http://localhost:8080/rest/wallets/1 | json
    @RequestMapping(value = "/wallets/{id}", method = RequestMethod.GET)
    public ResponseEntity<WalletInfo> getWalletById(@PathVariable("id") long id) {

        System.out.println("Get wallet info with Id = " + id);

        WalletInfo walletInfo = walletService.getWalletInfo(id);

        if (walletInfo == null) {
            return new ResponseEntity<WalletInfo>(HttpStatus.NOT_FOUND);
        }

        return new ResponseEntity<WalletInfo>(walletInfo, HttpStatus.OK);
    }

    // generate the address from the provided wallet name
    //  curl -X POST -d "name=uuuw" http://localhost:8080/rest/generateAddress
    // curl -H "Content-Type: application/json" -X POST -d "nonald" http://localhost:8080/rest/generateAddress
    @PostMapping("/generateAddress")
    public ResponseEntity<WalletInfoWrapper> generateAddress(@RequestBody String name) {

        if (Objects.isNull(name)) {
            return new ResponseEntity<WalletInfoWrapper>(HttpStatus.NOT_ACCEPTABLE);
        }

        WalletInfo walletInfo = walletService.generateAddress(name);

        if (Objects.isNull(walletInfo)) {
            return new ResponseEntity<WalletInfoWrapper>(HttpStatus.NOT_ACCEPTABLE);
        }

        WalletInfoWrapper walletInfoWrapper = new WalletInfoWrapper();
        walletInfoWrapper.setName(walletInfo.getName());

        return new ResponseEntity<WalletInfoWrapper>(walletInfoWrapper, HttpStatus.CREATED);
    }


    // curl -X POST -d "amount=0.56&address=myuoXZrmSKtybRzkR5GfJm4LNtUoUorqsn" http://localhost:8080/rest/sendMoney/3
    // send money to the external users
    @PostMapping(value = "/sendMoney/{walletId}")
    public ResponseEntity<WalletModel> sendMoneyByWalletId(@PathVariable("walletId") Long walletId,
                                                           @RequestBody String amount, @RequestBody String address) {
        WalletModel walletModel = walletService.getWalletModel(walletId);

        if (Objects.isNull(walletModel)) {
            return new ResponseEntity<WalletModel>(HttpStatus.NOT_FOUND);
        }

        walletModel = walletService.sendMoney(walletId, amount, address);
        return new ResponseEntity<WalletModel>(walletModel, HttpStatus.OK);
    }


    // delete a wallet with the Id
    // curl -i -X DELETE http://localhost:8080/rest/delete/9
    @RequestMapping(value = "/delete/{id}", method = RequestMethod.DELETE, produces = MediaType.APPLICATION_JSON_VALUE)
    public ResponseEntity<WalletInfo> deleteWalletInfoById(@PathVariable("id") long id) {

        WalletInfo walletInfo = walletService.getWalletInfo(id);

        if (walletInfo == null) {
            System.out.println("The WalletInfo obj with id = " + id + " is not found");
            return new ResponseEntity<WalletInfo>(HttpStatus.NOT_FOUND);
        }

        walletService.deleteWalletInfoById(id);
        return new ResponseEntity<WalletInfo>(HttpStatus.NO_CONTENT);
    }


    // curl -G http://localhost:8080/rest/balanace/1 | json
    // get the wallet balance with the Id
    @GetMapping(value = "/balanace/{id}")
    public ResponseEntity<String> getWalletBalanceById(@PathVariable("id") long id) {

        WalletModel walletModel = walletService.getWalletModel(id);

        if (Objects.isNull(walletModel)) {
            return new ResponseEntity<String>(HttpStatus.NOT_FOUND);
        }

        String balanace = String.valueOf(walletModel.getBalanceFloatFormat());
        return new ResponseEntity<String>(balanace, HttpStatus.OK);
    }


    /**
     * get the list of the transactions with the wallet Id
     *
     * @param walletId
     * @return
     */
    // curl -G http://localhost:8080/rest/transactions/1 | json
    @GetMapping(value = "/transactions/{walletId}")
    public ResponseEntity<List<String>> readAllTransactionsByWalletId(@PathVariable("walletId") Long walletId) {

        WalletModel walletModel = walletService.getWalletModel(walletId);

        if (Objects.isNull(walletModel)) {
            return new ResponseEntity<List<String>>(HttpStatus.NOT_FOUND);
        }

        List<Transaction> transactions = walletModel.getTransactions();

        List<String> list = new ArrayList<>();

        for (Transaction transaction : transactions) {
            list.add(walletModel.addTransactionHistory(transaction));
        }

        return new ResponseEntity<List<String>>(list, HttpStatus.OK);
    }


    /**
     * @return return the number of wallet created as String
     */
    // http://localhost:8080/rest/walletsNumber
    @ResponseBody
    @RequestMapping("/walletsNumber")
    public String getWalletsCount() {

        List<WalletInfo> wallets = walletService.getAllWallets();
        return String.valueOf(wallets.size());
    }


    /**
     * @param id takes wallet index as the Long ID argument
     * @return return the balance of the request wallet
     */
    @ResponseBody
    @RequestMapping("/walletBalance")
    public String getWalletBalance(@RequestParam final Long id) {
        WalletModel walletModel = walletService.getWalletModel(id);
        return String.valueOf(walletModel.getBalanceFloatFormat());
    }

    /**
     * @param id takes wallet index as the Long ID argument
     * @return return the number of transaction executed on
     * the requested wallet
     */
    @ResponseBody
    @RequestMapping("/walletTransactionsNumber")
    public String getWalletTransactionsNumber(@RequestParam final Long id) {

        WalletModel walletModel = walletService.getWalletModel(id);
        List<Transaction> history = walletModel.getTransactions();
        return String.valueOf(history.size());
    }


    /**
     * a wrapper class of the WalletInfo class
     */
    private class WalletInfoWrapper {

        String name;

        String address; 

        public String getName() {
            return name;
        }

        public void setName(String name) {
            this.name = name;
        }
    }
}

Answer 1

My suggestions:

1. Documentation:

Proper documentation is very helpful for you, the other development team and future developers that will ever work on this code base after you. Even if it's for your own use, still it's better to write proper documentation so that you know yourself after a month or two, what this method is exactly doing. How to Write Doc Comments for the Javadoc Tool

2. Exposing Model Details:

Do not expose model details since this can clearly tell what is the structure of your data storage. Instead send or receive only what is required. Data Transfer Object

Example

You want to show User Full Name and Date of Birth on UI but your model is something like this; (Assume)

class User {
     private int id; // probably won't require
     private String userName;
     private String fullName;
     private String email;
     private Calendar dateOfBirth;
     // and few other details related to user.
}

Now, if you send List<User> in response, you are actually sending all the details in response and also expose that your storage structure is as in each User response. So, instead, do this:

class UserDTO {
      private String name;
      private Calendar dob;
}

Now, when you send List<UserDTO> in response, it only sends what is required (no less, no more). And no one can tell what exactly is the structure of storage.

The only thing here is, you have to do the transformation from User to UserDTO in most of the cases.

3. Reusability:

WalletModel walletModel = walletService.getWalletModel(id);

Since this piece of code (and also there are few others too) is being used in multiple endpoints, so it's better to write a function that implements the common functionality and reuse it.

4. Logging:

A very important piece of any application that is going to be used somewhere should perform logging.

Update: Added example for section 2.

Answer 2

My Suggestions:

@Produces can be replaced with produces attribute in @RequestMapping. For example:

@RequestMapping(value = {"/rest"}, 
    produces = {
        MediaType.TEXT_PLAIN_VALUE, 
        MediaType.APPLICATION_XML_VALUE, 
        MediaType.APPLICATION_JSON_VALUE}
)

@Autowired is no longer necessary and can be replaced with constructor

private WalletService walletService;
private UserService userService;

public WalletRestController(WalletService walletService, UserService userService) {
    this.walletService = walletService;
    this.userService = userService;
}

@ResponseBody is not needed when a class is annotated with @RestController