Prohibit circular references by default

[wilkinsona]

We believe that circular references between beans should be avoided if at all possible. To help users avoid accidentally creating such dependency cycles, we should configure the BeanFactory to prohibit circular references by default. We'll allow them to be enabled via API and configuration property for those that really need them, as we did with bean definition overriding in 2.1. We should also update ApplicationContextRunner at the same time.

[bmaehr]

@wilkinsona I'm missing a good solution for the self references (needed für transaction handling) which are detected now.

[wilkinsona]

If you'd like some help, please ask on Stack Overflow or Gitter. Someone trying to help you will need to see the code that contains the circular references so that they can advise you on how to remove them.

[bmaehr]

@wilkinsona That's not true. It is a common pattern to inject the bean itself to be able to use spring annotations for "internal" calls. So if this behavior is disabled by default there should be a general suggestion how to solve these kind of problems (without the need changing the structure of the beans which are designed that way because of a good reason).

spring-projects/spring-framework#13096
https://stackguides.com/questions/3037006/starting-new-transaction-in-spring-bean
https://www.geekyhacker.com/2020/04/02/how-to-use-spring-boot-cacheable-on-self-invocation/

So I'm not looking for solution in a special case, there should be a feasible general solution for these use cases.

[bennettdams]

I would also like to know of a recommended solution.

Here is another very common use case: I have repo and a custom repo, where the custom repo uses the repo inside. Right now, the only thing that I came up with to prevent the circular reference problem is by using @Lazy for the initialization:

interface DogRepo : DogRepoCustom, MongoRepository<Dog, String> { ... }

class DogRepoImpl(@Lazy private val dogRepo: DogRepo) : DogRepoCustom { ... }

interface DogRepoCustom { ... }

The problem is that using @Lazy here is considered a workaround by someone who knows this topic way better than me.

[wilkinsona]

If you are looking for a general solution that does not require you to change your code, you can use the configuration property to permit circular dependencies again. This property is described in the release notes for Spring Boot 2.6.

[bennettdams]

@wilkinsona Thanks! I think the Spring team had good reasons to prohibit circular dependencies and I want to follow that.
I'm totally down to change my code, but I don't know the best way to do that. Taking my example above (repo & custom repo), is using @Lazy a bad idea? If yes, what is the recommended way to do so?
I think Spring should add something to the docs to show some examples on what to do against circular dependencies, now that you state that having them is a bad idea. I would like to know this for my example obviously, but the topic in general could be explained in the docs.

[wilkinsona]

With regards to repository fragments, that's really a question for the Spring Data team (/cc @mp911de). I'm not familiar with the other areas where self-injection has been recommended and I've never had a need for it so it's hard for me to comment.

[mp911de]

Generally speaking, if you defer repository bean access to after its creation (@Lazy, ObjectProvider, BeanFactory lookup) then you won't run into the circular bean error. Keep in mind that repository fragments are used to assemble the repository itself so circular references can lead to recursive calls.

[cfbo]

In the project I work we have been using self invocations for caching quite a lot.
Having upgraded to 2.6.3, I am looking for a migration strategy.

I was curios to get some feedback on a solution like that like the one below. Basically I would replace self-invocations with calls to that ProxyExecutor bean and do something like:

Function<UserService, UserConfiguration> fn = (userService) -> userService.getCurrentUserConfiguration(); UserConfiguration conf = proxyExecutor.execute(fn, UserService.class);

@Service
public class ProxyExecutor {

    @Autowired
    private ApplicationContext applicationContext;

    /**
     * Allows to call a bean method forcing to go through the proxy.
     * 
     * @param <T>
     *            the class of the spring bean to be invoked
     * @param <R>
     *            the result of the function to invoke on that bean
     * @param fn
     *            the function that needs to be invoked via proxy
     * @param beanClazz
     *            the bean class
     * @return the same result of the given function
     */
    @Transactional(propagation = Propagation.REQUIRED)
    public <T, R> R execute(Function<T, R> fn, Class<T> beanClazz) {
        T bean = applicationContext.getBean(beanClazz);
        return fn.apply(bean);
    }
}

