Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 0 additions & 35 deletions Kitodo/src/main/java/org/kitodo/production/forms/BaseForm.java
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some imports become unused and should be removed after the proposed changes:

  • org.kitodo.data.database.beans.Client
  • org.kitodo.data.database.beans.Project
  • org.kitodo.data.database.beans.Role
  • org.kitodo.production.services.data.ClientService
  • org.kitodo.production.services.data.RoleService

Original file line number Diff line number Diff line change
Expand Up @@ -394,41 +394,6 @@ public String getFormattedDate(Date date) {
return "";
}

/**
* Create and return String containing the titles of all given roles joined by a ", ".
*
* @param roles list of roles
* @return String containing role titles
*/
public String getRoleTitles(List<Role> roles) {
return RoleService.getRoleTitles(roles);
}

/**
* Create and return String containing the names of all given clients joined by a ", ".
*
* @param clients list of roles
* @return String containing client names
*/
public String getClientNames(List<Client> clients) {
return ClientService.getClientNames(clients);
}

/**
* Create and return String containing the titles of all given projects joined by a ", ".
*
* @param projects list of roles
* @return String containing project titles
*/
public String getProjectTitles(List<Project> projects) {
try {
return ServiceManager.getProjectService().getProjectTitles(projects);
} catch (DAOException e) {
Helper.setErrorMessage(e);
return "";
}
}

/**
* Reset first row to 0 if given String 'keepPagination' is empty.
*
Expand Down
31 changes: 28 additions & 3 deletions Kitodo/src/main/java/org/kitodo/production/forms/UserForm.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
import org.kitodo.production.forms.dataeditor.GalleryViewMode;
import org.kitodo.production.helper.Helper;
import org.kitodo.production.helper.metadata.pagination.PaginatorType;
import org.kitodo.production.model.LazyBeanModel;
import org.kitodo.production.model.LazyUserModel;
import org.kitodo.production.security.DynamicAuthenticationProvider;
import org.kitodo.production.security.SecuritySession;
import org.kitodo.production.security.password.KitodoPassword;
Expand Down Expand Up @@ -129,7 +129,7 @@ public class UserForm extends BaseForm {
@Inject
public UserForm(LoginForm loginForm) {
super();
super.setLazyBeanModel(new LazyBeanModel(userService));
super.setLazyBeanModel(new LazyUserModel(userService));
this.loginForm = loginForm;
}

Expand Down Expand Up @@ -300,14 +300,15 @@ public void resetTasksToOpen(User user) {
* user from a connected LDAP service.
*/
public void checkAndDelete() {
if (getTasksInProgress(userObject).isEmpty()) {
if (!hasTasksInProgress(userObject)) {
Copy link
Member

@solth solth Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this if...else condition check can be inverted to prevent the negation with ! and make it easier to read? Something like

if (hasTasksInProgress(userObject)) {
    PrimeFaces.current().ajax().update("usersTabView:confirmResetTasksDialog");
    PrimeFaces.current().executeScript("PF('confirmResetTasksDialog').show();");
} else {
    deleteUser(userObject);
}

deleteUser(userObject);
} else {
PrimeFaces.current().ajax().update("usersTabView:confirmResetTasksDialog");
PrimeFaces.current().executeScript("PF('confirmResetTasksDialog').show();");
}
}


/**
* Unassign all tasks in work from user and set their status back to open and delete the user.
*/
Expand Down Expand Up @@ -826,6 +827,30 @@ public FilterMenu getFilterMenu() {
return filterMenu;
}

private LazyUserModel getLazyUserModel() {
return (LazyUserModel) lazyBeanModel;
}

public String getRoleTitles(User user) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add javadoc to public methods (applies to all new public methods below as well)

List<String> roles = getLazyUserModel().getRolesCache().get(user.getId());
return (Objects.isNull(roles) || roles.isEmpty()) ? "" : String.join(", ", roles);
}

