So I have a controller called CircuitController where I want to retrieve an Entity both from the ID and from a reference:

    @GetMapping("/api/circuits/{id}")
    public EntityModel<Circuit> getCircuitById(@PathVariable Long id) {
        Circuit circuit = circuitRepository.findById(id)
                .orElseThrow(() -> new CircuitNotFoundException(id));

        return circuitModelAssembler.toModel(circuit);
    }

    @GetMapping("/api/circuits/")
    public EntityModel<Circuit> getCircuitByRef(@RequestParam(value="ref") String ref) {
        // no "circuitRepository.findByRef(ref) method here.
    }

For the second method, there is no findByRef(String ref) method already implemented in the JPARepository

I came up with 2 solutions:

1. In my CircuitRepository class I can define a default method findByRef(String ref) to do exactly what I need and then use it from the CircuitController class.

@Repository
public interface CircuitRepository extends JpaRepository<Circuit, Long>  {

    default Circuit findCircuitByRef(String ref) {
        Optional<Circuit> circuit = this.findAll()
                .stream()
                .filter(circuit1 -> circuit1.getCircuitRef().equals(ref))
                .findAny();

        if (circuit.isPresent()) {
            return circuit.get();
        } else {
            throw new CircuitNotFoundException(ref);
        }
    }
}

and then I can do:

   @GetMapping("/api/circuits/")
    public EntityModel<Circuit> getCircuitByRef(@RequestParam(value="ref") String ref) {
        Circuit circuit = circuitRepository.findCircuitByRef(ref);
        return circuitModelAssembler.toModel(circuit);
    }

2. The 2nd option would be to implement the logic in my getCircuitByRef method directly like this:

 @GetMapping("/api/circuits/")
    public EntityModel<Circuit> getCircuitByRef(@RequestParam(value="ref") String ref) {
        Circuit circuit;
        Optional<Circuit> optionalCircuit = circuitRepository.findAll()
                .stream()
                .filter(circuit1 -> circuit1.getCircuitRef().equals(ref))
                .findAny();

        if (optionalCircuit.isPresent()) {
            circuit = optionalCircuit.get();
        } else {
            throw new CircuitNotFoundException(ref);
        }
        return circuitModelAssembler.toModel(circuit);
    }

I'm not sure what is the best practice here, or how the code standards for spring applications are. On thing I've noticed is that with the first option, I can then later mock my CircuitRepository interface and mock the findByRef method for UT purposes.

CodePudding user response：

In general, it is a better practice to separate functional components in a project. I prefer to do it like this - Controller is responsible for handling the API calls, Repository - data access. So, I would stick with the 1st option.

As for the actual implementation, I would recommend using Spring's functionality. There is no method findCircuitByRef(String ref), but Spring can create an implementation of this method for you. What you can do is the following:

@Repository
public interface CircuitRepository extends JpaRepository<Circuit, Long>  {
    Circuit findByCircuitRef(String ref);
}

What Spring will do behind the scenes is the following - it will create an implementation of this method, which does what you are doing in the default implementation. You can see more information on the topic here.