I reckon it's a bit of a hack, but from a general perspective I don't see issues with it.

[b-charles]

Where can we get a more detailed and constructed answer?

Everybody seems to think that cyclic dependencies is the sign of a bad design, but I don't get it. Of course, if the beans are injected with the constructor, a cyclic dependency is terribly problematic. But the simple solution is doing injections by setters, and from my experience, this approach is more convenient and less verbose. In one of my project, we have a data model close to a graph, with some Node and Edge POJO. So, to process theses data, we need two beans, a NodeProcessor and an EdgeProcessor, and naturally, each one depends of the other: each processor is complex enough to be in is own class, but to process a node you need to process every output edges, and to process an edge, you have to process the associated node. This design seems to me clear and understandable. Every suggested solutions introduce every time a hack based on an obscure class without any business semantic meaning and with the only purpose of break the cycle, one way or another. That's seems to me a worst design than my just two cyclic beans. So, where come from this very dogmatic "cyclic dependencies are bad"? Does anyone have a creepypasta with a sadistic or diabolically evil cyclic dependency?

Spring Core seems to forbid the cyclic dependencies for now. Spring Boot introduced a way to allow it, but the release notes indicate clearly "you are strongly encouraged to update your configuration to break the dependency cycle". What is the long term vision of each Spring teams on that subject? Is the property spring.main.allow-circular-references is an acknowledgement that some cyclic dependency are necessary (but only from Spring Boot, with Spring Core denying it) or it is only a temporary solution to allow some time to developers to find a migration path?

[mcekovic]

I reckon it's a bit of a hack, but from a general perspective I don't see issues with it.

No, its not a hack. Self-injection is completely fine in order to allow reentrant calls to honor AOP advises, like @Cacheable, @transactional, @retryable

[Majstr]

In our project we have a lot of references to other beans, because we separated the code by logical context. For example there is UserService that handles all user actions and there is CustomerService that handles customer actions. Of course UserService has reference to CustomerService, if we want to find customer. And CustomerService has UserService to check role of user for example. We have no idea how to break this "dependency cycle" and make better architecture and not make big mess of services with everything in one place.

Is there any good practices how to make better architecture? How to even start solving this problem?

[mcekovic]

You have to separate your business logic into coherent pieces. Although I have no info what are your UserService and CustomerService doing, it seems fishy separation as User and Customer are similar things. Instead of grouping business logic by Entity (User, Customer), try grouping by processes, use cases, etc... If you cannot clearly identify business process groups for User and Customer, you can try following purely technical technique:

• Find all methods in UserService that need CustomerService and move them to new UserCustomerService
• Find all methods in CustomerService that need UserService and move them to new CustomerUserService
• Make UserCustomerService depend on CustomerService (not on CustomerUserService)
• Make CustomerUserService depend on UserService (not on UserCustomerService)
  This will help If you do not have circular method chains (UserService.methodABC calls CustomerService.methodEDF which calls UserService.methodGHI...). If you have such circular method chains, then you are in such a deep mess that is possible to address only by rewriting (and remodeling) all from scratch.

[bmaehr]

@Majstr What works quite well is to sepparate low level operations of customer service (usually doesn't need user service) from more complex operation of customer service and divide that into different services. Same for user service.

[jonenst]

No, its not a hack. Self-injection is completely fine in order to allow reentrant calls to honor AOP advises, like https://github.com/Cacheable, @transactional, @retryable

Is there an official strategy for these legitimate use cases ? using spring.main.allow-circular-references for the whole context just to allow one service to self-inject doesn't seem right.

[mcekovic]

Is there an official strategy for these legitimate use cases ?

Completely agree, there should be official strategy for legitimate use cases of circular dependencies, like self-ijnection. Maybe a special annotation just for self-injection is a possible solution. Or some other explicit way to address AOP interceptors for re-entrant calls.

[wilkinsona]

If this is something that you would like to see, please raise it with the Framework team. The only tool at Boot’s disposal is a BeanFactory-wide configuration option which is what spring.main.allow-circular-references maps onto.