Improve general performance

- Adjust logging
- Optimize cache speed
- Cleanup classes

Co-authored-by: René Pfeuffer <rene.pfeuffer@cloudogu.com>

Reviewed-by: Eduard Heimbuch <eduard.heimbuch@cloudogu.com>
This commit is contained in:
Eduard Heimbuch
2023-06-28 14:18:18 +02:00
parent cc54e2ce6b
commit 0e7a3ec53b
13 changed files with 140 additions and 449 deletions

View File

@@ -24,8 +24,6 @@
package sonia.scm.group;
//~--- non-JDK imports --------------------------------------------------------
import com.github.sdorra.ssp.PermissionActionCheck;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
@@ -40,7 +38,6 @@ import sonia.scm.SCMContextProvider;
import sonia.scm.auditlog.Auditor;
import sonia.scm.search.SearchRequest;
import sonia.scm.search.SearchUtil;
import sonia.scm.util.CollectionAppender;
import sonia.scm.util.Util;
import java.io.IOException;
@@ -55,47 +52,24 @@ import java.util.function.Predicate;
import static java.util.stream.Collectors.toSet;
//~--- JDK imports ------------------------------------------------------------
/**
*
* @author Sebastian Sdorra
*/
@Singleton
public class DefaultGroupManager extends AbstractGroupManager
{
public class DefaultGroupManager extends AbstractGroupManager {
/** the logger for XmlGroupManager */
private static final Logger logger =
LoggerFactory.getLogger(DefaultGroupManager.class);
private static final Logger LOG = LoggerFactory.getLogger(DefaultGroupManager.class);
private final GroupDAO groupDAO;
private final ManagerDaoAdapter<Group> managerDaoAdapter;
//~--- constructors ---------------------------------------------------------
/**
* Constructs ...
*
*
* @param groupDAO
*/
@Inject
public DefaultGroupManager(GroupDAO groupDAO, Set<Auditor> auditors)
{
public DefaultGroupManager(GroupDAO groupDAO, Set<Auditor> auditors) {
this.groupDAO = groupDAO;
this.managerDaoAdapter = new ManagerDaoAdapter<>(groupDAO, auditors);
}
//~--- methods --------------------------------------------------------------
/**
* Method description
*
*
* @throws IOException
*/
@Override
public void close() throws IOException
{
public void close() throws IOException {
// do nothing
}
@@ -106,7 +80,7 @@ public class DefaultGroupManager extends AbstractGroupManager
group.setType(groupDAO.getType());
}
logger.info("create group {} of type {}", group.getName(), group.getType());
LOG.info("create group {} of type {}", group.getName(), group.getType());
removeDuplicateMembers(group);
@@ -119,8 +93,8 @@ public class DefaultGroupManager extends AbstractGroupManager
}
@Override
public void delete(Group group){
logger.info("delete group {} of type {}", group.getName(), group.getType());
public void delete(Group group) {
LOG.info("delete group {} of type {}", group.getName(), group.getType());
managerDaoAdapter.delete(
group,
() -> GroupPermissions.delete(group.getName()),
@@ -129,18 +103,13 @@ public class DefaultGroupManager extends AbstractGroupManager
);
}
/**
* Method description
*
*
* @param context
*/
@Override
public void init(SCMContextProvider context) {}
public void init(SCMContextProvider context) {
}
@Override
public void modify(Group group){
logger.info("modify group {} of type {}", group.getName(), group.getType());
public void modify(Group group) {
LOG.info("modify group {} of type {}", group.getName(), group.getType());
managerDaoAdapter.modify(
group,
@@ -154,39 +123,23 @@ public class DefaultGroupManager extends AbstractGroupManager
}
@Override
public void refresh(Group group){
public void refresh(Group group) {
String name = group.getName();
if (logger.isInfoEnabled())
{
logger.info("refresh group {} of type {}", name, group.getType());
}
LOG.info("refresh group {} of type {}", name, group.getType());
GroupPermissions.read(name).check();
Group fresh = groupDAO.get(name);
if (fresh == null)
{
if (fresh == null) {
throw new NotFoundException(Group.class, group.getId());
}
fresh.copyProperties(group);
}
/**
* Method description
*
*
* @param searchRequest
*
* @return
*/
@Override
public Collection<Group> search(final SearchRequest searchRequest)
{
if (logger.isDebugEnabled())
{
logger.debug("search group with query {}", searchRequest.getQuery());
}
public Collection<Group> search(final SearchRequest searchRequest) {
LOG.debug("search group with query {}", searchRequest.getQuery());
final PermissionActionCheck<Group> check = GroupPermissions.read();
return SearchUtil.search(searchRequest, groupDAO.getAll(),
@@ -202,87 +155,45 @@ public class DefaultGroupManager extends AbstractGroupManager
return SearchUtil.matchesOne(searchRequest, group.getName(), group.getDescription());
}
//~--- get methods ----------------------------------------------------------
/**
* Method description
*
*
* @param id
*
* @return
*/
@Override
public Group get(String id)
{
public Group get(String id) {
GroupPermissions.read(id).check();
Group group = groupDAO.get(id);
if (group != null)
{
if (group != null) {
group = group.clone();
}
return group;
}
/**
* Method description
*
*
* @return
*/
@Override
public Collection<Group> getAll()
{
public Collection<Group> getAll() {
return getAll(group -> true, null);
}
/**
* Method description
*
*
* @param comparator
*
* @return
*/
@Override
public Collection<Group> getAll(Predicate<Group> filter, Comparator<Group> comparator)
{
public Collection<Group> getAll(Predicate<Group> filter, Comparator<Group> comparator) {
List<Group> groups = new ArrayList<>();
PermissionActionCheck<Group> check = GroupPermissions.read();
for (Group group : groupDAO.getAll())
{
for (Group group : groupDAO.getAll()) {
if (filter.test(group) && check.isPermitted(group)) {
groups.add(group.clone());
}
}
if (comparator != null)
{
if (comparator != null) {
Collections.sort(groups, comparator);
}
return groups;
}
/**
* Method description
*
*
*
* @param comparator
* @param start
* @param limit
*
* @return
*/
@Override
public Collection<Group> getAll(Comparator<Group> comparator, int start,
int limit)
{
int limit) {
final PermissionActionCheck<Group> check = GroupPermissions.read();
return Util.createSubCollection(groupDAO.getAll(), comparator,
@@ -293,38 +204,17 @@ public class DefaultGroupManager extends AbstractGroupManager
}, start, limit);
}
/**
* Method description
*
*
* @param start
* @param limit
*
* @return
*/
@Override
public Collection<Group> getAll(int start, int limit)
{
public Collection<Group> getAll(int start, int limit) {
return getAll(null, start, limit);
}
/**
* Method description
*
*
* @param member
*
* @return
*/
@Override
public Collection<Group> getGroupsForMember(String member)
{
public Collection<Group> getGroupsForMember(String member) {
LinkedList<Group> groups = new LinkedList<>();
for (Group group : groupDAO.getAll())
{
if (group.isMember(member))
{
for (Group group : groupDAO.getAll()) {
if (group.isMember(member)) {
groups.add(group.clone());
}
}
@@ -332,15 +222,8 @@ public class DefaultGroupManager extends AbstractGroupManager
return groups;
}
/**
* Method description
*
*
* @return
*/
@Override
public Long getLastModified()
{
public Long getLastModified() {
return groupDAO.getLastModified();
}
@@ -354,20 +237,12 @@ public class DefaultGroupManager extends AbstractGroupManager
* Remove duplicate members from group.
* Have a look at issue #439
*
*
* @param group group
*/
private void removeDuplicateMembers(Group group)
{
private void removeDuplicateMembers(Group group) {
List<String> members =
Lists.newArrayList(ImmutableSet.copyOf(group.getMembers()));
group.setMembers(members);
}
//~--- fields ---------------------------------------------------------------
/** Field description */
private GroupDAO groupDAO;
private final ManagerDaoAdapter<Group> managerDaoAdapter;
}

View File

@@ -77,7 +77,7 @@ import static sonia.scm.NotFoundException.notFound;
public class DefaultRepositoryManager extends AbstractRepositoryManager {
@SuppressWarnings("unchecked")
public static final Collector<String, Object, Collection<String>> LINKED_HASH_SET_COLLECTOR = new Collector<String, Object, Collection<String>>() {
public static final Collector<String, Object, Collection<String>> LINKED_HASH_SET_COLLECTOR = new Collector<>() {
@Override
public Supplier<Object> supplier() {
return LinkedHashSet::new;
@@ -103,7 +103,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
return emptySet();
}
};
private static final Logger logger = LoggerFactory.getLogger(DefaultRepositoryManager.class);
private static final Logger LOG = LoggerFactory.getLogger(DefaultRepositoryManager.class);
private final Map<String, RepositoryHandler> handlerMap;
private final KeyGenerator keyGenerator;
private final RepositoryDAO repositoryDAO;
@@ -159,7 +159,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
repository.setId(keyGenerator.createKey());
repository.setNamespace(namespaceStrategyProvider.get().createNamespace(repository));
logger.info("create repository {} of type {}", repository, repository.getType());
LOG.info("create repository {} of type {}", repository, repository.getType());
return managerDaoAdapter.create(
repository,
@@ -200,7 +200,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
@Override
public void delete(Repository repository) {
logger.info("delete repository {} of type {}", repository, repository.getType());
LOG.info("delete repository {} of type {}", repository, repository.getType());
managerDaoAdapter.delete(
repository,
() -> RepositoryPermissions.delete(repository),
@@ -227,7 +227,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
@Override
public void modify(Repository repository) {
logger.info("modify repository {} of type {}", repository, repository.getType());
LOG.info("modify repository {} of type {}", repository, repository.getType());
managerDaoAdapter.modify(
repository,
@@ -450,10 +450,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
type.getName().concat("already registered"));
}
if (logger.isInfoEnabled()) {
logger.info("added RepositoryHandler {} for type {}", handler.getClass(),
type);
}
LOG.info("added RepositoryHandler {} for type {}", handler.getClass(), type);
handlerMap.put(type.getName(), handler);
handler.init(contextProvider);

View File

@@ -21,10 +21,8 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
package sonia.scm.repository;
//~--- non-JDK imports --------------------------------------------------------
package sonia.scm.repository;
import com.github.legman.Subscribe;
import com.google.inject.Inject;
@@ -36,138 +34,70 @@ import sonia.scm.plugin.Extension;
import sonia.scm.web.security.AdministrationContext;
import sonia.scm.web.security.PrivilegedAction;
//~--- JDK imports ------------------------------------------------------------
/**
*
* @author Sebastian Sdorra
* @since 1.37
*/
@Extension
@EagerSingleton
public final class LastModifiedUpdateListener
{
public final class LastModifiedUpdateListener {
/**
* the logger for LastModifiedUpdateListener
*/
private static final Logger logger =
LoggerFactory.getLogger(LastModifiedUpdateListener.class);
private static final Logger LOG = LoggerFactory.getLogger(LastModifiedUpdateListener.class);
//~--- constructors ---------------------------------------------------------
private final AdministrationContext adminContext;
private final RepositoryManager repositoryManager;
/**
* Constructs ...
*
*
* @param adminContext
* @param repositoryManager
*/
@Inject
public LastModifiedUpdateListener(AdministrationContext adminContext,
RepositoryManager repositoryManager)
{
RepositoryManager repositoryManager) {
this.adminContext = adminContext;
this.repositoryManager = repositoryManager;
}
//~--- methods --------------------------------------------------------------
/**
* Method description
*
*
* @param event
*/
@Subscribe
public void onPostReceive(PostReceiveRepositoryHookEvent event)
{
public void onPostReceive(PostReceiveRepositoryHookEvent event) {
final Repository repository = event.getRepository();
if (repository != null)
{
if (repository != null) {
//J-
adminContext.runAsAdmin(
new LastModifiedPrivilegedAction(repositoryManager, repository)
);
//J+
}
else
{
logger.warn("recevied hook without repository");
} else {
LOG.warn("received hook without repository");
}
}
//~--- inner classes --------------------------------------------------------
static class LastModifiedPrivilegedAction implements PrivilegedAction {
/**
* Class description
*
*
* @version Enter version here..., 14/04/20
* @author Enter your name here...
*/
static class LastModifiedPrivilegedAction implements PrivilegedAction
{
private final Repository repository;
private final RepositoryManager repositoryManager;
/**
* Constructs ...
*
*
* @param repositoryManager
* @param repository
*/
public LastModifiedPrivilegedAction(RepositoryManager repositoryManager,
Repository repository)
{
Repository repository) {
this.repositoryManager = repositoryManager;
this.repository = repository;
}
//~--- methods ------------------------------------------------------------
/**
* Method description
*
*/
@Override
public void run()
{
public void run() {
Repository dbr = repositoryManager.get(repository.getId());
if (dbr != null)
{
logger.info("update last modified date of repository {}", dbr.getId());
if (dbr != null) {
LOG.debug("update last modified date of repository {}", dbr.getId());
dbr.setLastModified(System.currentTimeMillis());
try {
repositoryManager.modify(dbr);
} catch (NotFoundException e) {
logger.error("could not modify repository", e);
LOG.error("could not modify repository", e);
}
}
else
{
logger.error("could not find repository with id {}",
repository.getId());
} else {
LOG.error("could not find repository with id {}", repository.getId());
}
}
//~--- fields -------------------------------------------------------------
/** Field description */
private final Repository repository;
/** Field description */
private final RepositoryManager repositoryManager;
}
//~--- fields ---------------------------------------------------------------
/** Field description */
private final AdministrationContext adminContext;
/** Field description */
private final RepositoryManager repositoryManager;
}

View File

@@ -34,6 +34,7 @@ import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.authz.AuthorizationInfo;
import org.apache.shiro.authz.Permission;
import org.apache.shiro.authz.SimpleAuthorizationInfo;
import org.apache.shiro.subject.PrincipalCollection;
import org.apache.shiro.subject.Subject;
@@ -57,6 +58,7 @@ import java.util.Optional;
import java.util.Set;
import static java.util.Collections.emptySet;
import static java.util.Collections.unmodifiableCollection;
@Singleton
@Extension
@@ -133,7 +135,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector {
logger.trace("retrieve AuthorizationInfo for user {} from cache", user.getName());
}
return info;
return new UnmodifiableAuthorizationInfo(info);
}
private void collectGlobalPermissions(Builder<String> builder,
@@ -355,4 +357,35 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector {
return Objects.hashCode(username, groupnames);
}
}
private static class UnmodifiableAuthorizationInfo implements AuthorizationInfo {
private final AuthorizationInfo info;
public UnmodifiableAuthorizationInfo(AuthorizationInfo info) {
this.info = info;
}
@Override
public Collection<String> getRoles() {
return nullsafeUnmodifiable(info.getRoles());
}
@Override
public Collection<String> getStringPermissions() {
return nullsafeUnmodifiable(info.getStringPermissions());
}
@Override
public Collection<Permission> getObjectPermissions() {
return nullsafeUnmodifiable(info.getObjectPermissions());
}
private <T> Collection<T> nullsafeUnmodifiable(Collection<T> collection) {
if (collection == null) {
return null;
} else {
return unmodifiableCollection(collection);
}
}
}
}

View File

@@ -53,83 +53,43 @@ import java.util.Set;
import java.util.function.Predicate;
/**
*
* @author Sebastian Sdorra
*/
@Singleton @EagerSingleton
public class DefaultUserManager extends AbstractUserManager
{
@Singleton
@EagerSingleton
public class DefaultUserManager extends AbstractUserManager {
/** Field description */
public static final String STORE_NAME = "users";
private static final Logger LOG = LoggerFactory.getLogger(DefaultUserManager.class);
/** the logger for XmlUserManager */
private static final Logger logger =
LoggerFactory.getLogger(DefaultUserManager.class);
//~--- constructors ---------------------------------------------------------
private final UserDAO userDAO;
private final ManagerDaoAdapter<User> managerDaoAdapter;
private final PasswordService passwordService;
/**
* Constructs ...
*
* @param passwordService
* @param userDAO
*/
@Inject
public DefaultUserManager(PasswordService passwordService, UserDAO userDAO, Set<Auditor> auditors)
{
public DefaultUserManager(PasswordService passwordService, UserDAO userDAO, Set<Auditor> auditors) {
this.passwordService = passwordService;
this.userDAO = userDAO;
this.managerDaoAdapter = new ManagerDaoAdapter<>(userDAO, auditors);
}
//~--- methods --------------------------------------------------------------
/**
* Method description
*
*
* @throws IOException
*/
@Override
public void close() throws IOException
{
public void close() throws IOException {
// do nothing
}
/**
* Method description
*
*
* @param username
*
* @return
*/
@Override
public boolean contains(String username)
{
public boolean contains(String username) {
return userDAO.contains(username);
}
/**
* Method description
*
*
* @param user
*
* @throws IOException
*/
@Override
public User create(User user) {
String type = user.getType();
if (Util.isEmpty(type)) {
user.setType(userDAO.getType());
}
logger.info("create user {} of type {}", user.getName(), user.getType());
LOG.info("create user {}", user.getName());
ensurePasswordEncrypted(user);
return managerDaoAdapter.create(
@@ -142,7 +102,7 @@ public class DefaultUserManager extends AbstractUserManager
@Override
public void delete(User user) {
logger.info("delete user {} of type {}", user.getName(), user.getType());
LOG.info("delete user {} of type {}", user.getName());
managerDaoAdapter.delete(
user,
() -> UserPermissions.delete(user.getName()),
@@ -151,28 +111,13 @@ public class DefaultUserManager extends AbstractUserManager
);
}
/**
* Method description
*
*
* @param context
*/
@Override
public void init(SCMContextProvider context)
{
public void init(SCMContextProvider context) {
}
/**
* Method description
*
*
* @param user
*
* @throws IOException
*/
@Override
public void modify(User user) {
logger.info("modify user {} of type {}", user.getName(), user.getType());
LOG.info("modify user {}", user.getName());
ensurePasswordEncrypted(user);
managerDaoAdapter.modify(
user,
@@ -185,47 +130,23 @@ public class DefaultUserManager extends AbstractUserManager
user.setPassword(passwordService.encryptPassword(user.getPassword()));
}
/**
* Method description
*
*
* @param user
*
* @throws IOException
*/
@Override
public void refresh(User user) {
if (logger.isInfoEnabled())
{
logger.info("refresh user {} of type {}", user.getName(), user.getType());
}
LOG.info("refresh user {}", user.getName());
UserPermissions.read(user).check();
User fresh = userDAO.get(user.getName());
if (fresh == null)
{
if (fresh == null) {
throw new NotFoundException(User.class, user.getName());
}
fresh.copyProperties(user);
}
/**
* Method description
*
*
* @param searchRequest
*
* @return
*/
@Override
public Collection<User> search(final SearchRequest searchRequest)
{
if (logger.isDebugEnabled())
{
logger.debug("search user with query {}", searchRequest.getQuery());
}
public Collection<User> search(final SearchRequest searchRequest) {
LOG.debug("search user with query {}", searchRequest.getQuery());
final PermissionActionCheck<User> check = UserPermissions.read();
return SearchUtil.search(searchRequest, userDAO.getAll(), user -> {
@@ -240,54 +161,26 @@ public class DefaultUserManager extends AbstractUserManager
return SearchUtil.matchesOne(searchRequest, user.getName(), user.getDisplayName(), user.getMail());
}
//~--- get methods ----------------------------------------------------------
/**
* Method description
*
*
* @param id
*
* @return
*/
@Override
public User get(String id)
{
public User get(String id) {
UserPermissions.read().check(id);
User user = userDAO.get(id);
if (user != null)
{
if (user != null) {
user = user.clone();
}
return user;
}
/**
* Method description
*
*
* @return
*/
@Override
public Collection<User> getAll()
{
public Collection<User> getAll() {
return getAll(user -> true, null);
}
/**
* Method description
*
*
* @param comparator
*
* @return
*/
@Override
public Collection<User> getAll(Predicate<User> filter, Comparator<User> comparator)
{
public Collection<User> getAll(Predicate<User> filter, Comparator<User> comparator) {
List<User> users = new ArrayList<>();
PermissionActionCheck<User> check = UserPermissions.read();
@@ -304,17 +197,6 @@ public class DefaultUserManager extends AbstractUserManager
return users;
}
/**
* Method description
*
*
*
* @param comaparator
* @param start
* @param limit
*
* @return
*/
@Override
public Collection<User> getAll(Comparator<User> comaparator, int start, int limit) {
final PermissionActionCheck<User> check = UserPermissions.read();
@@ -326,42 +208,18 @@ public class DefaultUserManager extends AbstractUserManager
}, start, limit);
}
/**
* Method description
*
*
* @param start
* @param limit
*
* @return
*/
@Override
public Collection<User> getAll(int start, int limit)
{
public Collection<User> getAll(int start, int limit) {
return getAll(null, start, limit);
}
/**
* Method description
*
*
* @return
*/
@Override
public String getDefaultType()
{
public String getDefaultType() {
return userDAO.getType();
}
/**
* Method description
*
*
* @return
*/
@Override
public Long getLastModified()
{
public Long getLastModified() {
return userDAO.getLastModified();
}
@@ -401,8 +259,4 @@ public class DefaultUserManager extends AbstractUserManager
return Authentications.isSubjectAnonymous(user.getName());
}
//~--- fields ---------------------------------------------------------------
private final UserDAO userDAO;
private final ManagerDaoAdapter<User> managerDaoAdapter;
}

View File

@@ -49,7 +49,7 @@ public class AdministrationContextRealm extends AuthorizingRealm {
protected AuthorizationInfo doGetAuthorizationInfo(PrincipalCollection principals) {
AdministrationContextMarker marker = principals.oneByType(AdministrationContextMarker.class);
if (marker == AdministrationContextMarker.MARKER) {
LOG.info("assign admin permissions to admin context user {}", principals.getPrimaryPrincipal());
LOG.debug("assign admin permissions to admin context user {}", principals.getPrimaryPrincipal());
SimpleAuthorizationInfo authorizationInfo = new SimpleAuthorizationInfo(Sets.newHashSet(Role.USER));
authorizationInfo.setStringPermissions(Sets.newHashSet("*"));
return authorizationInfo;