Why are multiple DTOs altering the session Map?

Question

In My recent project, I am using jQuery Grid, generating dynamic grids when user click on button.

At server side data is manipulated by using the Session.

First I am getting Session object and casting that object to gatepassDTO.

Then I am creating one local object and I am copying properties from old to new object and putting into the Map.

After that I am creating futureDTO object and copying properties from gatepassDTO to futureDTO. If I change anything in futureDTO it effects on the Map that contains object.

Why this happens?

public String addOnemoreGatepass() {

        log.info("----------------Entering method addOnemoreGatepass------------");
        try {
            Object map = session.get("gatepassMap");

            GatepassDTO gatepassDTO = new GatepassDTO();
            gatepassDTO=(GatepassDTO)session.get("gatepassDTO");
            if(map!=null){

                //gatepassMap=Collections.synchronizedMap((HashMap)map);
            }
            if(truckNo!=null){
                gatepassDTO.setTruckNo(truckNo);
            }

                     if(gpDirect!=null){
                GatepassDTO tempDTO=new GatepassDTO();
                copyProperty(tempDTO,gatepassDTO);
            /*  HashMap<String,GatepassDTO> maps=null;
                if(gatepassNo.equals("1")){
                    maps=new HashMap<String, GatepassDTO>();
                local.saveMap(getRequest().getSession().getId(),maps);
                }
                maps=local.loadMap(getRequest().getSession().getId());
                maps.put(gatepassNo, tempDTO);*/
                putCachedObject("SINGLEGATEPASSCACHE", gatepassNo, tempDTO);
                gatepassMap.put(gatepassNo, tempDTO);

                //local.saveMap(getRequest().getSession().getId(),maps);
                session.put("documentType", documentType);
                session.put("gatepassMap", gatepassMap);
                return SUCCESS;
            }
            else{
                GatepassDTO tempDTO=new GatepassDTO();
                copyProperty(tempDTO,gatepassDTO);
                /*HashMap<String,GatepassDTO> maps=null;
                if(gatepassNo.equals("1")){
                    maps=new HashMap<String, GatepassDTO>();
                local.saveMap(getRequest().getSession().getId(),maps);
                }
                maps=local.loadMap(getRequest().getSession().getId());
                maps.put(gatepassNo, tempDTO);*/
                putCachedObject("SINGLEGATEPASSCACHE", gatepassNo, tempDTO);
                gatepassMap.put(gatepassNo, tempDTO);

                //local.saveMap(getRequest().getSession().getId(),maps);
                session.put("documentType", documentType);
                session.put("gatepassMap", gatepassMap);

            }
            GatepassDTO futureDTO=new GatepassDTO();
            copyProperty(futureDTO,gatepassDTO);

            DocumentDTO documentDTO =futureDTO.getDocumentDTO();
            List<GatepassGoodsDTO> goodsList=documentDTO.getGatepassGoodsList();
            int i=0;
            for(GatepassGoodsDTO goodsDTO:goodsList){
                if(goodsDTO.getRemovalType()!=null&&(goodsDTO.getRemovalType().equals("Quantity")||goodsDTO.getRemovalType().equals("Weight"))){
                goodsDTO.setPrevRemovalType(goodsDTO.getRemovalType());
                goodsDTO.setPrevGPQuantity((goodsDTO.getRemovalQuantity()!=null?goodsDTO.getRemovalQuantity():0));
                goodsDTO.setPrevGPWeight((goodsDTO.getRemovalWeight()!=null?goodsDTO.getRemovalWeight():0));
                goodsDTO.setBalanceQuantity(goodsDTO.getBalanceQuantity()-goodsDTO.getRemovalQuantity());
                goodsDTO.setBalanceWeight(goodsDTO.getBalanceWeight()-goodsDTO.getRemovalWeight());
                goodsDTO.getVehicleDetailsList().clear();
                goodsDTO.setVehicleDetailsList(goodsDTO.getDeletedVehicleDetailsList());
                goodsDTO.setDeletedVehicleDetailsList(new ArrayList<GatepassVehicleDTO>());
                goodsDTO.setRemovalType("");
                goodsDTO.setRemovalQuantity(0);
                goodsList.set(i, goodsDTO);}
                else{
                    goodsList.set(i, goodsDTO);
                }
                i++;
            }
            documentDTO.setGatepassGoodsList(goodsList);
            documentDTO.setContainerList(deletedContainerModel);
            futureDTO.setDocumentDTO(documentDTO);
            futureDTO.setTruckModel(new ArrayList<GatepassTruckDTO>());
            session.put("gatepassDTO",futureDTO);
            // setDocumentModel(null);
            // setGridModel(null);
            deletedVehicleModel.clear();
            deletedContainerModel.clear();
            // manifestNo=null;



        } catch (Exception e) {
            log.info(e.getMessage());
        } 
        log.info("---------------Ending method addOnemoreGatepass----------------");
        return SUCCESS;
    }


