Cannot write integration test: 'Lock cannot be acquired after being cancelled' (regression since Mongock 4)

[pkubowicz]

Description

I write integration tests of migrations written using Mongock: I execute them using MongockApplicationRunner.
What works in Mongock 4 crashes in Mongock 5. During Spring context load, Mongock enters a state where it refuses to run any further migrations.

PRIORITY

NORMAL

Version and environment

Mongock

• Mongock version: 5.0.36
• mongock-springboot + mongodb-springdata-v3-driver
• annotation approach

Environment

• Spring Boot 2.6.4
• Infrastructure: JUnit Jupiter + Spring Test

Steps to Reproduce

1. Write a @SpringBootTest
2. Let Spring inject MongockApplicationRunner into your test
3. Use MongockApplicationRunner to execute your migration

Behaviour

Expected behavior: Mongock runner executes my migration

Actual behavior: Runner does not even start, it crashes on acquiring lock:

2022-03-02 20:20:52.237 ERROR 81672 --- [    Test worker] i.m.r.core.executor.MongockRunnerImpl    : Mongock did not acquire process lock. EXITING WITHOUT RUNNING DATA MIGRATION

io.mongock.driver.api.lock.LockCheckException: Lock cannot be acquired after being cancelled
	at io.mongock.driver.core.lock.LockManagerDefault.initialize(LockManagerDefault.java:374) ~[mongock-driver-core-5.0.36.jar:na]
	at io.mongock.driver.core.lock.LockManagerDefault.acquireLockDefault(LockManagerDefault.java:145) ~[mongock-driver-core-5.0.36.jar:na]
	at io.mongock.runner.core.executor.operation.change.MigrationExecutorBase.executeMigration(MigrationExecutorBase.java:96) ~[mongock-runner-core-5.0.36.jar:na]
	at io.mongock.runner.core.executor.operation.change.MigrationExecutorBase.executeMigration(MigrationExecutorBase.java:46) ~[mongock-runner-core-5.0.36.jar:na]

The exception is misleading: nothing is ever cancelled. The previous run of Mongock (during Spring context load) finishes successfully.

io.mongock.driver.core.lock.LockManagerDefault#initialize documentation claims the method is 'idempotent'. Of course it isn't because it starts with:

    if (releaseStarted) {
      throw new LockCheckException("Lock cannot be acquired after being cancelled");
    }

releaseStarted is only set to true, never set to false. This means in Mongock 5, the lock manager becomes useless the moment after it's started for the first time. There is no way to correctly use it again.

How often the bug happens: Always

Link to repository using Mongock

https://github.com/pkubowicz/mongock5-springboot-test

A GitHub action proves that 100% identical code works in Mongock 4 but crashes in Mongock 5.

Additional context

Maybe this is not the most effective way of testing a migration, but the Mongock documentation does not offer any advice. Since my real-life system is built on Spring Boot, it seems logical to write tests in Spring Boot way. I have to control running migrations in my test, because I need to save initial state into the database.

[dieppa]

Hello @pkubowicz ,

this is the expected behaviour. A MongockRunner is expected to be used just once. After finished, it cannot be re-executed.

Anyway, I don't see why an application needs to run multiple times the same MongockRunner instance.Unless we are speaking about a multi-tenant scenario, for which Mongock provides a built-in solution.

The same way an integration test shouldn't neither and I believe there are ways to solve this issue. Let me take a look to the sample project you provide, between tomorrow and Friday, and I will try to come up with solution

[dieppa]

Hello @pkubowicz , we received a priority bug for a client that has took longer that we expected. We'll try to take a look and get back to you as soon as possible.

[pkubowicz]

Hello, do you have any suggestions? Just reminding, it seems that in Mongock 5, there is no way of testing migrations.

[dieppa]

Being frank, we have been to busy, I totally forgot about this.

But. what do you mean with "there is no way of testing migrations"? The only limitation you have is that a MongockRunner can only be executed once. But I don't see why that's an obstacle to test migration.

Anyway, I think we can have some free time this week to look at this. We'll try to and will let you know

[pkubowicz]

So how do you write your tests if:

• a Spring test begins with loading Spring context
• loading Spring context starts MongockRunner
• MongockRunner is not usable after being started
• so when a test method is started, test code cannot use MongockRunner to execute migrations

[dieppa]

Hello @pkubowicz .

Aplogies for the delay - I finally got the chance to take a look at your project. I forked it and did a PR with some suggested changes on how it is best practice to write the tests with Mongock.

During the analysis, I found out that you are loading the SpringContext - which already executes the Mongock migration - then you clean the data and execute Mongock again.

This is not the best approach for Mongock given the migration will already run during the loading time of the Spring Boot Application.

I'd suggest to do segregate the test cases:

• Have a unit test for that can test the changeUnit independently and
• Have an integration test that can test the entire Mongock process independently from Spring boot
• Have an integration test the entire Spring boot application with the Mongock migration in it.

