Is it good practice to access database in mapping between object

Question

Situation

I receive message from a 3rd party, and these messages are use to create or update database information.

Message and data to be updated can be de-serialize in object form, but are not exactly identical, so I need to use an object for mapping the properties of the incoming message object to the resulting entity.

The trick is that some of these properties must result in object retrieved from the database.

I have a set of DAO to handle all my database operations

Example (in Java code)

public class CarMessage {
    public String carType;
    public String carOwnerType;
    public String carOwnerId;
}

pubic class CarEntity {
    public String carType;
    public Person carOwner;
}

Question

Does the code doing the mapping can do the query in database ? Or do the database information retrieving should be done outside of the mapping code ?

In other words, should I do this :

public class CarMapper {

    private PersonDao personDao;

    public CarEntity mapMessageToEntity(CarMessage message) {
        CarEntity entity = new CarEntity();
        entity.carType = message.carType;
        entity.carOwner = personDao.retrievePersonFromDatabaseByTypeAndId(message.carOwnerId, message.carOwnerId);
        return entity;
    }   
}

public class MessageHandler {

    private CarMapper carMapper;
    private CarDao carDao;
    public void onMessage(CarMessage message) {
        CarEntity carEntity = carMapper.mapMessageToEntity(message);
        carDao.save(carEntity);
    }
}

or this :

public class CarMapper {


    public CarEntity mapMessageToEntity(CarMessage message) {
        CarEntity entity = new CarEntity();
        entity.carType = message.carType;

        return entity;
    }   
}

public class MessageHandler {

    private CarMapper carMapper;
    private CarDao carDao;
    private PersonDao personDao;

    public void onMessage(CarMessage message) {
        CarEntity carEntity = carMapper.mapMessageToEntity(message);
        carEntity.carOwner = personDao.retrievePersonFromDatabase(message.carOwnerId, message.carOwnerId);
        carDao.save(carEntity);
    }
}

Personally, I am more inclined in the second solution (all the DAO in the handler), as it make the error more manageable but I have some doubt and I would like to hear the pros and cons of the community.

Answer 1

I also would go with the second solution, because it gives CarMapper and MessageHandler clearer responsibility boundaries. In the first solution CarMapper maps cars into your internal representation and is responsible to fetch some information from the DB. If you want to test CarMapper in isolation you always have to go through the hassle of mocking you DB access. Where in the second solution you just feed messages to your test instance and assert on the created CarEntitys.

Maybe a proposal for solution 2.b:

To keep MessageHandler focused on the general process flow (step one: mapping, step 2: add DB information and step 3: saving) you could introduce a third class (something like CarEntityEnricher) which contains the logic of fetching information from the DB and populating the CarEntity instance.