MongoTemplate uses the name of the POJO object instead the DB field in findById [DATAMONGO-1426]

[spring-projects-issues]

Jose Luis opened DATAMONGO-1426 and commented

Hi

In MongoTemplate findById it uses the POJO property name to build the query instead of the field name in the database.

public <T> T findById(Object id, Class<T> entityClass, String collectionName) {
		MongoPersistentEntity<?> persistentEntity = mappingContext.getPersistentEntity(entityClass);
		MongoPersistentProperty idProperty = persistentEntity == null ? null : persistentEntity.getIdProperty();
		String idKey = idProperty == null ? ID_FIELD : idProperty.getName();
		return doFindOne(collectionName, new BasicDBObject(idKey, id), null, entityClass);
	}

if the @Id is pressent, then the query issues the field name in the POJO instead in the db side, for instance, if we'd have this class:

public class Customer
{
      @Id
      String customerId;
}

then the query of findOne would be { customerId : "12345" } instead of { _id : "12345"}

save is working correctly, and uses the _id field. Actually removing the @Id, makes it work correctly. (String idKey = idProperty == null ? ID_FIELD : idProperty.getName();).

so actually using idProperty.getFieldName() should fix it.

Also exists fail, a object recently created does not exists.

---

Affects: 1.9.1 (Hopper SR1)

0 votes, 5 watchers

[spring-projects-issues]

Jose Luis commented

Please note that
exists fails if you don't add the @Id annotation with a Null pointer exception.

java.lang.NullPointerException
	at org.springframework.data.mongodb.repository.support.MappingMongoEntityInformation.getIdAttribute(MappingMongoEntityInformation.java:101)
	at org.springframework.data.mongodb.repository.support.SimpleMongoRepository.getIdCriteria(SimpleMongoRepository.java:129)
	at org.springframework.data.mongodb.repository.support.SimpleMongoRepository.getIdQuery(SimpleMongoRepository.java:125)
	at org.springframework.data.mongodb.repository.support.SimpleMongoRepository.exists(SimpleMongoRepository.java:139)

[spring-projects-issues]

Oliver Drotbohm commented

If you annotated a field with @Id it will become the identifier field of the MongoDB document which natually is _id. See this section of the reference documentation for details.

Inclined to close this one as works as designed. In case you still think anythings broken, feel free to add a minimal test case showing what you expect to happen and what actually happens instead

[spring-projects-issues]

Jose Luis commented

Is what I'm trying to explain, it does not work as explained. If you use @Id, then it does use in the *query * to the the _id field, but the name of the java pojo, so of course no document is returned, because not a single document has that field.

[spring-projects-issues]

Jose Luis commented

Hi Oliver,

please check this unit tests, both method fails. (funny you rename Id to myId and works)

@Document(collection = "customer")
public class Customer
{

    @Id
    public Integer Id;

    public  String name;

}

@Repository
public interface CustomerRepository extends MongoRepository<Customer, Integer> {

}

public class CustomerRepositoryTest {

    @Autowired
    private CustomerRepository repository;

    private final static Logger logger = LoggerFactory.getLogger(CustomerRepositoryTest.class);

    private Integer key = 12345;

    @Test()
    public void findByIdTest() {

        Customer c = new Customer();
        c.Id = key;

        repository.save(c);

        Customer n = repository.findOne(key);
        assert(n != null);
    }


    @Test()
    public void existsTest() {

        Customer c = new Customer();
        c.Id = key;

        repository.save(c);

        boolean exists = repository.exists(key);
        assert(exists);
    }
}

[spring-projects-issues]

Jose Luis commented

Please check comments

[spring-projects-issues]

Oliver Drotbohm commented

Just a quick heads up as I played with that a bit. The culprit seems to be that your identifier property starts with an uppercase letter. Our query translation mechanism uses the infrastructure that constructs path expressions from camel case expressions and thus uncapitalizes the field name which then in turn causes the property not to be found on query creation. So far that hasn't been a problem apparently as hardly anyone chooses to start property names with a capital letter. The test cases work fine if you switch to a proper property name and tweak the first assertion to actually use ….equals(…) (assuming equals is properly implemented).

I'll see how we can work around this issue. In the mean time, just stick with Java coding conventions for field names

[spring-projects-issues]

Jose Luis commented

Hi Oliver,

just removing the @Id makes it work also, but yes, I moved everything to lower case, my point anyway is, if you have a look at this line of code:

String idKey = idProperty == null ? ID_FIELD : idProperty.getName();
return doFindOne(collectionName, new BasicDBObject(idKey, id), null, entityClass);

I think it should be

String idKey = idProperty == null ? ID_FIELD : idProperty.getProperty();
return doFindOne(collectionName, new BasicDBObject(idKey, id), null, entityClass);

in other words, If the idProperty is defined, use the name in the DB (_id) for that property (field), not the name on the POJO class (CustomerId). I think that should be the case in any scenario no? At the end idKey variabel is used only for the query, so it can never be a POJO name no?

Rgds

[spring-projects-issues]

Oliver Drotbohm commented

No it shouldn't. You're missing quite a few implementation details downstream. The query created here has to use the property name as only that will allow the query mapping applied downstream to map the property to the corresponding field name correctly. Simply changing that line of code will not affect the exists(…) call as it suffers from the same problem. This has to be solved more fundamentally.

Removing @Id works as then your class doesn't carry an identifier property from the perspective of the mapping infrastructure and thus an auto-generated _id field is used

[spring-projects-issues]

MyronLee commented

The 'idKey' is only used by 'new BasicDBObject', and the id field name of mongodb must be '_id', so the method should be:

public <T> T findById(Object id, Class<T> entityClass, String collectionName) {
	return doFindOne(collectionName, new BasicDBObject(ID_FIELD, id), null, entityClass);
}

[spring-projects-issues]

MyronLee commented

If it have to use 'idProperty', it should be:

String idKey = idProperty == null ? ID_FIELD : idProperty.getFieldName();

Because the comment on 'org.springframework.data.mongodb.core.mapping.MongoPersistentProperty.getFieldName()' was 'Returns the name of the field a property is persisted to.'.
and the value returned by 'getName' is only set at 'org.springframework.data.mapping.model.AbstractPersistentProperty.AbstractPersistentProperty(Field, PropertyDescriptor, PersistentEntity<?, P>, SimpleTypeHolder)':

this.name = field == null ? propertyDescriptor.getName() : field.getName();

[spring-projects-issues]

TruthNZ commented

Under what case would the id field (on the Mongo side) not be "_id"?