This issue mainly covers the last one, testing the entire Spring boot application so that the Mongock runner again is not executed again.

To summarise: run the Spring boot application, with the Mongock process, ensuring some preconditions (in this case some pre-populated data in the database). This can be achieved in different ways, depending on the case, here you can create a InitializingBean like this:

package com.example.mongock;

@TestConfiguration
public class InitializerContext {

    @Bean
    @Order(1)
    public InitializingBean initData(MongoDatabaseFactory mongoDatabaseFactory) {
        return new InitializingBean() {
            @Override
            public void afterPropertiesSet() throws Exception {

                MongoDatabase mongoDatabase = mongoDatabaseFactory.getMongoDatabase();
                mongoDatabase.getCollection("user").deleteMany(new Document());
                mongoDatabase.getCollection("user").insertMany(
                        Stream.of("alice", "bob", "chris")
                                .map(name -> new Document().append("email", name + "@mail.com"))
                                .collect(toList())
                );
                mongoDatabase.getCollection("summary").deleteMany(new Document());
            }
        };
    }
}

and adding @Import(InitializerContext.class):

@SpringBootTest
@Import(InitializerContext.class)
@ContextConfiguration(initializers = MongoInitializer.class)
public abstract class BaseCountingMigrationTest {

Then you can remove the MongockRunner and leave the test as simple as:

    @Test
    public void works() {
        var found = mongoDatabase.getCollection("summary").find();
        assertThat(found).hasSize(1);
        //noinspection ConstantConditions
        assertThat(found.first().getInteger("userCount")).isEqualTo(3);
    }

I hope this clarifies your case :)

[pkubowicz]

Thank you for the answer.

I'd suggest to do segregate the test cases:

I'm sure you already use Mongock in real-life projects. Is the segregation you suggest the one you use in your projects?

unit test for that can test the changeUnit independently

I used to have tests like this. For me the test was like - skip annotations, and manually call the migration as plain Java code:

void myTest(@Autowired MongoTemplate template) {
  new CountingMigration().run(template);
}

Very quickly I came into a situation where all tests passed, but the migration did not happen on production. The reason was that my migration was not annotated at all.

So I don't want any tests where I simulate what Mongock runner does. I don't want to follow changes in Mongock API and internals, and update my tests as annotations and their handling change over time. I want a black-box test that tells Mongock "here is the migration, run it", and inspect what is in the database.

This black-box approach (showed in a simplified way in my example repo) worked very well in Mongock 4. It allowed me to focus on data and not on the migration tool. But I am unable to continue using this approach.

Maybe I misunderstood what you suggest and your way works well. Can you share a simplified test code?

Sorry for bothering you, but, as I mentioned before, Mongock documentation offers no suggestions how to approach testing. I think it should provide even a tiniest example of a migration test so that users know where to start.

Imagine Project Reactor documentation with no word on how to approach testing when your code uses Reactive Streams.

InitializingBean

I don't want to write tests this way. This bean contains part of test scenario: it knows which elements populate the database before the migration is started. Just a few of numerous drawbacks of this approach:

• unreadable and unmaintainable: I need to read two files to understand what are inputs and outputs of a single simple scenario
• explosion of classes: if I have 2 scenarios: 1) database initially has 1 element 2) database initially has 3 elements, then I need to write 2 beans like this. What if I have 3 migrations, each tested with 2 scenarios? 6 files to add, and later remove if I clean up too old migrations from my code. 6 files which I may forget to remove, and will eat time during compilation, may break when I update major Spring version etc.
• need to make those independent: @TestConfiguration will be picked by all tests. Now I have this InitializingBean for CountingMigration operating on user collection. Imagine I add a GreetingMigration operating on greetings migration. I write GreetingMigrationTest. Spring picks up InitializingBean test configuration, which results in users being put into the database. But GreetingMigration does not know about user collection, there is no sense it doing so. So maybe a Spring profile to separate initializing beans? Ok, it will work, but this is one more thing to think about: add migration, add its own test, add its own initializing bean, add its own Spring test profile.

[dieppa]

Hello again @pkubowicz ,

first thing, we need to create a section in the documentation to provide some advices to how to test it. However, there is no one way of testing and depends very much on what framework the team is using and actually how that framework is used, as there are many ways of using Spring boot, for example. Anyway, we really need to provide some help on that.

Said that, like with any other tool or project there are different levels of testing. The one you mentioned without annotations, it's basically a unit test for the changeUnit. That can be very useful in specific scenarios, but obviously is far from being enough.

In simple words you are just trying to provide an integration test, in which you run spring boot and the migration is supposed to be part of the startup. You don't care, you just need to check the output is in the database as expected. Although still insufficient as if you don't set any precondition you are assuming the database is always empty, we can start from that. The problem with the test you had is that you were running Spring boot(and Mongock with it), then in your test you clean the database, populate some data into the database, run manually Mongock again and then check the database state.

