I have already asked a question on stackoverflow on how to understand the Reporitory Pattern but I'm still having very big troubles in getting a good design implemented on my server. I'm desperately trying to refactor my server and move a lot of service-code into repositories (where it actually belongs).
Here is what we are talking about:
- Hibernate ORM
- GWT 2.7.0 Server / Client (Web Application)
What can happen for example is that the client sends me a ToolSetDTO
like this to store it in the database (the @OneToMany
etc. annotations are there just to show you the relation between those DTOs as they reflect the relation in the database):
public class ToolSetDTO {
Long id;
@OneToOne
public ToolDTO tool1;
@OneToOne
public ToolDTO tool2;
}
public class ToolDTO {
public Long id;
@OneToMany
public ToolDescriptionDTO toolDescription;
}
public class ToolDescriptionDTO {
public Long id;
public String name;
@OneToMany
public String LanguageDTO;
}
public class LanguageDTO {
public Long id;
public String name;
}
as it arrives on my server, the service routine takes that DTO and tries to save it and to return the stored DTO object with valid IDs etc.
@Override
public ToolSetDTO persistToolSet(ToolStoreDTO toolStoreDto, ToolSetDTO toolSetDto) {
ToolSetRepository toolSetRepository = new ToolSetRepository();
ToolSetDTO savedToolSetDto = toolSetRepository.save(toolStoreDto, toolSetDto);
return savedToolSetDto;
}
and we arrive already at the first problem. The incoming toolStoreDto
could contain a fake ID causing hibernate to store a ToolSet
into the list of another ToolStore
. So I was thinking about doing this:
private String getSessionId() {
HttpServletRequest httpRequest = getThreadLocalRequest();
return httpRequest.getSession().getId();
}
@Override
public ToolSetDTO persistToolSet(ToolStoreDTO toolStoreDto, ToolSetDTO toolSetDto) {
ToolSetRepository toolSetRepository = new ToolSetRepository(this.getSessionId());
ToolSetDTO savedToolSetDto = toolSetRepository.save(toolStoreDto, toolSetDto);
return savedToolSetDto;
}
Just pass the session ID to ToolSetRepository
that accesses the ToolStore
as follows via the ToolStoreRepository
that internally checks if the access is allowed or not. I am not sure if I designed that well..
public abstract class AbstractXsrfRepository<T> extends AbstractRepository<T> {
private String sessionId;
public AbstractXsrfRepository(String sessionId) {
this.sessionId = sessionId;
}
protected String getSessionId() {
return this.sessionId;
}
}
public class ToolStoreRepository extends AbstractXsrfRepository<ToolStoreDTO> {
@Override
public ToolStoreDTO get(Long id) {
this.openSession();
ToolStoreOwnerRepository toolStoreOwnerRepo = new ToolStoreOwnerRepository(this.getSessionId());
ToolStoreOwnerDTO toolStoreOwnerDto = toolStoreOwnerRepo.get(null);
Long toolStoreOwnerId = toolStoreOwnerDto.getId();
Criteria cr = this.getSession().createCriteria(ToolStore.class);
cr.add(Restrictions.eq("toolStoreOwner.id", toolStoreOwnerId));
cr.add(Restrictions.eq("id", id));
@SuppressWarnings("unchecked")
List<ToolStore> toolStores = cr.list();
if(toolStores.isEmpty() == true) {
String err = "toolStoreOwner.id seems not to be associated with toolStore.id";
throw new RuntimeException(err);
}
ToolStoreDTO toolStoreDto = ConvertEntity.converToolStore(toolStores.get(0));
this.closeSession();
return toolStoreDto;
}
}
Beforehand: The fact that get(Long id)
returns just a DTO and not the Entity object is a problem for me. I sometimes need access to ToolStore
from other repositories but they can't retrieve them from ToolStoreRepository
. They would have to re-implement private getToolStore(Long id)
that does the exact same as ToolStoreRepository.get(Long id)
.
Saving a ToolSet
could then look something like this:
public class ToolSetRepository<ToolSetDTO> {
public ToolSetDTO save(ToolStoreDTO toolStoreDto, ToolSetDTO toolSetDto) {
this.openSessionWithTransaction();
ToolStoreRepository toolStoreRepository = new ToolStoreRepository();
ToolStore toolStore = toolStoreRepository.get(toolStoreDto.getId());
// ..
Tool tool1 = null;
ToolDTO tool1Dto = toolSetDto.getTool1();
if(tool1Dto != null) {
ToolDTO toolDto = tool1Dto.getTool();
ToolDescriptionDTO toolDescriptionDto = toolDto.getToolDescription(languageDto.getId());
// PLEASE NOTE: That toolDescriptionRepository.save() returns an Entity and not a DTO!
ToolDescription toolDescription = toolDescriptionRepository.save(toolStoreDto, languageDto, toolDto, toolDescriptionDto);
ToolRepository toolRepository = new MiddayMenuAppetizerRepository();
tool1 = toolRepository.merge(toolStoreDto, toolDescription.getTool());
}
// ..
ToolSet toolSet = new ToolSet(toolStore, tool1, tool2);
ToolSetDTO savedToolSetDto = ConvertEntity.convertToolSet(toolSet);
this.closeSessionWithTransaction();
return savedToolSetDto;
}
}
As you can see in one comment // PLEASE NOTE ..
The ToolDescriptionRepository.save()
does return an Entity object! That is another problem I am confronted with my design at the moment. As I understood it, The repository should only take business objects and return business objects. Here I am violating this constraint!
And that is where I am now: Assuming that my abstract class AbstractXsrfRepository<T>
wasn't such a bad idea, I was thinking about another abstract class AbstractToolStoreRepository
. But why? because I could do this:
public class AbstractToolStoreRepository extends AbstractXsrfRepository<ToolSetDTO> {
public AbstractToolStoreRepository(String sessionId) {
super(sessionId);
}
protected ToolStore get(Long id) {
// Implementation of ToolSetRepository.get(Long id);
// that checks with "this.getSessionId()" if the access is
// valid or not.
}
}
and let every repository class that has somehow to get access to ToolStore
implement AbstractToolStoreRepository
. Please note that get(Long id)
is now protected
and returns ToolStore
instead of ToolStoreDTO
! That way such a repository could access ToolStore
without having to re-implement the get(Long id)
function from above and I don't have to make a function that actually returns an Entity object public
- instead it's just protected
and provided by AbstractToolStoreRepository
.
What could also be done is e.g. just throwing away the AbstractXsrfRepository<T>
abstract class and move those few lines right directly into AbstractToolStoreRepository
because one could argue that AbstractXsrfRepository<T>
in fact doesn't really do anything. The actual work is done by the the implementing class of AbstractXsrfRepository
which is here AbstractToolStoreRepository
.
I am not sure if anybody is going to read all that but if somebody got the time I'd like to know if this whole AbstractXsrfRepository<T>
thing was a good idea and if I should extend this inheritance chain to AbstractToolStoreRepository
or if I'll come into hells kitchen or something..
ToolDTO
. Also why are you saving a DTO? – gonzalon Jul 18 '15 at 8:49