public String getProjectTitles(User user) {
List<String> projects = getLazyUserModel().getProjectsCache().get(user.getId());
return (Objects.isNull(projects) || projects.isEmpty()) ? "" : String.join(", ", projects);
}

public String getClientNames(User user) {
List<String> clients = getLazyUserModel().getClientsCache().get(user.getId());
return (Objects.isNull(clients) || clients.isEmpty()) ? "" : String.join(", ", clients);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (Objects.isNull(clients) || clients.isEmpty()) ? "" : String.join(", ", clients);
return (Objects.isNull(clients) || clients.isEmpty()) ? "" : String.join(", ", clients);

}

public boolean hasTasksInProgress(User user) {
return Boolean.TRUE.equals(getLazyUserModel().getTasksCache().get(user.getId()));

}

private void deselectRoleClientColumn() {
selectedColumns = ServiceManager.getListColumnService().removeColumnByTitle(selectedColumns, "role.client");
}
Expand Down
111 changes: 111 additions & 0 deletions Kitodo/src/main/java/org/kitodo/production/model/LazyUserModel.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* (c) Kitodo. Key to digital objects e. V. <[email protected]>
*
* This file is part of the Kitodo project.
*
* It is licensed under GNU General Public License version 3 or later.
*
* For the full copyright and license information, please read the
* GPL3-License.txt file that was distributed with this source code.
*/

package org.kitodo.production.model;

import static java.lang.Math.toIntExact;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import org.apache.commons.lang3.StringUtils;
import org.hibernate.exception.SQLGrammarException;
import org.kitodo.data.database.beans.User;
import org.kitodo.data.database.exceptions.DAOException;
import org.kitodo.exceptions.FilterException;
import org.kitodo.production.services.data.FilterService;
import org.kitodo.production.services.data.UserService;
import org.kitodo.utils.Stopwatch;
import org.primefaces.PrimeFaces;
import org.primefaces.model.FilterMeta;
import org.primefaces.model.SortMeta;
import org.primefaces.model.SortOrder;