That's not ideal and it's an approach somehow in the middle. Or you either run manually a MongockRunner instance without Spring boot or you run Spring boot with Mongock, but not both. I really recommend the second one, as it's more complete.

To achieve you can see the PR I raised. You just need to remove the @TestConfiguration(and obviously update the checks accordingly in your testing, as it now changes a bit). With this approach, you can run Springboot(Mongock will be executed as part of the application startup) and you can check the expected output of tour changeUnits.

But, as said, you very likely want to test it with some pre-populated data. You can choose the way you want, I think to create an InitializingBean for test is a decent approach. To be honest, I don't fully agree all your points you make about it:

• unreadable and unmaintainable: I need to read two files to understand what are inputs and outputs of a single simple scenario

Not really, you can set that TestConfiguration class(with your the bean) in the actual test class

• Explosion of classes: if I have 2 scenarios: 1) database initially has 1 element 2) d...

This is actual true. But it's more a limitation(or design) of the test framework. However, I see 
a cool and easy way to get around this , but it will require a new release. 

Being totally honest, we cannot commit to a new release this week, but we can have it soon. 

• need to make those independent: @TestConfiguration will be picked by all tests...

Also not really. You don't need to share the @TestConfiguration class among all your tests, you
 can have it locally per test

My suggestion is to apply something similar to the PR I raised and then do the small change once the new release provide that feature(with documentation). Otherwise, you can just wait until it's released.

[dieppa]

@pkubowicz ,

I have thinking and while we develop this small mechanism to make it easier(plus documentation) and assuming that you don't want to use that configuration I provided, you can try to keep your current code and in your test, instead of autowiring the MongockRunner, autowire the RunnerSpringbootBuilder.

So instead of

    @Autowired
    private MigrationExecutor migrationExecutor;

you should use

    @Autowired
    private RunnerSpringbootBuilder runnerSpringbootBuilder;

and then in you test:

    @Test
    public void works() {
        mongoDatabase.getCollection("user").deleteMany(new Document());
        mongoDatabase.getCollection("user").insertMany(
                Stream.of("alice", "bob", "chris")
                        .map(name -> new Document().append("email", name + "@mail.com"))
                        .collect(toList())
        );
        mongoDatabase.getCollection("summary").deleteMany(new Document());

        System.out.println("RUNNING MIGRATIONS");
        runnerSpringbootBuilder.buildRunner().execute();
        //migrationExecutor.runAllMigrations(mongoDatabase, migrationCollectionName);

        var found = mongoDatabase.getCollection("summary").find();
        assertThat(found).hasSize(1);
        //noinspection ConstantConditions
        assertThat(found.first().getInteger("userCount")).isEqualTo(3);
    }

This is still not right, as Mongock is running twice, but it should work. And you can perform as many test as you wish.

Please let me know if it works and on our side will notify you when the release is done as the mentioned mechanism is available 😉

[pkubowicz]

I tried RunnerSpringbootBuilder. If I execute what you suggest (runnerSpringbootBuilder instead of migrationExecutor), I get:

MigrationExecutorBase      : Mongock skipping the data migration. All change set items are already executed or there is no change set item.

Which is no surprise, since the migration is marked as 'applied' before the test start, at the time when the Spring context starts.

To be able to execute a migration after the context is loaded, I need to clean change-log-repository-name collection. Which is exactly why I have MigrationExecutor: it first cleans the collection, then calls a Mongock runner.

I tried first cleaning the collection, and then starting Mongock with RunnerSpringbootBuilder. But then we return to the starting point:

MongockRunnerImpl    : Mongock did not acquire process lock. EXITING WITHOUT RUNNING DATA MIGRATION
io.mongock.driver.api.lock.LockCheckException: Lock cannot be acquired after being cancelled
	at io.mongock.driver.core.lock.LockManagerDefault.initialize(LockManagerDefault.java:374) ~[mongock-driver-core-5.0.36.jar:na]

and, as I have written:

releaseStarted is only set to true, never set to false. This means in Mongock 5, the lock manager becomes useless the moment after it's started for the first time

[pkubowicz]

I managed to write a test I want using StandaloneRunner.

In short:

• I use mongock-standalone as a test-only dependency
• I declare a @SpringBootConfiguration to prevent Spring Boot test from picking up @EnableMongock from the production configuration, and to prevent starting mongock before the test logic
• I inject values from application.yml to make MongockStandalone configuration as close to the production configuration as possible

https://github.com/pkubowicz/mongock5-springboot-test/pull/2/files

I think I can use this PoC as an example how to update my real project. If you find anything there useful, feel free to use it when writing the documentation.

[dieppa]

Hello @pkubowicz ,

we have release some utility classes for testing, as well as added examples of unit and integration tests in example project MongoDB/quick-start and added to the documentation.

If you find anything, please let us know.

Regards