Skip to content

Commit

Permalink
Minimise and simplify SQL requests during desktop activity: Simpler m…
Browse files Browse the repository at this point in the history
…odel for InstanceSession, InstanceSessionMember and InstanceActivity - reduce mapping to composed entities; Use "partial" objects (dtos) for select and update operations. Refactoring of InstanceSessionMemberService/Repo.
  • Loading branch information
stuartcaunt committed Dec 3, 2024
1 parent 1d69f5a commit 7934fa1
Show file tree
Hide file tree
Showing 27 changed files with 457 additions and 283 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import eu.ill.visa.cloud.services.CloudClient;
import eu.ill.visa.core.entity.Instance;
import eu.ill.visa.core.entity.InstanceSession;
import eu.ill.visa.core.entity.InstanceSessionMember;
import eu.ill.visa.core.entity.partial.InstanceSessionMemberPartial;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;

Expand All @@ -19,6 +19,7 @@ public class InstanceActionServiceProvider {
private final InstanceService instanceService;

private final InstanceSessionService instanceSessionService;
private final InstanceSessionMemberService instanceSessionMemberService;
private final InstanceCommandService instanceCommandService;
private final SecurityGroupService securityGroupService;
private final InstrumentService instrumentService;
Expand All @@ -31,6 +32,7 @@ public class InstanceActionServiceProvider {
@Inject
public InstanceActionServiceProvider(final InstanceService instanceService,
final InstanceSessionService instanceSessionService,
final InstanceSessionMemberService instanceSessionMemberService,
final InstanceCommandService instanceCommandService,
final SecurityGroupService securityGroupService,
final InstrumentService instrumentService,
Expand All @@ -41,6 +43,7 @@ public InstanceActionServiceProvider(final InstanceService instanceService,
final EventDispatcher eventDispatcher) {
this.instanceService = instanceService;
this.instanceSessionService = instanceSessionService;
this.instanceSessionMemberService = instanceSessionMemberService;
this.instanceCommandService = instanceCommandService;
this.securityGroupService = securityGroupService;
this.instrumentService = instrumentService;
Expand All @@ -60,13 +63,13 @@ public void clearSessionsForInstance(Instance instance) {
final List<InstanceSession> sessions = this.instanceSessionService.getAllByInstance(instance);
for (InstanceSession session : sessions) {
session.setCurrent(false);
this.instanceSessionService.save(session);
this.instanceSessionService.updatePartial(session);
}

final List<InstanceSessionMember> sessionMembers = this.instanceSessionService.getAllSessionMembersByInstance(instance);
for (InstanceSessionMember member : sessionMembers) {
final List<InstanceSessionMemberPartial> sessionMembers = this.instanceSessionMemberService.getAllPartialsByInstanceId(instance.getId());
for (InstanceSessionMemberPartial member : sessionMembers) {
member.setActive(false);
this.instanceSessionService.saveInstanceSessionMember(member);
this.instanceSessionMemberService.updatePartial(member);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,17 @@
package eu.ill.visa.business.services;

import eu.ill.visa.business.InstanceConfiguration;
import eu.ill.visa.core.entity.Instance;
import eu.ill.visa.core.entity.InstanceActivity;
import eu.ill.visa.core.entity.User;
import eu.ill.visa.core.entity.enumerations.InstanceActivityType;
import eu.ill.visa.persistence.repositories.InstanceActivityRepository;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import jakarta.transaction.Transactional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.List;

@Transactional
@Singleton
public class InstanceActivityService {

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

private final InstanceActivityRepository repository;
private final InstanceConfiguration configuration;

Expand All @@ -30,18 +22,6 @@ public InstanceActivityService(final InstanceActivityRepository repository,
this.configuration = configuration;
}

public List<InstanceActivity> getAll() {
return this.repository.getAll();
}

public List<InstanceActivity> getAllForUser(User user) {
return this.repository.getAllForUser(user);
}

public List<InstanceActivity> getAllForInstance(Instance instance) {
return this.repository.getAllForInstance(instance);
}

public boolean cleanupActive() {
return this.configuration.activityRetentionPeriodDays() != 0;
}
Expand All @@ -52,8 +32,8 @@ public void cleanup() {
}
}

public InstanceActivity create(User user, Instance instance, InstanceActivityType action) {
InstanceActivity instanceActivity = new InstanceActivity(user, instance, action);
public InstanceActivity create(String userId, Long instanceId, InstanceActivityType action) {
InstanceActivity instanceActivity = new InstanceActivity(userId, instanceId, action);
this.repository.save(instanceActivity);

return instanceActivity;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import eu.ill.visa.core.entity.*;
import eu.ill.visa.core.entity.enumerations.InstanceMemberRole;
import eu.ill.visa.core.entity.enumerations.InstanceState;
import eu.ill.visa.core.entity.partial.InstancePartial;
import eu.ill.visa.persistence.repositories.InstanceRepository;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
Expand Down Expand Up @@ -410,4 +411,13 @@ public List<Instance> handleFetches(List<Instance> instances, List<InstanceFetch

return instances;
}

public InstancePartial getPartialById(Long id) {
return this.repository.getPartialById(id);
}

public void updatePartial(InstancePartial instance) {
this.repository.updatePartialById(instance);
}

}
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
package eu.ill.visa.business.services;

import eu.ill.visa.core.domain.Pagination;
import eu.ill.visa.core.entity.Instance;
import eu.ill.visa.core.entity.InstanceSession;
import eu.ill.visa.core.entity.InstanceSessionMember;
import eu.ill.visa.core.entity.User;
import eu.ill.visa.core.entity.enumerations.InstanceMemberRole;
import eu.ill.visa.core.entity.partial.InstanceSessionMemberPartial;
import eu.ill.visa.persistence.repositories.InstanceSessionMemberRepository;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import jakarta.transaction.Transactional;
import jakarta.validation.constraints.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -35,4 +41,64 @@ public Long countAll() {
public Long countAllActive() {
return repository.countAllActive();
}

public void create(@NotNull InstanceSession instanceSession, String memberId, User user, InstanceMemberRole role) {
// Ensure we do not have any previous InstanceSessionMembers that have not been deactivated correctly
InstanceSessionMemberPartial instanceSessionMemberPartial = this.getPartialByInstanceSessionIdAndSessionId(instanceSession.getId(), memberId);
if (instanceSessionMemberPartial != null) {
instanceSessionMemberPartial.setActive(false);
this.updatePartial(instanceSessionMemberPartial);
logger.warn("Deleting a InstanceSessionMember for instance {} with session Id {} that was not previously deleted", instanceSession.getInstanceId(), memberId);
}

InstanceSessionMember sessionMember = new InstanceSessionMember(instanceSession, memberId, user, role);
sessionMember.setActive(true);

this.repository.save(sessionMember);
}

public List<InstanceSessionMember> getAllByInstanceSessionId(@NotNull Long sessionId) {
return this.repository.getAllByInstanceSessionId(sessionId);
}

public List<InstanceSessionMember> getAllHistorySessionMembersByInstanceId(final Long instanceId) {
return this.repository.getAllHistorySessionMembersByInstanceId(instanceId);
}

public List<InstanceSessionMember> getAllByInstance(@NotNull Instance instance) {
return this.getAllByInstanceId(instance.getId());
}

public List<InstanceSessionMember> getAllByInstanceId(@NotNull Long instanceId) {
return this.repository.getAllByInstanceId(instanceId);
}

public List<List<InstanceSessionMember>> getAllByInstanceIds(@NotNull List<Long> instanceIds) {
List<InstanceSessionMember> ungroupedSessionMembers = this.repository.getAllByInstanceIds(instanceIds);
return instanceIds.stream().map(id -> {
return ungroupedSessionMembers.stream().filter(sessionMember -> sessionMember.getInstanceSession().getInstanceId().equals(id)).toList();
}).toList();
}

public List<InstanceSessionMemberPartial> getAllPartials() {
return this.repository.getAllPartials();
}

public List<InstanceSessionMemberPartial> getAllPartialsByInstanceSessionId(@NotNull Long sessionId) {
return this.repository.getAllPartialsByInstanceSessionId(sessionId);
}

public List<InstanceSessionMemberPartial> getAllPartialsByInstanceId(@NotNull Long instanceId) {
return this.repository.getAllPartialsByInstanceId(instanceId);
}

public InstanceSessionMemberPartial getPartialByInstanceSessionIdAndSessionId(Long instanceSessionId, final String sessionId) {
return this.repository.getPartialByInstanceSessionIdAndSessionId(instanceSessionId, sessionId);

}

public void updatePartial(final InstanceSessionMemberPartial instanceSessionMember) {
this.repository.updatePartial(instanceSessionMember);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import eu.ill.visa.core.entity.*;
import eu.ill.visa.core.entity.enumerations.InstanceMemberRole;
import eu.ill.visa.persistence.repositories.InstanceSessionMemberRepository;
import eu.ill.visa.core.entity.partial.InstanceSessionMemberPartial;
import eu.ill.visa.persistence.repositories.InstanceSessionRepository;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
Expand All @@ -22,19 +22,19 @@ public class InstanceSessionService {
private static final Logger logger = LoggerFactory.getLogger(InstanceSessionService.class);

private final InstanceSessionRepository repository;
private final InstanceSessionMemberRepository instanceSessionMemberRepository;
private final InstanceSessionMemberService instanceSessionMemberService;
private final InstanceJupyterSessionService instanceJupyterSessionService;
private final InstanceService instanceService;
private final UserService userService;

@Inject
public InstanceSessionService(final InstanceSessionRepository repository,
final InstanceSessionMemberRepository instanceSessionMemberRepository,
final InstanceSessionMemberService instanceSessionMemberService,
final InstanceJupyterSessionService instanceJupyterSessionService,
final InstanceService instanceService,
final UserService userService) {
this.repository = repository;
this.instanceSessionMemberRepository = instanceSessionMemberRepository;
this.instanceSessionMemberService = instanceSessionMemberService;
this.instanceJupyterSessionService = instanceJupyterSessionService;
this.instanceService = instanceService;
this.userService = userService;
Expand All @@ -48,8 +48,8 @@ public InstanceSession getById(@NotNull Long id) {
return this.repository.getById(id);
}

public InstanceSession create(@NotNull Instance instance, String protocol, String connectionId) {
InstanceSession session = new InstanceSession(instance, protocol, connectionId);
public InstanceSession create(@NotNull Long instanceId, String protocol, String connectionId) {
InstanceSession session = new InstanceSession(instanceId, protocol, connectionId);

this.save(session);

Expand All @@ -72,68 +72,31 @@ public InstanceSession getByInstanceIdAndProtocol(Long instanceId, String protoc
return this.repository.getByInstanceIdAndProtocol(instanceId, protocol);
}

public void save(@NotNull InstanceSession instanceSession) {
this.repository.save(instanceSession);
}

public void addInstanceSessionMember(@NotNull InstanceSession instanceSession, String memberId, User user, String role) {
InstanceSessionMember sessionMember = new InstanceSessionMember(instanceSession, memberId, user, role);
sessionMember.setActive(true);

this.instanceSessionMemberRepository.save(sessionMember);
}

public void removeInstanceSessionMember(@NotNull InstanceSession instanceSession, String memberId) {
InstanceSessionMember sessionMember = this.instanceSessionMemberRepository.getSessionMember(instanceSession, memberId);
public void deleteSessionMember(@NotNull InstanceSession instanceSession, String memberId) {
InstanceSessionMemberPartial sessionMember = this.instanceSessionMemberService.getPartialByInstanceSessionIdAndSessionId(instanceSession.getId(), memberId);
if (sessionMember != null) {
sessionMember.setActive(false);
this.instanceSessionMemberRepository.save(sessionMember);
this.instanceSessionMemberService.updatePartial(sessionMember);

List<InstanceSessionMember> members = this.getAllSessionMembersByInstanceSessionId(instanceSession.getId());
List<InstanceSessionMemberPartial> members = this.instanceSessionMemberService.getAllPartialsByInstanceSessionId(instanceSession.getId());

if (members.isEmpty()) {
logger.info("Session for Instance with Id: {} is no longer current as it has no connected members", instanceSession.getInstance().getId());
logger.info("Session for Instance with Id: {} is no longer current as it has no connected members", instanceSession.getInstanceId());
instanceSession.setCurrent(false);
}
this.repository.save(instanceSession);
this.updatePartial(instanceSession);

} else {
logger.warn("Got a null session member (memberId {}) for instance {}", memberId, instanceSession.getInstance().getId());
logger.warn("Got a null session member (memberId {}) for instance {}", memberId, instanceSession.getInstanceId());
}
}

public List<InstanceSessionMember> getAllSessionMembersByInstanceSession(@NotNull InstanceSession instanceSession) {
return this.getAllSessionMembersByInstanceSessionId(instanceSession.getId());
}

public List<InstanceSessionMember> getAllSessionMembersByInstanceSessionId(@NotNull Long sessionId) {
return this.instanceSessionMemberRepository.getAllSessionMembersByInstanceSessionId(sessionId);
}

public List<InstanceSessionMember> getAllHistorySessionMembersByInstanceId(final Long instanceId) {
return this.instanceSessionMemberRepository.getAllHistorySessionMembersByInstanceId(instanceId);
}

public List<InstanceSessionMember> getAllSessionMembersByInstance(@NotNull Instance instance) {
return this.getAllSessionMembersByInstanceId(instance.getId());
}

public List<InstanceSessionMember> getAllSessionMembersByInstanceId(@NotNull Long instanceId) {
return this.instanceSessionMemberRepository.getAllSessionMembersByInstanceId(instanceId);
}

public List<List<InstanceSessionMember>> getAllSessionMembersByInstanceIds(@NotNull List<Long> instanceIds) {
List<InstanceSessionMember> ungroupedSessionMembers = this.instanceSessionMemberRepository.getAllSessionMembersByInstanceIds(instanceIds);
return instanceIds.stream().map(id -> {
return ungroupedSessionMembers.stream().filter(sessionMember -> sessionMember.getInstanceSession().getInstance().getId().equals(id)).toList();
}).toList();
}

public InstanceSessionMember getSessionMemberBySessionId(String sessionId) {
return this.instanceSessionMemberRepository.getBySessionId(sessionId);
public void save(@NotNull InstanceSession instanceSession) {
this.repository.save(instanceSession);
}

public void saveInstanceSessionMember(InstanceSessionMember instanceSessionMember) {
this.instanceSessionMemberRepository.save(instanceSessionMember);
public void updatePartial(@NotNull InstanceSession instanceSession) {
this.repository.updatePartial(instanceSession);
}

public void cleanupSession() {
Expand All @@ -143,10 +106,10 @@ public void cleanupSession() {
this.repository.save(instanceSession);
}

List<InstanceSessionMember> activeSessionMembers = this.instanceSessionMemberRepository.getAll();
for (InstanceSessionMember instanceSessionMember : activeSessionMembers) {
List<InstanceSessionMemberPartial> activeSessionMembers = this.instanceSessionMemberService.getAllPartials();
for (InstanceSessionMemberPartial instanceSessionMember : activeSessionMembers) {
instanceSessionMember.setActive(false);
this.instanceSessionMemberRepository.save(instanceSessionMember);
this.instanceSessionMemberService.updatePartial(instanceSessionMember);
}

// Do the cleanup here... not ideal but at least it is centralised
Expand Down
Loading

0 comments on commit 7934fa1

Please sign in to comment.