spring-data-mongodb icon indicating copy to clipboard operation
spring-data-mongodb copied to clipboard

5065 empty map with document reference

Open bamapookie opened this issue 3 months ago • 4 comments

  • [x] You have read the Spring Data contribution guidelines.
  • [x] You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • [x] You submit test cases (unit or integration tests) that back your changes.
  • [x] You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

Copyright date already set to 2025. No new files. No author lists in files I touched, so I left my name out.

Fixes #5065

bamapookie avatar Oct 06 '25 23:10 bamapookie

I added lower level tests in the PR that didn't require the @NonNull annotations to test. They really only caused the issue to fail faster. I can add a test for { map: null }

On Wed, Oct 15, 2025, 9:18 AM Christoph Strobl @.***> wrote:

@.**** commented on this pull request.

Thank you for taking the time to prepare this PR. Kindly let me know if you'd like to add the tests from the review comment. If not, I can take it from here and continue the work.

In spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java https://github.com/spring-projects/spring-data-mongodb/pull/5066#discussion_r2432523881 :

  •   	EmptyMapDocument target = converter.read(EmptyMapDocument.class, document);
    
  • 	assertThat(target.map).isNotNull().isEmpty();
    
  • }
    
  • @Test // GH-5065
    
  • @DisplayName("Converter should read an empty object as an empty map when using @DocumentReference.")
    
  • void readsEmptyMapWithDocumentReferenceCorrectly() {
    
  • 	org.bson.Document document = org.bson.Document.parse("{\"map\":{}}");
    
  • 	DocumentReferenceEmptyMapDocument target = converter.read(DocumentReferenceEmptyMapDocument.class, document);
    
  • 	assertThat(target.map).isNotNull().isEmpty();
    
  • }
    
  • @Test // GH-5065
    
  • @DisplayName("Converter should read an empty object as an empty map with a valis values property when using @DocumentReference(lazy = true).")
    
  • void readsEmptyMapWithLazyLoadedDocumentReferenceCorrectly() {
    
  • 	org.bson.Document document = org.bson.Document.parse("{\"map\":{}}");
    

I think it would also be interesting to have test for { 'map' : null } expecting null for the target.map.

— Reply to this email directly, view it on GitHub https://github.com/spring-projects/spring-data-mongodb/pull/5066#pullrequestreview-3340354529, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOV6PSVBD63KGIQMSPM2OT3XZCSBAVCNFSM6AAAAACIOSR3NKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGNBQGM2TINJSHE . You are receiving this because you authored the thread.Message ID: @.*** com>

bamapookie avatar Oct 15 '25 23:10 bamapookie

I added tests where the map is explicitly assigned null and rebased on the latest main. There is one error occurring on the null assignment to the @DocumentReference Map<>. I also added @DBRef tests as well, though I don't think they are affected by the underlying bug. I'll look into the failing test tomorrow.

bamapookie avatar Oct 16 '25 04:10 bamapookie

Are you still looking in the failing tests, or should we take over?

schauder avatar Nov 03 '25 14:11 schauder

I haven't had any availability to get back to this. I would recommend breaking out the failing tests to a different issue, since I believe they are unrelated.

bamapookie avatar Nov 18 '25 14:11 bamapookie