SystemScopeService: Optimize SQL access

The scopesMatch() method is called for each ApprovedSite in the approval
process and for each ApprovedSite entry all SystemScopes are read by a
single SQL statement.

Optimise this a bit and switch to a List so only one SQL is issued per
ApprovedSite.
pull/1502/head
Thomas Meyer 2019-07-27 15:36:31 +02:00
parent cc6bd4b590
commit ce1ea638ee
4 changed files with 32 additions and 21 deletions

View File

@ -38,11 +38,13 @@ import javax.persistence.Table;
@Table(name = "system_scope") @Table(name = "system_scope")
@NamedQueries({ @NamedQueries({
@NamedQuery(name = SystemScope.QUERY_ALL, query = "select s from SystemScope s ORDER BY s.id"), @NamedQuery(name = SystemScope.QUERY_ALL, query = "select s from SystemScope s ORDER BY s.id"),
@NamedQuery(name = SystemScope.QUERY_BY_VALUE, query = "select s from SystemScope s WHERE s.value = :" + SystemScope.PARAM_VALUE) @NamedQuery(name = SystemScope.QUERY_BY_VALUE, query = "select s from SystemScope s WHERE s.value = :" + SystemScope.PARAM_VALUE),
@NamedQuery(name = SystemScope.QUERY_BY_VALUES, query = "select s from SystemScope s WHERE s.value in :" + SystemScope.PARAM_VALUE)
}) })
public class SystemScope { public class SystemScope {
public static final String QUERY_BY_VALUE = "SystemScope.getByValue"; public static final String QUERY_BY_VALUE = "SystemScope.getByValue";
public static final String QUERY_BY_VALUES = "SystemScope.getByValues";
public static final String QUERY_ALL = "SystemScope.findAll"; public static final String QUERY_ALL = "SystemScope.findAll";
public static final String PARAM_VALUE = "value"; public static final String PARAM_VALUE = "value";

View File

@ -36,6 +36,8 @@ public interface SystemScopeRepository {
public SystemScope getByValue(String value); public SystemScope getByValue(String value);
public Set<SystemScope> getByValues(Set<String> values);
public void remove(SystemScope scope); public void remove(SystemScope scope);
public SystemScope save(SystemScope scope); public SystemScope save(SystemScope scope);

View File

@ -23,6 +23,7 @@ package org.mitre.oauth2.repository.impl;
import static org.mitre.util.jpa.JpaUtil.getSingleResult; import static org.mitre.util.jpa.JpaUtil.getSingleResult;
import static org.mitre.util.jpa.JpaUtil.saveOrUpdate; import static org.mitre.util.jpa.JpaUtil.saveOrUpdate;
import java.util.HashSet;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.Set; import java.util.Set;
@ -76,6 +77,13 @@ public class JpaSystemScopeRepository implements SystemScopeRepository {
return getSingleResult(query.getResultList()); return getSingleResult(query.getResultList());
} }
@Override
public Set<SystemScope> getByValues(Set<String> values) {
TypedQuery<SystemScope> query = em.createNamedQuery(SystemScope.QUERY_BY_VALUES, SystemScope.class);
query.setParameter(SystemScope.PARAM_VALUE, values);
return new HashSet<>(query.getResultList());
}
/* (non-Javadoc) /* (non-Javadoc)
* @see org.mitre.oauth2.repository.SystemScopeRepository#remove(org.mitre.oauth2.model.SystemScope) * @see org.mitre.oauth2.repository.SystemScopeRepository#remove(org.mitre.oauth2.model.SystemScope)
*/ */

View File

@ -20,8 +20,10 @@
*/ */
package org.mitre.oauth2.service.impl; package org.mitre.oauth2.service.impl;
import java.util.HashSet;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.Set; import java.util.Set;
import java.util.stream.Collectors;
import org.mitre.oauth2.model.SystemScope; import org.mitre.oauth2.model.SystemScope;
import org.mitre.oauth2.repository.SystemScopeRepository; import org.mitre.oauth2.repository.SystemScopeRepository;
@ -32,7 +34,6 @@ import org.springframework.stereotype.Service;
import com.google.common.base.Function; import com.google.common.base.Function;
import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.common.base.Predicates; import com.google.common.base.Predicates;
import com.google.common.base.Strings;
import com.google.common.collect.Collections2; import com.google.common.collect.Collections2;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
@ -67,24 +68,6 @@ public class DefaultSystemScopeService implements SystemScopeService {
} }
}; };
private Function<String, SystemScope> stringToSystemScope = new Function<String, SystemScope>() {
@Override
public SystemScope apply(String input) {
if (Strings.isNullOrEmpty(input)) {
return null;
} else {
// get the real scope if it's available
SystemScope s = getByValue(input);
if (s == null) {
// make a fake one otherwise
s = new SystemScope(input);
}
return s;
}
}
};
private Function<SystemScope, String> systemScopeToString = new Function<SystemScope, String>() { private Function<SystemScope, String> systemScopeToString = new Function<SystemScope, String>() {
@Override @Override
public String apply(SystemScope input) { public String apply(SystemScope input) {
@ -120,6 +103,10 @@ public class DefaultSystemScopeService implements SystemScopeService {
return repository.getByValue(value); return repository.getByValue(value);
} }
private Set<SystemScope> getByValues(Set<String> values) {
return repository.getByValues(values);
}
/* (non-Javadoc) /* (non-Javadoc)
* @see org.mitre.oauth2.service.SystemScopeService#remove(org.mitre.oauth2.model.SystemScope) * @see org.mitre.oauth2.service.SystemScopeService#remove(org.mitre.oauth2.model.SystemScope)
*/ */
@ -149,7 +136,19 @@ public class DefaultSystemScopeService implements SystemScopeService {
if (scope == null) { if (scope == null) {
return null; return null;
} else { } else {
return new LinkedHashSet<>(Collections2.filter(Collections2.transform(scope, stringToSystemScope), Predicates.notNull())); Set<String> scopeValues = scope.stream().filter(s -> s != null).collect(Collectors.toSet());
Set<SystemScope> scopesFromDB = getByValues(scopeValues);
Set<String> scopesFromDBValues = scopesFromDB.stream().map(SystemScope::getValue).collect(Collectors.toSet());
Set<SystemScope> missingScopesFromDB = scopesFromDB
.stream()
.map(SystemScope::getValue)
.filter(sv -> !scopesFromDBValues.contains(sv))
.map(sv -> new SystemScope(sv))
.collect(Collectors.toSet());
Set<SystemScope> allScopes = new HashSet<SystemScope>();
allScopes.addAll(scopesFromDB);
allScopes.addAll(missingScopesFromDB);
return allScopes;
} }
} }