[NAE-2342] Improve Quartz configuration#404
Conversation
… quartz mongo write concern policy
WalkthroughAdds a new configuration property Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/netgrif/application/engine/configuration/quartz/QuartzConfiguration.java (1)
90-91: Pre-existing bug: SecondsetQuartzPropertiescall overwrites the first.Line 91 completely overwrites the properties set on line 90. This means all properties loaded from
quartz.propertiesfiles viaquartzProperties()are discarded, and only the programmatically-defined properties (lines 74-85) are applied.The new
mongoOptionWriteConcernWproperty will work because it's in thepropertiesobject used on line 91, but any file-based Quartz configuration is currently being ignored.🔧 Proposed fix to merge properties
SchedulerFactoryBean schedulerFactory = new SchedulerFactoryBean(); schedulerFactory.setApplicationContext(applicationContext); schedulerFactory.setAutoStartup(false); schedulerFactory.setApplicationContextSchedulerContextKey("applicationContext"); - schedulerFactory.setQuartzProperties(quartzProperties()); - schedulerFactory.setQuartzProperties(properties); + Properties mergedProperties = quartzProperties(); + mergedProperties.putAll(properties); + schedulerFactory.setQuartzProperties(mergedProperties); jobFactory.setApplicationContext(applicationContext);
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/netgrif/application/engine/configuration/quartz/QuartzConfiguration.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-18T19:20:32.658Z
Learnt from: Smotanka
Repo: netgrif/application-engine PR: 401
File: src/main/java/com/netgrif/application/engine/elastic/service/ElasticsearchQuerySanitizer.java:57-66
Timestamp: 2025-12-18T19:20:32.658Z
Learning: When performing multiple string replacements in Java files (e.g., sanitization/escaping logic like ElasticsearchQuerySanitizer), prefer using StringUtils.replaceEach() over chaining multiple StringUtils.replace() calls. This avoids cascading replacements where the result of one replacement triggers another replacement and potentially corrupts the final output. Apply this pattern to other similar sanitization code across the codebase as appropriate.
Applied to files:
src/main/java/com/netgrif/application/engine/configuration/quartz/QuartzConfiguration.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
- GitHub Check: task-list-completed
🔇 Additional comments (2)
src/main/java/com/netgrif/application/engine/configuration/quartz/QuartzConfiguration.java (2)
41-42: LGTM! New configuration property for MongoDB write concern.The property injection with default value
"majority"is appropriate. MongoDB'smajoritywrite concern is a good default for data durability in clustered environments.Valid values to document for users include:
"majority","W1","W2","W3", or numeric strings. Consider adding a note in the project documentation about accepted values to prevent runtime configuration errors.
80-80: Property addition looks correct.The new write concern property is correctly added to the Quartz job store configuration.
Added new property nae.quartz.mongoOptionWriteConcernW for quartz mongo write concern policy
Description
Added new property nae.quartz.mongoOptionWriteConcernW for quartz mongo write concern policy
Implements NAE-2342
Dependencies
Third party dependencies
Blocking Pull requests
How Has Been This Tested?
This improvement was tested on local device and checked if MongoDB write concern changes for Quartz
Tested with properties:
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc.>
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.