How bad is a service circular reference?

Question

I have a question and it is related to the error that I am getting. How bad is it really to have a circular reference in my service? I know very well what the error is due to and how to possibly solve it, only that in the company where I work a Senior recommended me that for transactional issues it is necessary to make such a circular reference and in fact it is a very recurrent practice there, but as I am starting a personal project from scratch is the first time I get the error and it triggered the doubt again. Thank you very much in advance!

Here is the code of the service

public class MedicalRecordServiceImpl implements MedicalRecordService {

    private final MedicalRecordRepository medicalRecordRepository;
    private final MedicalRecordService medicalRecordService;
    private final PatientService patientService;
    private final TutorService tutorService;
    private final MedicalHistoryAnswerService medicalHistoryAnswerService;
    private final DentalHistoryAnswerService dentalHistoryAnswerService;

    public MedicalRecordServiceImpl(MedicalRecordRepository medicalRecordRepository, MedicalRecordService medicalRecordService, PatientService patientService, TutorService tutorService, MedicalHistoryAnswerService medicalHistoryAnswerService, DentalHistoryAnswerService dentalHistoryAnswerService) {
        this.medicalRecordRepository = medicalRecordRepository;
        this.medicalRecordService = medicalRecordService;

        this.patientService = patientService;
        this.tutorService = tutorService;
        this.medicalHistoryAnswerService = medicalHistoryAnswerService;
        this.dentalHistoryAnswerService = dentalHistoryAnswerService;
    }

    @Override
    @Transactional(propagation = Propagation.REQUIRED)
    public void saveMedicalRecord(MedicalRecordEntity medicalRecord) {
        medicalRecordRepository.save(medicalRecord);
    }

    @Override
    @Transactional(propagation = Propagation.REQUIRED)
    public ResponseEntity<?> createNewMedicalRecord(MedicalRecordDTO medicalRecordDTO) {

        PatientEntity patient = this.storeMedicalRecordIntoPatient(medicalRecordDTO);
        TutorEntity tutor = this.storeMedicalRecordIntoTutor(medicalRecordDTO);
        List<MedicalHistoryAnswerEntity> medicalHistoryAnswers = this.storeMedicalRecordIntoMedicalHisAns(medicalRecordDTO);
        List<DentalHistoryAnswerEntity> dentalHistoryAnswers = this.storeMedicalRecordIntoDentalHisAns(medicalRecordDTO);

        patientService.savePatient(patient);
        tutor.setPatient(patient);
        tutorService.saveTutor(tutor);

        MedicalRecordEntity medicalRecord = this.createMedicalRecord(patient, tutor);

        medicalRecordService.saveMedicalRecord(medicalRecord);

        medicalHistoryAnswers.forEach(medicalHistoryAnswer -> {
            medicalHistoryAnswer.setMedicalRecord(medicalRecord);
            medicalHistoryAnswerService.saveMedicalHistoryAnswer(medicalHistoryAnswer);
        });

        dentalHistoryAnswers.forEach(dentalHistoryAnswer -> {
            dentalHistoryAnswer.setMedicalRecord(medicalRecord);
            dentalHistoryAnswerService.saveDentalHistoryAnswer(dentalHistoryAnswer);
        });

        return ResponseEntity.status(HttpStatus.OK).body("");
    }
}

Unless you have multiple implementations of `MedicalRecordService`, it doesn't make any sense to call methods using a reference which will refer to the same Service instance (as beans by default are `Singleton`) — Chetan Ahirrao, May 24 '22 at 04:42
In this case the self reference doesn't make sense. It would make sense if the `saveMedicalRecord` would run in a new transaction. But here it doesn't make sense. Another thing wrong with this service is that it is tied to the web layer by returning a `ResponseEntity` that belongs in the controller **not** your service. — M. Deinum, May 24 '22 at 05:57
@M.Deinum Then you recommend me to return the response entity directly from the controller? the thing is some people taught me that the controllers should not have any logic, they should only be limited to call the service then what I do from the controller is to call the service and already, then the service is responsible for returning a 200, 404, 500... — Juan Jose Garcia, May 24 '22 at 23:41
That is wrong. Your service shouldn't be tied to the web, the controllers responsibility is to convert the service answer to something suitable for the web and to convert the input from the web into something the service can use. There should be only conversion logic (more or less) in your controller. All business logic should reside in services. Now you have a service which is useless when using from a message queue, soap webservice etc. while the idea is that you should be able to reuse that. — M. Deinum, May 25 '22 at 05:34

score 2 · Answer 1 · edited May 24 '22 at 21:01

As you said, I will assume that you know what the error is and how to resolve it. A circular reference is bad for the following reason:

Spring loads beans the moment you start the project, meaning it loads each bean in the correct order so it can load all beans and references them successfully. If you have a circular reference Spring won't know with which bean to start first, and so the error occurs. It's about how Spring works.

I also had this error in my current project and you are not limited to not making the circular reference, you just need to instruct Spring, so that it knows how to handle each bean in these cases.

score 2 · Accepted Answer · 2023-02-16T14:42:24.830

2

The only reason why you may need circular dependency is the case when you want to access to "this" as to a bean to trigger annotated method logic.

For example if you have two methods "foo" (annotated with @Transactional) and "bar" (invokes "foo" within). You will have to use self injection to trigger transaction in case of invocation bar>foo (selfBean.foo() instead of this.foo()).

Also you can use @Lasy for self injection to avoid the circular dependency error.

But it's a pretty ugly solution and you should avoid it if it's possible. It depends on the situation, may be it's possible to split logic to different services or use TransactionTemplate.

edited Feb 16 '23 at 14:42

answered May 24 '22 at 20:58

As you just said, for example if I remove the "@Transactional" to the createNewMedicalRecord method I get the following error with the saveMedicalRecord method "Methods should not call same-class methods with incompatible '@Transactional' values" and the only way it disappeared was making the circular reference or putting @Transactional to the parent method and I really do not understand why. and why it is solved in those two ways previously mentioned. – Juan Jose Garcia May 24 '22 at 23:48
Do you use cglib? https://stackoverflow.com/questions/3423972/spring-transaction-method-call-by-the-method-within-the-same-class-does-not-wo – May 25 '22 at 05:26
It's expected that @Transactional is ignored when you access to object method without proxy object (bean object). It happens when you invokes an object method within the object. – May 25 '22 at 05:31

score 1 · Answer 3 · answered May 24 '22 at 20:14

Well, imagine this: You get a new phone, you are all excited about it. You want to unlock it, but it protectd with password. The password is available inside the notes in the locked phone.

So, you want to use your phone, for it you need the pass. You want the pass, for it you need to unlock the phone. You want to use your phone, for it you need your pass... Etc.

The same happens when you have circular references, for A you need B, for B you need A, so you cannot create A and cannot create B either and cannot proceed.

How bad is a service circular reference?

3 Answers3