public class LazyUserModel extends LazyBeanModel {

private final UserService userService;

private Map<Integer, List<String>> rolesCache = new HashMap<>();
private Map<Integer, List<String>> clientsCache = new HashMap<>();
private Map<Integer, List<String>> projectsCache = new HashMap<>();
private Map<Integer, Boolean> tasksCache = new HashMap<>();

public LazyUserModel(UserService service) {
super(service);
this.userService = service;
}

@Override
@SuppressWarnings("unchecked")
public List<Object> load(int first, int pageSize,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add javadoc

Map<String, SortMeta> sortBy,
Map<String, FilterMeta> filters) {

String sortField = "";
SortOrder sortOrder = SortOrder.ASCENDING;
if (!sortBy.isEmpty()) {
SortMeta sortMeta = sortBy.values().iterator().next();
sortField = sortMeta.getField();
sortOrder = sortMeta.getOrder();
}
Stopwatch stopwatch = new Stopwatch(this, "load", "first", Integer.toString(first), "pageSize", Integer
.toString(pageSize), "sortField", sortField, "sortOrder", Objects.toString(sortOrder), "filters",
Objects.toString(filters));
try {
HashMap<String, String> filterMap = new HashMap<>();
if (!StringUtils.isBlank(this.filterString)) {
filterMap.put(FilterService.FILTER_STRING, this.filterString);
}
setRowCount(toIntExact(searchService.countResults(filterMap)));
entities = searchService.loadData(first, pageSize, sortField, sortOrder, filterMap);
List<Integer> ids = new ArrayList<>(entities.size());
for (Object o : entities) {
ids.add(((User) o).getId());
}
rolesCache = userService.loadRolesForUsers(ids);
clientsCache = userService.loadClientsForUsers(ids);
projectsCache = userService.loadProjectsForUsers(ids);
tasksCache = userService.loadTasksInProgressForUsers(ids);
logger.info("{} entities loaded!", entities.size());
return stopwatch.stop(entities);
} catch (DAOException | SQLGrammarException e) {
setRowCount(0);
logger.error(e.getMessage(), e);
} catch (FilterException e) {
setRowCount(0);
PrimeFaces.current().executeScript("PF('sticky-notifications').renderMessage("
+ "{'summary':'Filter error','detail':'" + e.getMessage() + "','severity':'error'});");
logger.error(e.getMessage(), e);
}
return stopwatch.stop(new LinkedList<>());
}

public Map<Integer, List<String>> getRolesCache() {
return rolesCache;
}

public Map<Integer, List<String>> getClientsCache() {
return clientsCache;
}

public Map<Integer, List<String>> getProjectsCache() {
return projectsCache;
}

public Map<Integer, Boolean> getTasksCache() {
return tasksCache;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ public UserDetails loadUserByUsername(String username) {
@Override
public List<User> loadData(int first, int pageSize, String sortField, SortOrder sortOrder, Map filters) {
HashMap<String, Object> filterMap;

try {
filterMap = ServiceManager.getFilterService().getSQLFilterMap(filters, User.class);
} catch (NoSuchFieldException | NumberFormatException e) {
Expand Down Expand Up @@ -564,4 +565,118 @@ public static List<Client> getClientsOfUserSorted(User user) {
.sorted(Comparator.comparing(Client::getName, String.CASE_INSENSITIVE_ORDER))
.collect(Collectors.toList());
}

/**
* Loads all role titles for the given user IDs in a single query.
*
* @param userIds list of user IDs
* @return a map of userId and list of role titles
*/
public Map<Integer, List<String>> loadRolesForUsers(List<Integer> userIds) {
if (userIds.isEmpty()) {
return Collections.emptyMap();
}
String hql = "SELECT u.id, r.title "
+ "FROM User u "
+ "JOIN u.roles r "
+ "WHERE u.id IN (:ids)";
Map<String, Object> params = new HashMap<>();
params.put("ids", userIds);
List<Object[]> rows = dao.getProjectionByQuery(hql, params);
Map<Integer, List<String>> result = new HashMap<>();
for (Object[] row : rows) {
Integer userId = (Integer) row[0];
String title = (String) row[1];
result.computeIfAbsent(userId, k -> new ArrayList<>()).add(title);
}
return result;
Comment on lines +583 to +592
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part to gather the query parameters and retrieve the result is duplicated in the individual load..ForUsers methods and could/should be moved to a separate private method to avoid code duplication.

}

/**
* Loads all client names for the given user IDs in a single query.
*
* @param userIds list of user IDs
* @return a map of userId and list of client names
*/
public Map<Integer, List<String>> loadClientsForUsers(List<Integer> userIds) {
if (userIds.isEmpty()) {
return Collections.emptyMap();
}
String hql = "SELECT u.id, c.name "
+ "FROM User u "
+ "JOIN u.clients c "
+ "WHERE u.id IN (:ids)";
Map<String, Object> params = new HashMap<>();
params.put("ids", userIds);
List<Object[]> rows = dao.getProjectionByQuery(hql, params);
Map<Integer, List<String>> result = new HashMap<>();
for (Object[] row : rows) {
Integer userId = (Integer) row[0];
String clientName = (String) row[1];
result.computeIfAbsent(userId, k -> new ArrayList<>()).add(clientName);
}
return result;
}

/**
* Loads all project titles for the given user IDs in a single query.
*
* @param userIds list of user IDs
* @return a map of userId and list of project titles
*/
public Map<Integer, List<String>> loadProjectsForUsers(List<Integer> userIds) {
if (userIds.isEmpty()) {
return Collections.emptyMap();
}
String hql = "SELECT u.id, p.title "
+ "FROM User u "
+ "JOIN u.projects p "
+ "WHERE u.id IN (:ids)";
Map<String, Object> params = new HashMap<>();
params.put("ids", userIds);
List<Object[]> rows = dao.getProjectionByQuery(hql, params);
Map<Integer, List<String>> result = new HashMap<>();
for (Object[] row : rows) {
Integer userId = (Integer) row[0];
String projectTitle = (String) row[1];
result.computeIfAbsent(userId, k -> new ArrayList<>()).add(projectTitle);
}
return result;
}

/**
* Loads a boolean flag for each user ID indicating whether the user
* has at least one task currently in progress.
*
* @param userIds list of user IDs
* @return map of userId → true/false
*/
public Map<Integer, Boolean> loadTasksInProgressForUsers(List<Integer> userIds) {
if (userIds.isEmpty()) {
return Collections.emptyMap();
}

// We only need the user IDs; returning a constant boolean simplifies post-processing.
String hql = "SELECT DISTINCT u.id, true "
+ "FROM Task t "
+ "JOIN t.processingUser u "
+ "WHERE u.id IN (:ids) "
+ "AND t.processingStatus = :status "
+ "AND t.process IS NOT NULL";

Map<String, Object> params = new HashMap<>();
params.put("ids", userIds);
params.put("status", TaskStatus.INWORK);

List<Object[]> rows = dao.getProjectionByQuery(hql, params);

Map<Integer, Boolean> result = new HashMap<>();
for (Integer userId : userIds) {
result.put(userId, false);
}
for (Object[] row : rows) {
result.put((Integer) row[0], true);
}
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,18 +91,18 @@
</p:column>
<p:column headerText="#{msgs.roles}"
rendered="#{UserForm.showColumn('user.roles')}">
<h:outputText value="#{UserForm.getRoleTitles(item.roles)}"
title="#{UserForm.getRoleTitles(item.roles)}"/>
<h:outputText value="#{UserForm.getRoleTitles(item)}"
title="#{UserForm.getRoleTitles(item)}"/>
</p:column>
<p:column headerText="#{msgs.clients}"
rendered="#{UserForm.showColumn('user.clients')}">
<h:outputText value="#{UserForm.getClientNames(item.clients)}"
title="#{UserForm.getClientNames(item.clients)}"/>
<h:outputText value="#{UserForm.getClientNames(item)}"
title="#{UserForm.getClientNames(item)}"/>
</p:column>
<p:column headerText="#{msgs.projects}"
rendered="#{UserForm.showColumn('user.projects')}">
<h:outputText value="#{UserForm.getProjectTitles(item.projects)}"
title="#{UserForm.getProjectTitles(item.projects)}"/>
<h:outputText value="#{UserForm.getProjectTitles(item)}"
title="#{UserForm.getProjectTitles(item)}"/>
</p:column>
<p:column headerText="#{msgs.loggedIn}"
rendered="#{UserForm.showColumn('user.loggedIn')}"
Expand Down Expand Up @@ -138,14 +138,14 @@

<h:panelGroup styleClass="action"
rendered="#{SecurityAccessController.hasAuthorityToUnassignTasks()}"
title="#{empty UserForm.getTasksInProgress(item) ? msgs.userWithoutTasks : msgs.unassignTasks}">
title="#{UserForm.hasTasksInProgress(item) ? msgs.unassignTasks : msgs.userWithoutTasks}">
<p:commandLink id="unassignTasks"
action="#{UserForm.resetTasksToOpen(item)}"
styleClass="#{empty UserForm.getTasksInProgress(item) ? 'ui-state-disabled' : ''}"
disabled="#{empty UserForm.getTasksInProgress(item)}"
styleClass="#{UserForm.hasTasksInProgress(item) ? '' : 'ui-state-disabled'}"
disabled="#{not UserForm.hasTasksInProgress(item)}"
update="@this">
<h:outputText><i class="fa fa-user-times"/></h:outputText>
<p:confirm message=" #{msgs.confirmUnassignTasks}" header="#{msgs.confirmRelease}"/>
<p:confirm message="#{msgs.confirmUnassignTasks}" header="#{msgs.confirmRelease}"/>
</p:commandLink>
</h:panelGroup>

Expand Down
Loading