Refactor ObjectMapper configuration for Elasticsearch#337
Refactor ObjectMapper configuration for Elasticsearch#337tuplle merged 2 commits intorelease/6.4.2from
Conversation
Moved the ObjectMapper configuration from ElasticsearchConfiguration to ElasticIndexService. This improves cohesion by localizing the configuration closer to its usage. Removed unused bean, reducing complexity in the configuration class.
WalkthroughRemoves the custom ObjectMapper bean from ElasticsearchConfiguration and moves Jackson JavaTime configuration into ElasticIndexService, which now creates and configures its own ObjectMapper with LocalDateTime serializers/deserializers. ElasticIndexService no longer receives an injected ObjectMapper; other Elasticsearch beans remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as ApplicationContext
participant EIS as ElasticIndexService
participant ES as ElasticsearchRestTemplate
note over C: Bean initialization
C->>EIS: create()
activate EIS
EIS->>EIS: new ObjectMapper()
EIS->>EIS: configureMapper()\\n(register JavaTimeModule,\\nLocalDateTime serializer/deserializer,\\ndisable timestamps)
deactivate EIS
note over EIS,ES: Indexing/search operations use internal mapper
EIS->>EIS: serialize/deserialize LocalDateTime
EIS->>ES: perform ES operations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ 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). (9)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Removed unnecessary imports and unused dependencies from ElasticsearchConfiguration. This simplifies the code, improves readability, and ensures maintainability by decluttering the file.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/netgrif/application/engine/configuration/ElasticsearchConfiguration.java (1)
3-3: Clean up unused imports introduced by the refactorThese imports are not used anymore in this file and can be safely removed.
-import com.fasterxml.jackson.annotation.JsonInclude; - import org.springframework.beans.factory.annotation.Qualifier; - import org.springframework.core.annotation.Order;Also applies to: 21-21, 25-25
src/main/java/com/netgrif/application/engine/elastic/service/ElasticIndexService.java (2)
108-110: Minor: consider reusing this ObjectMapper where feasibleObjectMapper is thread-safe after configuration. Keeping a single instance per singleton service is fine. If you want to avoid extra instantiation elsewhere, you could reuse this mapper in parseAnalysisSettings() instead of creating a new one (not critical).
661-667: Consider excluding nulls from ES documents for leaner payloadsIf the previous configuration (via the removed bean) excluded nulls, add it back here to preserve behavior and reduce index size. Safe for ES indexing and typically desired.
Apply:
+import com.fasterxml.jackson.annotation.JsonInclude; @@ private void configureMapper() { JavaTimeModule javaTimeModule = new JavaTimeModule(); javaTimeModule.addSerializer(LocalDateTime.class, new LocalDateTimeJsonSerializer()); javaTimeModule.addDeserializer(LocalDateTime.class, new LocalDateTimeJsonDeserializer()); objectMapper.registerModule(javaTimeModule); objectMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS); + objectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/com/netgrif/application/engine/configuration/ElasticsearchConfiguration.java(2 hunks)src/main/java/com/netgrif/application/engine/elastic/service/ElasticIndexService.java(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/netgrif/application/engine/configuration/ElasticsearchConfiguration.java (1)
src/main/java/com/netgrif/application/engine/auth/domain/LoggedUser.java (1)
JsonInclude(16-137)
src/main/java/com/netgrif/application/engine/elastic/service/ElasticIndexService.java (2)
src/main/java/com/netgrif/application/engine/elastic/serializer/LocalDateTimeJsonDeserializer.java (1)
LocalDateTimeJsonDeserializer(13-29)src/main/java/com/netgrif/application/engine/elastic/serializer/LocalDateTimeJsonSerializer.java (1)
LocalDateTimeJsonSerializer(11-18)
⏰ 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). (7)
- 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
🔇 Additional comments (2)
src/main/java/com/netgrif/application/engine/elastic/service/ElasticIndexService.java (2)
5-7: LGTM: localized Jackson configuration for ES payloadsMoving the JavaTimeModule and custom LocalDateTime (de)serializers close to where JSON is produced for Elasticsearch is sensible. Keeps ES-specific JSON concerns isolated from the rest of the app.
Also applies to: 12-13
96-96: Verify DI wiring after removing ObjectMapper from ElasticIndexService constructorDropping the ObjectMapper from this constructor tightens its API surface. The automated scan returned no hits for:
@Qualifier("elasticCaseObjectMapper")or@Qualifier("configureMapper")- Calls to
ElasticsearchConfiguration.configureMapper(...)- Manual instantiations via
new ElasticIndexService(...)- Any
@Autowiredor field injections ofObjectMapperexpecting an ES-specific mapperPlease manually confirm that:
- No Spring bean definitions (JavaConfig or XML) still reference the old constructor signature
- Test classes or factory methods aren’t instantiating
ElasticIndexServicedirectly with the removed parameter- Any other modules or components don’t rely on the ES-tailored
ObjectMapperbeing injected here
3 participants
Description
Moved the ObjectMapper configuration from ElasticsearchConfiguration to ElasticIndexService, because its quilified configuration overwrote the base ObjectMapper bean configuration. This improves cohesion by localizing the configuration closer to its usage. Removed unused bean, reducing complexity in the configuration class.
Additional fix to NAE-2136
Dependencies
No new dependencies were introduced
Third party dependencies
No new dependencies were introduced.
Blocking Pull requests
There are no dependencies on other PR.
How Has Been This Tested?
Manually and with unit tests.
Test Configuration
Checklist:
Summary by CodeRabbit