From 1a3aac4802eed19230b284c8b4c62707e27c6c6f Mon Sep 17 00:00:00 2001 From: anhefti Date: Thu, 22 Aug 2019 15:38:19 +0200 Subject: [PATCH] SEBSERV-86 fixed and test fixes --- .../gbl/model/user/UserActivityLog.java | 2 +- .../sebserver/gui/content/SebClientLogs.java | 29 -------- .../gui/content/UserActivityLogs.java | 16 +++-- .../dao/impl/ClientEventDAOImpl.java | 69 ++++++++++--------- .../dao/impl/UserActivityLogDAOImpl.java | 62 +++++++++-------- .../api/UserActivityLogController.java | 11 ++- .../integration/api/admin/QuizDataTest.java | 30 +++++--- .../integration/api/admin/UserAPITest.java | 2 +- .../api/admin/UserActivityLogAPITest.java | 13 ++-- 9 files changed, 112 insertions(+), 122 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/model/user/UserActivityLog.java b/src/main/java/ch/ethz/seb/sebserver/gbl/model/user/UserActivityLog.java index 4493f995..96416edf 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/model/user/UserActivityLog.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/model/user/UserActivityLog.java @@ -18,7 +18,7 @@ import ch.ethz.seb.sebserver.gbl.model.Entity; public class UserActivityLog implements Entity { public static final String ATTR_USER_NAME = "username"; - public static final String FILTER_ATTR_USER = "user"; + public static final String FILTER_ATTR_USER_NAME = ATTR_USER_NAME; public static final String FILTER_ATTR_FROM = "from"; public static final String FILTER_ATTR_TO = "to"; public static final String FILTER_ATTR_FROM_TO = "from_to"; diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/SebClientLogs.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/SebClientLogs.java index e95f8fd6..781ecebb 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/SebClientLogs.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/SebClientLogs.java @@ -9,7 +9,6 @@ package ch.ethz.seb.sebserver.gui.content; import java.util.Map; -import java.util.function.BooleanSupplier; import java.util.function.Function; import org.eclipse.swt.widgets.Composite; @@ -27,7 +26,6 @@ import ch.ethz.seb.sebserver.gbl.model.exam.QuizData; import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection; import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent; import ch.ethz.seb.sebserver.gbl.model.session.ExtendedClientEvent; -import ch.ethz.seb.sebserver.gbl.model.user.UserRole; import ch.ethz.seb.sebserver.gbl.profile.GuiProfile; import ch.ethz.seb.sebserver.gbl.util.Utils; import ch.ethz.seb.sebserver.gui.content.action.ActionDefinition; @@ -45,7 +43,6 @@ import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.RestService; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.GetExam; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.logs.GetExtendedClientEventPage; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.session.GetClientConnection; -import ch.ethz.seb.sebserver.gui.service.remote.webservice.auth.CurrentUser; import ch.ethz.seb.sebserver.gui.table.ColumnDefinition; import ch.ethz.seb.sebserver.gui.table.ColumnDefinition.TableFilterAttribute; import ch.ethz.seb.sebserver.gui.table.EntityTable; @@ -67,8 +64,6 @@ public class SebClientLogs implements TemplateComposer { private static final LocTextKey EMPTY_TEXT_KEY = new LocTextKey("sebserver.seblogs.list.empty"); - private static final LocTextKey INSTITUTION_TEXT_KEY = - new LocTextKey("sebserver.seblogs.list.column.institution"); private static final LocTextKey EXAM_TEXT_KEY = new LocTextKey("sebserver.seblogs.list.column.exam"); private static final LocTextKey CLIENT_SESSION_TEXT_KEY = @@ -160,7 +155,6 @@ public class SebClientLogs implements TemplateComposer { @Override public void compose(final PageContext pageContext) { - final CurrentUser currentUser = this.resourceService.getCurrentUser(); final WidgetFactory widgetFactory = this.pageService.getWidgetFactory(); final RestService restService = this.resourceService.getRestService(); // content page layout with title @@ -173,35 +167,12 @@ public class SebClientLogs implements TemplateComposer { .clearEntityKeys() .clearAttributes()); - final BooleanSupplier isSebAdmin = - () -> currentUser.get().hasRole(UserRole.SEB_SERVER_ADMIN); - - final Function institutionNameFunction = - this.resourceService.getInstitutionNameFunction() - .compose(log -> { - try { - final ClientConnection connection = restService.getBuilder(GetClientConnection.class) - .withURIVariable(API.PARAM_MODEL_ID, String.valueOf(log.getConnectionId())) - .call().getOrThrow(); - return String.valueOf(connection.getInstitutionId()); - } catch (final Exception e) { - return Constants.EMPTY_NOTE; - } - }); // table final EntityTable table = this.pageService.entityTableBuilder( restService.getRestCall(GetExtendedClientEventPage.class)) .withEmptyMessage(EMPTY_TEXT_KEY) .withPaging(this.pageSize) - .withColumnIf( - isSebAdmin, - () -> new ColumnDefinition<>( - Domain.INSTITUTION.ATTR_NAME, - INSTITUTION_TEXT_KEY, - institutionNameFunction) - .widthProportion(2)) - .withColumn(new ColumnDefinition<>( Domain.CLIENT_CONNECTION.ATTR_EXAM_ID, EXAM_TEXT_KEY, diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/UserActivityLogs.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/UserActivityLogs.java index a225b2f5..a24466b3 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/UserActivityLogs.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/UserActivityLogs.java @@ -19,6 +19,7 @@ import org.springframework.stereotype.Component; import ch.ethz.seb.sebserver.gbl.Constants; import ch.ethz.seb.sebserver.gbl.api.API; import ch.ethz.seb.sebserver.gbl.model.Domain; +import ch.ethz.seb.sebserver.gbl.model.Entity; import ch.ethz.seb.sebserver.gbl.model.user.UserActivityLog; import ch.ethz.seb.sebserver.gbl.model.user.UserInfo; import ch.ethz.seb.sebserver.gbl.model.user.UserRole; @@ -73,7 +74,9 @@ public class UserActivityLogs implements TemplateComposer { private final static LocTextKey EMPTY_SELECTION_TEXT = new LocTextKey("sebserver.userlogs.info.pleaseSelect"); - private final TableFilterAttribute userFilter; + private final TableFilterAttribute institutionFilter; + private final TableFilterAttribute userNameFilter = + new TableFilterAttribute(CriteriaType.TEXT, UserActivityLog.FILTER_ATTR_USER_NAME); private final TableFilterAttribute activityFilter; private final TableFilterAttribute entityFilter; @@ -93,10 +96,10 @@ public class UserActivityLogs implements TemplateComposer { this.widgetFactory = pageService.getWidgetFactory(); this.pageSize = pageSize; - this.userFilter = new TableFilterAttribute( + this.institutionFilter = new TableFilterAttribute( CriteriaType.SINGLE_SELECTION, - UserActivityLog.FILTER_ATTR_USER, - this.resourceService::userResources); + Entity.FILTER_ATTR_INSTITUTION, + this.resourceService::institutionResource); this.activityFilter = new TableFilterAttribute( CriteriaType.SINGLE_SELECTION, @@ -151,13 +154,14 @@ public class UserActivityLogs implements TemplateComposer { () -> new ColumnDefinition<>( UserActivityLog.FILTER_ATTR_INSTITUTION, INSTITUTION_TEXT_KEY, - institutionNameFunction)) + institutionNameFunction) + .withFilter(this.institutionFilter)) .withColumn(new ColumnDefinition<>( UserActivityLog.ATTR_USER_NAME, USER_TEXT_KEY, UserActivityLog::getUsername) - .withFilter(this.userFilter)) + .withFilter(this.userNameFilter)) .withColumn(new ColumnDefinition( Domain.USER_ACTIVITY_LOG.ATTR_ACTIVITY_TYPE, diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientEventDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientEventDAOImpl.java index 80280705..7d6e1655 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientEventDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientEventDAOImpl.java @@ -77,39 +77,42 @@ public class ClientEventDAOImpl implements ClientEventDAO { final FilterMap filterMap, final Predicate predicate) { - return Result.tryCatch(() -> this.clientEventRecordMapper - .selectByExample() - .where( - ClientEventRecordDynamicSqlSupport.connectionId, - isEqualToWhenPresent(filterMap.getClientEventConnectionId())) - .and( - ClientEventRecordDynamicSqlSupport.type, - isEqualToWhenPresent(filterMap.getClientEventTypeId())) - .and( - ClientEventRecordDynamicSqlSupport.type, - SqlBuilder.isNotEqualTo(EventType.LAST_PING.id)) - .and( - ClientEventRecordDynamicSqlSupport.clientTime, - SqlBuilder.isGreaterThanOrEqualToWhenPresent(filterMap.getClientEventClientTimeFrom())) - .and( - ClientEventRecordDynamicSqlSupport.clientTime, - SqlBuilder.isLessThanOrEqualToWhenPresent(filterMap.getClientEventClientTimeTo())) - .and( - ClientEventRecordDynamicSqlSupport.serverTime, - SqlBuilder.isGreaterThanOrEqualToWhenPresent(filterMap.getClientEventServerTimeFrom())) - .and( - ClientEventRecordDynamicSqlSupport.serverTime, - SqlBuilder.isLessThanOrEqualToWhenPresent(filterMap.getClientEventServerTimeTo())) - .and( - ClientEventRecordDynamicSqlSupport.text, - SqlBuilder.isLikeWhenPresent(filterMap.getClientEventText())) - .build() - .execute() - .stream() - .map(ClientEventDAOImpl::toDomainModel) - .flatMap(DAOLoggingSupport::logAndSkipOnError) - .filter(predicate) - .collect(Collectors.toList())); + return Result.tryCatch(() -> { + + return this.clientEventRecordMapper + .selectByExample() + .where( + ClientEventRecordDynamicSqlSupport.connectionId, + isEqualToWhenPresent(filterMap.getClientEventConnectionId())) + .and( + ClientEventRecordDynamicSqlSupport.type, + isEqualToWhenPresent(filterMap.getClientEventTypeId())) + .and( + ClientEventRecordDynamicSqlSupport.type, + SqlBuilder.isNotEqualTo(EventType.LAST_PING.id)) + .and( + ClientEventRecordDynamicSqlSupport.clientTime, + SqlBuilder.isGreaterThanOrEqualToWhenPresent(filterMap.getClientEventClientTimeFrom())) + .and( + ClientEventRecordDynamicSqlSupport.clientTime, + SqlBuilder.isLessThanOrEqualToWhenPresent(filterMap.getClientEventClientTimeTo())) + .and( + ClientEventRecordDynamicSqlSupport.serverTime, + SqlBuilder.isGreaterThanOrEqualToWhenPresent(filterMap.getClientEventServerTimeFrom())) + .and( + ClientEventRecordDynamicSqlSupport.serverTime, + SqlBuilder.isLessThanOrEqualToWhenPresent(filterMap.getClientEventServerTimeTo())) + .and( + ClientEventRecordDynamicSqlSupport.text, + SqlBuilder.isLikeWhenPresent(filterMap.getClientEventText())) + .build() + .execute() + .stream() + .map(ClientEventDAOImpl::toDomainModel) + .flatMap(DAOLoggingSupport::logAndSkipOnError) + .filter(predicate) + .collect(Collectors.toList()); + }); } @Override diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/UserActivityLogDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/UserActivityLogDAOImpl.java index 6ee8989f..7b75dfa7 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/UserActivityLogDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/UserActivityLogDAOImpl.java @@ -278,7 +278,7 @@ public class UserActivityLogDAOImpl implements UserActivityLogDAO { return all( filterMap.getInstitutionId(), - filterMap.getString(UserActivityLog.FILTER_ATTR_USER), + filterMap.getString(UserActivityLog.FILTER_ATTR_USER_NAME), filterMap.getUserLogFrom(), filterMap.getUserLofTo(), filterMap.getString(UserActivityLog.FILTER_ATTR_ACTIVITY_TYPES), @@ -290,7 +290,7 @@ public class UserActivityLogDAOImpl implements UserActivityLogDAO { @Transactional(readOnly = true) public Result> all( final Long institutionId, - final String userId, + final String userName, final Long from, final Long to, final String activityTypes, @@ -305,37 +305,39 @@ public class UserActivityLogDAOImpl implements UserActivityLogDAO { ? Arrays.asList(StringUtils.split(entityTypes, Constants.LIST_SEPARATOR)) : null; - final Predicate _predicate = (predicate != null) + Predicate _predicate = (predicate != null) ? predicate : model -> true; - return this.toDomainModel( - institutionId, - this.userLogRecordMapper.selectByExample() - .join(UserRecordDynamicSqlSupport.userRecord) - .on( - UserRecordDynamicSqlSupport.uuid, - SqlBuilder.equalTo(UserActivityLogRecordDynamicSqlSupport.userUuid)) - .where( - UserRecordDynamicSqlSupport.institutionId, - SqlBuilder.isEqualToWhenPresent(institutionId)) - .and( - UserActivityLogRecordDynamicSqlSupport.userUuid, - SqlBuilder.isEqualToWhenPresent(userId)) - .and( - UserActivityLogRecordDynamicSqlSupport.timestamp, - SqlBuilder.isGreaterThanOrEqualToWhenPresent(from)) - .and( - UserActivityLogRecordDynamicSqlSupport.timestamp, - SqlBuilder.isLessThanWhenPresent(to)) - .and( - UserActivityLogRecordDynamicSqlSupport.activityType, - SqlBuilder.isInCaseInsensitiveWhenPresent(_activityTypes)) - .and( - UserActivityLogRecordDynamicSqlSupport.entityType, - SqlBuilder.isInCaseInsensitiveWhenPresent(_entityTypes)) - .build() - .execute()) + if (StringUtils.isNotBlank(userName)) { + _predicate = _predicate.and(model -> model.getUsername().contains(userName)); + } + + final List records = this.userLogRecordMapper + .selectByExample() + .leftJoin(UserRecordDynamicSqlSupport.userRecord) + .on( + UserRecordDynamicSqlSupport.uuid, + SqlBuilder.equalTo(UserActivityLogRecordDynamicSqlSupport.userUuid)) + .where( + UserRecordDynamicSqlSupport.institutionId, + SqlBuilder.isEqualToWhenPresent(institutionId)) + .and( + UserActivityLogRecordDynamicSqlSupport.timestamp, + SqlBuilder.isGreaterThanOrEqualToWhenPresent(from)) + .and( + UserActivityLogRecordDynamicSqlSupport.timestamp, + SqlBuilder.isLessThanWhenPresent(to)) + .and( + UserActivityLogRecordDynamicSqlSupport.activityType, + SqlBuilder.isInCaseInsensitiveWhenPresent(_activityTypes)) + .and( + UserActivityLogRecordDynamicSqlSupport.entityType, + SqlBuilder.isInCaseInsensitiveWhenPresent(_entityTypes)) + .build() + .execute(); + + return this.toDomainModel(institutionId, records) .stream() .filter(_predicate) .collect(Collectors.toList()); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/UserActivityLogController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/UserActivityLogController.java index 106288a5..4ffc8db5 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/UserActivityLogController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/UserActivityLogController.java @@ -14,8 +14,8 @@ import org.springframework.web.bind.annotation.RestController; import ch.ethz.seb.sebserver.gbl.api.API; import ch.ethz.seb.sebserver.gbl.api.EntityType; +import ch.ethz.seb.sebserver.gbl.api.authorization.PrivilegeType; import ch.ethz.seb.sebserver.gbl.model.user.UserActivityLog; -import ch.ethz.seb.sebserver.gbl.model.user.UserRole; import ch.ethz.seb.sebserver.gbl.profile.WebServiceProfile; import ch.ethz.seb.sebserver.gbl.util.Result; import ch.ethz.seb.sebserver.webservice.datalayer.batis.mapper.UserActivityLogRecordDynamicSqlSupport; @@ -68,12 +68,11 @@ public class UserActivityLogController extends ReadonlyEntityController quizzes = new RestAPITestHelper() .withAccessToken(getSebAdminAccess()) .withPath(API.QUIZ_DISCOVERY_ENDPOINT) @@ -81,16 +81,7 @@ public class QuizDataTest extends AdministrationAPIIntegrationTester { .withExpectedStatus(HttpStatus.OK) .getAsObject(new TypeReference() { }); - new RestAPITestHelper() - .withAccessToken(getAdminInstitution2Access()) - .withPath(API.LMS_SETUP_ENDPOINT) - .withPath(String.valueOf(lmsSetup2.id)).withPath("/active") - .withMethod(HttpMethod.POST) - .withExpectedStatus(HttpStatus.OK) - .getAsObject(new TypeReference() { - }); - // now we should not get any quizzes for the seb-admin quizzes = new RestAPITestHelper() .withAccessToken(getSebAdminAccess()) .withPath(API.QUIZ_DISCOVERY_ENDPOINT) @@ -101,6 +92,25 @@ public class QuizDataTest extends AdministrationAPIIntegrationTester { assertNotNull(quizzes); assertTrue(quizzes.content.size() == 0); + new RestAPITestHelper() + .withAccessToken(getAdminInstitution2Access()) + .withPath(API.LMS_SETUP_ENDPOINT) + .withPath(String.valueOf(lmsSetup2.id)).withPath("/active") + .withMethod(HttpMethod.POST) + .withExpectedStatus(HttpStatus.OK) + .getAsObject(new TypeReference() { + }); + + quizzes = new RestAPITestHelper() + .withAccessToken(getSebAdminAccess()) + .withPath(API.QUIZ_DISCOVERY_ENDPOINT) + .withExpectedStatus(HttpStatus.OK) + .getAsObject(new TypeReference>() { + }); + + assertNotNull(quizzes); + assertTrue(quizzes.content.size() == 7); + // but for the now active lmsSetup2 we should get the quizzes quizzes = new RestAPITestHelper() .withAccessToken(getAdminInstitution2Access()) diff --git a/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/UserAPITest.java b/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/UserAPITest.java index a4b86a9f..22a8db7e 100644 --- a/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/UserAPITest.java +++ b/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/UserAPITest.java @@ -483,7 +483,7 @@ public class UserAPITest extends AdministrationAPIIntegrationTester { this.mockMvc .perform( get(this.endpoint + API.USER_ACTIVITY_LOG_ENDPOINT - + "?user=user1&activity_types=CREATE") + + "?username=admin&activity_types=CREATE") .header("Authorization", "Bearer " + token) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)) diff --git a/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/UserActivityLogAPITest.java b/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/UserActivityLogAPITest.java index 0e8179a6..ebda56b6 100644 --- a/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/UserActivityLogAPITest.java +++ b/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/UserActivityLogAPITest.java @@ -50,9 +50,10 @@ public class UserActivityLogAPITest extends AdministrationAPIIntegrationTester { // for a user in another institution, the institution has to be defined Page logs = this.jsonMapper.readValue( this.mockMvc - .perform(get(this.endpoint + API.USER_ACTIVITY_LOG_ENDPOINT + "?user=user4&institutionId=2") - .header("Authorization", "Bearer " + token) - .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)) + .perform(get( + this.endpoint + API.USER_ACTIVITY_LOG_ENDPOINT + "?username=examAdmin1&institutionId=2") + .header("Authorization", "Bearer " + token) + .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)) .andExpect(status().isOk()) .andReturn().getResponse().getContentAsString(), new TypeReference>() { @@ -63,7 +64,7 @@ public class UserActivityLogAPITest extends AdministrationAPIIntegrationTester { // for a user in the same institution no institution is needed logs = this.jsonMapper.readValue( - this.mockMvc.perform(get(this.endpoint + API.USER_ACTIVITY_LOG_ENDPOINT + "?user=user3") + this.mockMvc.perform(get(this.endpoint + API.USER_ACTIVITY_LOG_ENDPOINT + "?username=inst2Admin") .header("Authorization", "Bearer " + token) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)) .andExpect(status().isOk()) @@ -291,10 +292,10 @@ public class UserActivityLogAPITest extends AdministrationAPIIntegrationTester { .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)) .andExpect(status().isForbidden()); - // no privilege to query logs of users of other institution + // no privilege to query logs of users of other institution for institutional admin token = getAdminInstitution1Access(); final Page logs = this.jsonMapper.readValue( - this.mockMvc.perform(get(this.endpoint + API.USER_ACTIVITY_LOG_ENDPOINT + "?user=user4") + this.mockMvc.perform(get(this.endpoint + API.USER_ACTIVITY_LOG_ENDPOINT + "?username=examAdmin1") .header("Authorization", "Bearer " + token) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)) .andExpect(status().isOk())