private void copyProperty(GatepassDTO tempDTO,GatepassDTO gatepassDTO){`enter code here`
        tempDTO.setDocumentDTO(gatepassDTO.getDocumentDTO());
        tempDTO.setTruckNo(gatepassDTO.getTruckNo());
    }

Why this problem occurs? Is this core Java problem or Struts2 JSON problem?

What is the problem? You mean if you change, for instance, the "DocumentDTO" of your futureDTO, it also affects the object in session? — mael, Jul 16 '13 at 06:43
yes?if iam changing future dtoDocument datails it also changes Hasmap contains tempDTO.I tried same thing in core java it will work fine but in my project — user1965582, Jul 16 '13 at 09:12

score 1 · Answer 1 · answered Jul 16 '13 at 10:14

You are working with references (think to them as pointers).

When you get the object from the Session Map, you are getting its reference (even the Map is containing only the reference, not the real object)

gatepassDTO = (GatepassDTO)session.get("gatepassDTO");

Then you instantiate tempDTO, and assign the reference of gatepassDTO.getDocumentDTO() (that is a reference itself) to tempDTO

GatepassDTO tempDTO = new GatepassDTO();
copyProperty(tempDTO,gatepassDTO);
// that inside inside copyProperty does:
tempDTO.setDocumentDTO(gatepassDTO.getDocumentDTO());

Then you instantiate futureDTO, and assign AGAIN the reference of gatepassDTO.getDocumentDTO():

GatepassDTO futureDTO = new GatepassDTO();
copyProperty(futureDTO,gatepassDTO);
// that inside inside copyProperty does:
futureDTO.setDocumentDTO(gatepassDTO.getDocumentDTO());

At this point, if you call .getDocumentDTO().setSomething(); from gatepassDTO, or from tempDTO, or even from futureDTO, you are ALWAYS altering the same physical object, that is still (referenced) inside the Map.

Actually, copyProperty isn't copying anything.

This doesn't happen with primitive types, line int and char, nor with immutable objects, like Integer and String;

But this will always happen with all the other objects, like the one you coded, DocumentDTO.

To prevent this kind of problems, you need to return defensive copies of your objects, that is the preferred way, or at least to manually copy each properties, like this:

private void copyProperty(GatepassDTO tempDTO,GatepassDTO gatepassDTO){
    DocumentDTO src = gatepassDTO.getDocumentDTO();
    DocumentDTO dest = new DocumentDTO();
    dest.setSomething(src.getSomething());
    dest.setSomethingElse(src.getSomethingElse());
    dest.setEtc(src.getEtc());
    /* go on like that with primitives or immutables, 
    and instantiate new objects for each mutable object you find */

    tempDTO.setDocumentDTO(dest);
    tempDTO.setTruckNo(gatepassDTO.getTruckNo());
}

Obviously if you put this kind of logic in the getters, you won't need to duplicate this code snippet in you application, and you will prevent the risk of forgetting something or writing typos.

Note1

to help you in this kind of situations, Apache is there.

BeanUtils.copyProperties is what you are looking for, it will copy every field with the name matching between two objects.

Note2

your code could be higly refactored... for example this:

 if(gpDirect!=null){
    GatepassDTO tempDTO = new GatepassDTO();
    copyProperty(tempDTO,gatepassDTO);
    putCachedObject("SINGLEGATEPASSCACHE", gatepassNo, tempDTO);
    gatepassMap.put(gatepassNo, tempDTO);
    session.put("documentType", documentType);
    session.put("gatepassMap", gatepassMap);
    return SUCCESS;
}else{
    GatepassDTO tempDTO = new GatepassDTO();
    copyProperty(tempDTO,gatepassDTO);
    putCachedObject("SINGLEGATEPASSCACHE", gatepassNo, tempDTO);
    gatepassMap.put(gatepassNo, tempDTO);
    session.put("documentType", documentType);
    session.put("gatepassMap", gatepassMap);
}

could be written as

GatepassDTO tempDTO = new GatepassDTO();
copyProperty(tempDTO,gatepassDTO);
putCachedObject("SINGLEGATEPASSCACHE", gatepassNo, tempDTO);
gatepassMap.put(gatepassNo, tempDTO);
session.put("documentType", documentType);
session.put("gatepassMap", gatepassMap);

if(gpDirect!=null) return SUCCESS;

Take your time to clean it up, it will save you time and headaches in the future.

score 0 · Answer 2 · answered Jul 16 '13 at 09:43

When you're getting an object from a Map, you're not getting a copy of the object, you get a reference to the object. Which means that if you change a property of the object, the object "in" the hashmap will also have its property changed (because that's the same object).

This is what happens to your code:

You get a reference to the session object "getpassDTO"
You're getting references on the properties of that object (copyProperty copies references of objects, it does not clone your object)
You're modifying properties on these references. But your "getPassDTO" session object still references the same properties.

Why are multiple DTOs altering the session Map?

2 Answers2