Improve JmixDataRepository API to simplify FetchPlan creation

[fractal3000]

There is a group of methods in the JmixDataRepository, such as the following
T getById(ID id, @nullable FetchPlan fetchPlan);

These methods contain a FetchPlan object as a parameter.

The first problem is that we need to inject additional bean to solve the single problem (getting data from repository). This is the FetchPlans bean, so we need two beans to call a single method. At the same time the FetchPlans bean is the single bean that provides fetch plans.

The second problem is the testability of the beans that use the repositories. This issue is due to FetchPlans API. It is a fluent-style API, which is not suitable for unit testing (for mocking and stubbing).

This problem is exacerbated by the fact that the FetchPlan class has no any public constructor. As a result, the mocking of the FetchPlan class requires the objenesis libruary that as a consequence looks like a smell of architecture problems.

The preposition is to modify JmixDataRepository API in order to avoid using the additional bean. It looks like a good way is to use lambdas like in the DataManager fluent-style API(FetchPlanBuilder).

Using lambdas supports an ability to mocking and subbing the data repository bean (lambda can be made as a static field that can be used in the test). Additionally, using lambdas for fetch plans is a very popular and appopriate way that is practiced with using the DataManager.

[dtaimanov]

Here is draft implementation of builder paramer usage in JmixDataRepositoryContext:
FetchPlan_builder_as_parameter_of_JmixDataRepositoryContext.patch

//-------------------------- OrderRepository: -------------------------------
public interface OrderRepository extends JmixDataRepository<SalesOrder, UUID> {

    List<SalesOrder> findByNumberLike(String numberPattern, JmixDataRepositoryContext context);
}

//-------------------------- Usage: -------------------------------

import static io.jmix.core.repository.JmixDataRepositoryContext.of;
//...
    List<SalesOrder> orders = orderRepository.findByNumberLike("14", of(b -> b.add("customer", FetchPlan.LOCAL)))
//...

What’s left to do?

• JavaDocs
• Tests

[Fedoseew]

Probably will be useful add following method?

Optional<T> findById(UUID id, JmixDataRepositoryContext context);

[fractal3000]

Current state with code examples.

This is the example of the data repository

@JmixDataRepository
public interface OrderRepository extends JmixDataRepository<SalesOrder, UUID> {

    List<SalesOrder> findByNumberLike(String numberPattern, FetchPlan fetchPlan);
}

If we need to use it with the dynamic fetch plan creation(creation feth plan with the java code), we need to write something like:

@Component
public OrderService{

	@Autowired
	OrderRepository repository;
	
	Autowired
	//Should be always injected with the repository bean.
	FetchPlans fetchPlans;
	
	void findByNumberLikeWithDetails(String numberPattern){
	
		//Should be created with the builder
		FetchPlan fetchPlan = fetchPlans.builder(SalesOrder.class)
            .addFetchPlan(FetchPlan.BASE)
            .add("customer")
			.add("orderLines", FetchPlan.BASE)
            .build();
			
		return findByNumberLike("%somePattern%", FetchPlan );
	}
}

If we need to write a unit test on this service we should write something like that or more complex code in order to mock FetchPlanBuilder behaviour:

    def "test"() {
        given: "OrderService with mocked dependencies"
        OrderRepository orderRepository = Mock()
        FetchPlans fetchPlans = Mock()
        FetchPlanBuilder builder = Mock()
        def service = new OrderService(orderRepository, fetchPlans)

        and: "Mocked fetch plan building chain (verifies BASE is used for OrderItem)"
        1 * fetchPlans.builder(Order) >> builder
        1 * builder.add("customerName") >> builder
        1 * builder.add("items", _) >> { Object[] args ->
            def consumer = args[1]
            FetchPlanBuilder sub = Mock(FetchPlanBuilder)
            1 * sub.addFetchPlan(FetchPlan.BASE) >> sub
            consumer.accept(sub)
            return builder
        }
        1 * builder.build() >> Mock(FetchPlan)

        and: "Repository returns a list of orders"
        def expected = [new Order()]
        1 * orderRepository.findByNumberLike("%123%", _ as FetchPlan) >> expected

        when: "Service method is invoked"
        def result = service.findByNumberLikeWithDetails("%123%")

        then: "Result from repository is returned"
        result == expected
    }

If we have had an option to use

@Component
public OrderService{

    protected static final Consumer<FetchPlanBuilder> SPECIFIC_FETCH_PLAN_FOR_ORDER = fpb -> fpb.add("customerName")
            .add("items", itemBuilder -> itemBuilder.addFetchPlan(FetchPlan.BASE));
    private final OrderRepository orderRepository;

    public OrderService(OrderRepository orderRepository) {
        this.orderRepository = orderRepository;
    }

    void findByNumberLikeWithDetails(String numberPattern){
        return orderRepository.findByNumberLike(numberPattern, SPECIFIC_FETCH_PLAN_FOR_ORDER);
    }
}

And the test for it will look more simple:

    def "test with a lambda"() {
        given: "OrderService with mocked repository"
        OrderRepository orderRepository = Mock()
        def service = new OrderService(orderRepository)

        and: "Repository returns a list of orders and is called with the predefined fetch plan constant"
        def expected = [new Order()]
        1 * orderRepository.findByNumberLike("%123%", OrderService.SPECIFIC_FETCH_PLAN_FOR_ORDER) >> expected

        when: "Service method is invoked"
        def result = service.findByNumberLikeWithDetails("%123%")

        then: "Result from repository is returned"
        result == expected
    }

The suggested by Dmitry Taimanov option with the static method for the JmixDataRepositoryContext creating looks like a good compromise option.

[fractal3000]

The code with option of the Dmitry Taimanov's suggested solution.

Repository

@Repository
public interface OrderRepository extends JmixDataRepository<Order, UUID> {

    List<Order> findByNumberLike(String numberPattern, JmixDataRepositoryContext context);
}

Service

@Component
public class OrderService {

    public static final JmixDataRepositoryContext REPOSITORY_CONTEXT_FOR_ORDER = of(fpb -> fpb.add("customerName")
            // Ensure items are loaded; use BASE for OrderItem
            .add("items", itemBuilder -> itemBuilder.addFetchPlan(FetchPlan.BASE)));
    
    private final OrderRepository orderRepository;

    public OrderService(OrderRepository orderRepository) {
        this.orderRepository = orderRepository;
    }

    public List<Order> findByNumberLikeWithDetails(String numberPattern) {
        return orderRepository.findByNumberLike(numberPattern, REPOSITORY_CONTEXT_FOR_ORDER);
    }
}

Test

    def "unit test"() {
        given: "OrderService with mocked repository"
        OrderRepository orderRepository = Mock()
        def service = new OrderService(orderRepository)

        and: "Repository returns empty list when called with null pattern and specific fetch plan"
        def expected = []
        1 * orderRepository.findByNumberLike("some string", OrderService.REPOSITORY_CONTEXT_FOR_ORDER) >> expected

        when: "Service method is invoked with null"
        def result = service.findByNumberLikeWithDetails("some string")

        then: "Repository result is returned as-is"
        result == expected
    }