Fix index creation statements in MongoDB DDL script

[yoseplee]

Motivation

• Me and my colleague are recently trying MongoDB as a job repository.
• According to the document I was able to access the example DDL scripts.
• I run the script without reading it carefully, and it failed, and fixed it.
  • Usage of createIndex() was wrong.
  • Name of the index duplicated which made error as well.

What I did

• Fixed createIndex() query.
  • According to the syntax, it should have looked like db.collection.createIndex(<keys>, <options>, <commitQuorum>) but was likedb.collection.createIncex(<name>, <keys>, <commitQuorum>).
• Renamed duplicated index names of BATCH_JOB_EXECUTION collection.

Note

• Unlike the script, I read through the document and found suggestions about indexing metadata tables.
• Please let me know the duplicated name in the script is intended.
• In addition, improving integration test case that is introduced in What's new in Spring Batch 5.2 might be a good idea.

[fmbenhassine]

Hi, thank you for the PR!

Regarding the syntax issue, which version of mongo db are you using? Is it the same as the one in the integration test?

Please let me know the duplicated name in the script is intended.

Indeed, I think the following indexes have duplicate names:

db.getCollection("BATCH_JOB_EXECUTION").createIndex("job_instance_idx", {"jobInstanceId": 1}, {});
db.getCollection("BATCH_JOB_EXECUTION").createIndex("job_instance_idx", {"jobInstanceId": 1, "status": 1}, {});

The second one should be something like job_instance_status_idx. Please update the PR accordingly.

In addition, improving integration test case that is introduced in What's new in Spring Batch 5.2 might be a good idea.

yes probably that was missed, the test should include the index creation statements exactly in the same way as in the script schema-mongodb.js. Please add that to the PR and it should be good to merge.

Many thanks upfront!

[yoseplee]

Hi, thanks for checking this out. Our MongoDB version in use is 6.0.14. But the MongoDB image version that is used in integration test is 8.0.1

I'm not sure such syntax is now available in newer version of MongoDB but I'll check if the original ddl script work in 8.0.1 to make it clear.

[yoseplee]

Hi @fmbenhassine, jobs done. Please review this.

Regarding the syntax issue, which version of mongo db are you using? Is it the same as the one in the integration test?

The syntax issue also happens in MongoDB 8.0.1 which version is in the integration test. Referring to the syntax guidance in MongoDB that I linked earlier, the syntax of createIndex() wasn't changed.

To clarify this, I run the query in MongoDB 8.0.1 and it eventually failed like below.

yosep@AL01983618 run % docker ps
CONTAINER ID   IMAGE         COMMAND                   CREATED       STATUS       PORTS     NAMES
9810389c1c18   mongo:8.0.1   "docker-entrypoint.s…"   2 hours ago   Up 2 hours             mongodb-container

db.getCollection("BATCH_JOB_INSTANCE").createIndex("job_name_idx", {"jobName": 1}, {});
[MongoServerError[TypeMismatch]:](https://github.com/spring-projects/spring-batch/pull/4758#) BSON field 'createIndexes.commitQuorum' is the wrong type 'object', expected types '[int, string, long, double, decimal']

In addition, improving integration test case that is introduced in What's new in Spring Batch 5.2 might be a good idea.

yes probably that was missed, the test should include the index creation statements exactly in the same way as in the script schema-mongodb.js. Please add that to the PR and it should be good to merge.

I've added them into @BeforeEach to ensure that indices in the DDL script will be created in the integration test.

By the way, I somehow feel like all collection names should be used as Consts or something because I found that I was keep copying and pasting them all the time during the work. Collection names are defined among DAO implementations like MongoJobInstanceDao and MongoJobExecutionDao. If you feel the same way, please tell me. I will work on it and make PR for this.

[fmbenhassine]

Thank you for checking the syntax and for updating the PR! Appreciated 👍

The changes LGTM now, rebased and merged as a79b6f7.

By the way, I somehow feel like all collection names should be used as Consts or something because I found that I was keep copying and pasting them all the time during the work. Collection names are defined among DAO implementations like MongoJobInstanceDao and MongoJobExecutionDao. If you feel the same way, please tell me. I will work on it and make PR for this.

I agree. What could be better is probably try to load the script from the schema-mongoldb.js file and execute it as is in the integration test (maybe mongoTemplate#executeCommand or something similar). Anyway, I am open to any improvements.