Id on nested documents are mapped to _id [DATAMONGO-2496]

[spring-projects-issues]

trafficstars

provirus opened DATAMONGO-2496 and commented

Having the following classes:

@Document
class Parent {
  @Id
  private String id;

  private Child child;

  // Getters and setters
}

class Child {
  private Long id;
  private String name;

  // Getters and setters
}

Spring Data produces the following document:

{
  "_id": ...,
  "child": {
    "_id": 10,
    "name": "Foo"
  }
}

  The expected document is:

{
  "_id": ...,
  "child": {
    "id": 10,
    "name": "Foo"
  }
}

Since there is no need for id mapping on the child document

---

Affects: 2.2.5 (Moore SR5)

Issue Links:

• DATACMNS-1714 Static Entity Metadata

[spring-projects-issues]

provirus commented

I debugged and found this logic:

MappingMongoConverter.writeProperties is going through all the properties
    when is a class -> writePropertyInternal
      which considers it as an entity = BasicMongoPersistentEntity (which looks strange since there is no @Document on that class)
      and calls writeInternal
        which considers idProperty = "id" (this is wrong)
      	

[spring-projects-issues]

Christoph Strobl commented

This behaviour has been the same since the early days of Spring Data MongoDB. Therefore a change would certainly affect many existing applications.

Please use the @Field annotation to customize naming.

class Parent {
    @Id String id;
    Child child;
}

class Child {
    @Field("id") String id;
}

[spring-projects-issues]

provirus commented

Hi,

I cannot use @Field in the child since the child is a normal POJO from a separate project that doesn't have any Spring dependencies.

I know that the fix to this major bug would affect current installation due to data change. In that case, the fix needs to support both by choosing different implementation:

• correct way
• legacy way

And people can migrate when they are ready.

thanks

[ivan-zaitsev]

What is the status of this issue?

[kkurczewski]

I can try to provide fix for this in my free time if there is willingness from Spring Data team to merge such change.

Personally I consider this as a bug which needs to be fixed. Fixing bug by workaround above not works for all cases (e.g. models exposed as a library) and is not nice at all. If someone already decided to "patch" issue using method above then they even won't notice change after fixing it.

On the other hand, if someone already expects nested ids to be handled like @Id then it would be fair to expect from them to annotate such nested fields and not rely on undocumented side effect of a bug.

[sjucovschi]

I believe that this is a bug and needs to be addressed. Today we lost almost half a day trying to figure out why one of our queries did not work, just to find that a nested field called "id" was renamed to "_id". The worst thing was that we had integration tests with an in memory Mongo, but these were working because saving the initial test data went through Spring Data and the generated documents had "_id" instead of "id". It drove us nuts, until we logged the queries and saw the issue.

[takanuva15]

I missed this issue when searching and filed my own at #4596. I've got 200+ model classes I'm migrating to Spring Data and I quickly found out that Spring is naming embedded id fields as _id, which doesn't work with our existing data stored as id. Going through our 200+ models and adding @Field("id") everywhere is extremely cumbersome just to make Spring happy, which is why I was asking to add a new option to disable Spring's behavior of mapping "id" to "_id" and instead, just use @Id as the reference whether to convert a field to _id during database writes.

2 questions here:

1. Is there a way to override MappingMongoConverter to not treat "id" fields as "_id" across all models? (ie just use the @Id annotation?)
2. @christophstrobl In terms of a proper fix, how about:

   • we add a property like spring.data.mongodb.enable-automatic-id-field-mapping=true

   • Inside MappingMongoConverter.writeInternal, we can check that property's value and, if true, we can automatically convert "id" fields to "_id". Otherwise, only @Id annotations will get considered.

   • For Spring Data v4, we can set this property's value to true by default so it doesn't break existing code.

     • For Spring Data v5, we can set this property's value to false by default so future Spring Mongo users don't get surprised by embedded id fields getting converted to _id

   • Alternatively, we could keep the default automatic id-field-mapping behavior, but the class must have the @Document annotation on it in order to be eligible for automatically converting the id field to _id. Thus embedded "id" fields will be skipped over.

I know Spring's core tenet is "convention over configuration", so making users explicitly add annotations is undesirable, but having embedded "id" turned into "_id" is also a confusing (& time-costly) bug that doesn't make sense in the canonical usage of Mongo.

Since this ticket has "ideal-for-contribution", as long as someone from Spring can agree on the approach so the fix PR doesn't get immediately rejected, I think anyone could be willing to help resolve this.

[christophstrobl]

@takanuva15 we've had similar discussions in the team already a couple of times. Using the @Document annotation to identify the root and applying "default" id mapping only to annotated types was amongst the preferred solutions. Having an additioal opt in flag is mandatory for a change like this since the current behaviour does not demand types to be annotated. The change will also require updates to the query & update mapping and extensive testing. PRs are always welcome :)