From ce1ea638ee59b200de9be1e8b851533d2385aa78 Mon Sep 17 00:00:00 2001
From: Thomas Meyer <thomas@m3y3r.de>
Date: Sat, 27 Jul 2019 15:36:31 +0200
Subject: [PATCH] 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.
---
.../org/mitre/oauth2/model/SystemScope.java | 4 +-
.../repository/SystemScopeRepository.java | 2 +
.../impl/JpaSystemScopeRepository.java | 8 ++++
.../impl/DefaultSystemScopeService.java | 39 +++++++++----------
4 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/openid-connect-common/src/main/java/org/mitre/oauth2/model/SystemScope.java b/openid-connect-common/src/main/java/org/mitre/oauth2/model/SystemScope.java
index 0807b160e..83e84dfd1 100644
--- a/openid-connect-common/src/main/java/org/mitre/oauth2/model/SystemScope.java
+++ b/openid-connect-common/src/main/java/org/mitre/oauth2/model/SystemScope.java
@@ -38,11 +38,13 @@ import javax.persistence.Table;
@Table(name = "system_scope")
@NamedQueries({
@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 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 PARAM_VALUE = "value";
diff --git a/openid-connect-common/src/main/java/org/mitre/oauth2/repository/SystemScopeRepository.java b/openid-connect-common/src/main/java/org/mitre/oauth2/repository/SystemScopeRepository.java
index 8c891d566..dd41ace47 100644
--- a/openid-connect-common/src/main/java/org/mitre/oauth2/repository/SystemScopeRepository.java
+++ b/openid-connect-common/src/main/java/org/mitre/oauth2/repository/SystemScopeRepository.java
@@ -36,6 +36,8 @@ public interface SystemScopeRepository {
public SystemScope getByValue(String value);
+ public Set<SystemScope> getByValues(Set<String> values);
+
public void remove(SystemScope scope);
public SystemScope save(SystemScope scope);
diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/repository/impl/JpaSystemScopeRepository.java b/openid-connect-server/src/main/java/org/mitre/oauth2/repository/impl/JpaSystemScopeRepository.java
index f646b5724..f2d575b19 100644
--- a/openid-connect-server/src/main/java/org/mitre/oauth2/repository/impl/JpaSystemScopeRepository.java
+++ b/openid-connect-server/src/main/java/org/mitre/oauth2/repository/impl/JpaSystemScopeRepository.java
@@ -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.saveOrUpdate;
+import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Set;
@@ -76,6 +77,13 @@ public class JpaSystemScopeRepository implements SystemScopeRepository {
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)
* @see org.mitre.oauth2.repository.SystemScopeRepository#remove(org.mitre.oauth2.model.SystemScope)
*/
diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultSystemScopeService.java b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultSystemScopeService.java
index 21474fe6e..61022a73c 100644
--- a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultSystemScopeService.java
+++ b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultSystemScopeService.java
@@ -20,8 +20,10 @@
*/
package org.mitre.oauth2.service.impl;
+import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Set;
+import java.util.stream.Collectors;
import org.mitre.oauth2.model.SystemScope;
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.Predicate;
import com.google.common.base.Predicates;
-import com.google.common.base.Strings;
import com.google.common.collect.Collections2;
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>() {
@Override
public String apply(SystemScope input) {
@@ -120,6 +103,10 @@ public class DefaultSystemScopeService implements SystemScopeService {
return repository.getByValue(value);
}
+ private Set<SystemScope> getByValues(Set<String> values) {
+ return repository.getByValues(values);
+ }
+
/* (non-Javadoc)
* @see org.mitre.oauth2.service.SystemScopeService#remove(org.mitre.oauth2.model.SystemScope)
*/
@@ -149,7 +136,19 @@ public class DefaultSystemScopeService implements SystemScopeService {
if (scope == null) {
return null;
} 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;
}
}