Mongock Change Units/Logs allows duplicate and empty order strings

[TheSteinn]

Description

Something that is perhaps a bug, but definitely isn't documented, is that Mongock does not throw an exception if two of more change units/logs share the same order string value. I would have expected it to throw an exception similar to Flyway, given the similar use of version/order numbers.

Looking at the code in ChangeLogComparator.java, it looks like if the order string is equal, it uses the change unit/log class's canonical name.

I can't think of a use case for allowing duplicate order strings, but recognise it would be a breaking change to start throwing an exception in such cases. It's something to at least think about for a new version.

An alternative short-term approach could also be to log a (warning) message when change logs/units share duplicate order strings in order to warn a developer.

If this is an intended feature, it should be properly documented

In addition, an empty string for the order value appears to be allowed. This seems contradictory to the documentation, which states it is a mandatory parameter. The change unit ID is also a mandatory parameter, and has checks to ensure it is not null or empty in ChangeLogItem.java. However this is not done for the order parameter.

PRIORITY

Normal

Version and environment

Mongock

• Mongock version: 5.4.0
• Modules involved: mongock-springboot-v3, mongodb-springdata-v4-driver
• How Mongock is used: annotation approach (via @EnableMongock and application.yml)

Environment

• Framework and libraries versions. Especially those that affect directly to Mongock(Spring, Spring data, MongoDB driver, etc.): Spring Boot 3.2.3, spring-boot-starter-data-mongodb 3.2.3 (Spring Data MongoDB 4.2.3, MongoDB driver 4.11.1), spring-boot-starter-jdbc 3.2.3 (Spring JDBC 6.1.4)
• Infrastructure: Kubernetes, Docker, SO, etc.: N/A

Steps to Reproduce

Duplicate order values:

1. Create two migrations with duplicate order string value
2. When the application starts up and migrations are run, both migrations are run, with order dependent on class name

Empty order values:

1. Create a migration with an empty string order ""
2. Start the application

Behaviour

Duplicate order values:
Expected behavior: An exception is throw (if this was not an intended feature)

Actual behavior: Mongock allows the duplicate order values

Empty order values:
Expected behavior: An exception is throw (if this was not an intended feature)

Actual behavior: Mongock allows the empty string order value

How often the bug happens: Always

[TheSteinn]

I don't mind contributing a PR for the empty order string issue, if a chosen solution can be aligned on. Consistency with existing code and documentation would dictate the value should be checked in ChangeLogItem.java, similar to what is done for the id value. However that would cause a breaking change for existing users if they've configured their migrations with empty order strings. I believe there could also be further propagation of changes to ChangeSetItem.java(?), and possibly other classes

[dieppa]

Hello @TheSteinn ,let us understand well this issue and if you are happy to contribute with a PR, that would be amazing!

[dieppa]

Hello @TheSteinn , should I assume you are not performing this PR? Just to doing the backlog cleaning

[TheSteinn]

Hi, last I heard you wanted to get a better understanding of the issue, unless I misunderstood.

There are two related issues here:

1. Two ChangeUnit migrations can have the same order value
2. order is allowed to be an empty or whitespace string

As far as I can tell, fixing either issue would be a breaking change. I'm also somewhat confused on the relationship between ChangeLogItem and ChangeSetItem`, and their usage.