Skip to content

Commit a1fa1de

Browse files
authored
fix(pagination): split service and scope queries to avoid paginating scopes (#26)
1 parent dd05d12 commit a1fa1de

File tree

2 files changed

+169
-150
lines changed

2 files changed

+169
-150
lines changed

src/main/java/org/fiware/iam/repository/ServiceRepository.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,25 @@
22

33
import io.micronaut.core.annotation.NonNull;
44
import io.micronaut.data.annotation.Join;
5+
import io.micronaut.data.annotation.Query;
56
import io.micronaut.data.model.Page;
67
import io.micronaut.data.model.Pageable;
78
import io.micronaut.data.repository.PageableRepository;
89

10+
import java.util.List;
911
import java.util.Optional;
1012

1113
/**
1214
* Extension of the base repository to support {@link Service}
1315
*/
1416
public interface ServiceRepository extends PageableRepository<Service, String> {
1517

16-
@Join(value = "oidcScopes", type = Join.Type.LEFT_FETCH)
17-
Optional<Service> findById(@NonNull String id);
18+
@Join(value = "oidcScopes", type = Join.Type.LEFT_FETCH)
19+
Optional<Service> findById(@NonNull String id);
1820

19-
@Join(value = "oidcScopes", type = Join.Type.LEFT_FETCH)
20-
@NonNull Page<Service> findAll(@NonNull Pageable pageable);
21+
@NonNull
22+
Page<Service> findAll(@NonNull Pageable pageable);
23+
24+
@Query("SELECT * FROM scope_entry WHERE service_id IN (:serviceIds)")
25+
List<ScopeEntry> findScopesByServiceIds(List<String> serviceIds);
2126
}

src/main/java/org/fiware/iam/rest/ServiceApiController.java

Lines changed: 160 additions & 146 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@
1212
import lombok.extern.slf4j.Slf4j;
1313
import org.fiware.iam.ServiceMapper;
1414
import org.fiware.iam.ccs.api.ServiceApi;
15-
import org.fiware.iam.ccs.model.*;
15+
import org.fiware.iam.ccs.model.CredentialVO;
16+
import org.fiware.iam.ccs.model.ServiceScopesEntryVO;
17+
import org.fiware.iam.ccs.model.ServiceVO;
18+
import org.fiware.iam.ccs.model.ServicesVO;
1619
import org.fiware.iam.exception.ConflictException;
1720
import org.fiware.iam.repository.ScopeEntry;
1821
import org.fiware.iam.repository.ScopeEntryRepository;
@@ -34,27 +37,27 @@
3437
@RequiredArgsConstructor
3538
public class ServiceApiController implements ServiceApi {
3639

37-
private final ServiceRepository serviceRepository;
40+
private final ServiceRepository serviceRepository;
3841
private final ScopeEntryRepository scopeEntryRepository;
39-
private final ServiceMapper serviceMapper;
42+
private final ServiceMapper serviceMapper;
4043

41-
@Override
42-
public HttpResponse<Object> createService(@NonNull ServiceVO serviceVO) {
43-
if (serviceVO.getId() != null && serviceRepository.existsById(serviceVO.getId())) {
44-
throw new ConflictException(String.format("The service with id %s already exists.", serviceVO.getId()),
45-
serviceVO.getId());
46-
}
47-
validateServiceVO(serviceVO);
44+
@Override
45+
public HttpResponse<Object> createService(@NonNull ServiceVO serviceVO) {
46+
if (serviceVO.getId() != null && serviceRepository.existsById(serviceVO.getId())) {
47+
throw new ConflictException(String.format("The service with id %s already exists.", serviceVO.getId()),
48+
serviceVO.getId());
49+
}
50+
validateServiceVO(serviceVO);
4851

49-
Service mappedService = serviceMapper.map(serviceVO);
52+
Service mappedService = serviceMapper.map(serviceVO);
5053

51-
Service savedService = serviceRepository.save(mappedService);
54+
Service savedService = serviceRepository.save(mappedService);
5255

53-
return HttpResponse.created(
54-
URI.create(
55-
ServiceApi.PATH_GET_SERVICE.replace(
56-
"{id}", savedService.getId())));
57-
}
56+
return HttpResponse.created(
57+
URI.create(
58+
ServiceApi.PATH_GET_SERVICE.replace(
59+
"{id}", savedService.getId())));
60+
}
5861

5962
@Transactional
6063
@Override
@@ -71,57 +74,68 @@ public HttpResponse<Object> deleteServiceById(@NonNull String id) {
7174
return HttpResponse.noContent();
7275
}
7376

74-
@Override
75-
public HttpResponse<List<String>> getScopeForService(@NonNull String id, @Nullable String oidcScope) {
76-
Optional<Service> service = serviceRepository.findById(id);
77-
if (service.isEmpty()) {
78-
return HttpResponse.notFound();
79-
}
80-
String selectedOidcScope =
81-
oidcScope == null ?
82-
serviceMapper.map(service.get()).getDefaultOidcScope() :
83-
oidcScope;
84-
ServiceScopesEntryVO serviceScopesEntryVO = serviceMapper.map(service.get())
85-
.getOidcScopes()
86-
.get(selectedOidcScope);
87-
return HttpResponse.ok(
88-
Optional.ofNullable(
89-
serviceScopesEntryVO.getCredentials())
90-
.orElse(List.of()).stream().map(CredentialVO::getType).toList());
91-
}
92-
93-
@Override
94-
public HttpResponse<ServiceVO> getService(@NonNull String id) {
95-
Optional<ServiceVO> serviceVO = serviceRepository.findById(id)
96-
.map(serviceMapper::map);
97-
return serviceVO
98-
.map(HttpResponse::ok)
99-
.orElse(HttpResponse.notFound());
100-
}
101-
102-
@Override
103-
public HttpResponse<ServicesVO> getServices(@Nullable Integer nullablePageSize,
104-
@Nullable Integer nullablePage) {
105-
var pageSize = Optional.ofNullable(nullablePageSize).orElse(100);
106-
var page = Optional.ofNullable(nullablePage).orElse(0);
107-
if (pageSize < 1) {
108-
throw new IllegalArgumentException("PageSize has to be at least 1.");
109-
}
110-
if (page < 0) {
111-
throw new IllegalArgumentException("Offsets below 0 are not supported.");
112-
}
113-
114-
Page<Service> requestedPage = serviceRepository.findAll(
115-
Pageable.from(page, pageSize, Sort.of(Sort.Order.asc("id"))));
116-
return HttpResponse.ok(
117-
new ServicesVO()
118-
.total((int) requestedPage.getTotalSize())
119-
.pageNumber(page)
120-
.pageSize(requestedPage.getContent().size())
121-
.services(requestedPage.getContent().stream().map(serviceMapper::map).toList()));
122-
}
123-
124-
@Transactional
77+
@Override
78+
public HttpResponse<List<String>> getScopeForService(@NonNull String id, @Nullable String oidcScope) {
79+
Optional<Service> service = serviceRepository.findById(id);
80+
if (service.isEmpty()) {
81+
return HttpResponse.notFound();
82+
}
83+
String selectedOidcScope =
84+
oidcScope == null ?
85+
serviceMapper.map(service.get()).getDefaultOidcScope() :
86+
oidcScope;
87+
ServiceScopesEntryVO serviceScopesEntryVO = serviceMapper.map(service.get())
88+
.getOidcScopes()
89+
.get(selectedOidcScope);
90+
return HttpResponse.ok(
91+
Optional.ofNullable(
92+
serviceScopesEntryVO.getCredentials())
93+
.orElse(List.of()).stream().map(CredentialVO::getType).toList());
94+
}
95+
96+
@Override
97+
public HttpResponse<ServiceVO> getService(@NonNull String id) {
98+
Optional<ServiceVO> serviceVO = serviceRepository.findById(id)
99+
.map(serviceMapper::map);
100+
return serviceVO
101+
.map(HttpResponse::ok)
102+
.orElse(HttpResponse.notFound());
103+
}
104+
105+
@Override
106+
public HttpResponse<ServicesVO> getServices(@Nullable Integer nullablePageSize,
107+
@Nullable Integer nullablePage) {
108+
var pageSize = Optional.ofNullable(nullablePageSize).orElse(100);
109+
var page = Optional.ofNullable(nullablePage).orElse(0);
110+
if (pageSize < 1) {
111+
throw new IllegalArgumentException("PageSize has to be at least 1.");
112+
}
113+
if (page < 0) {
114+
throw new IllegalArgumentException("Offsets below 0 are not supported.");
115+
}
116+
117+
Page<Service> requestedPage = serviceRepository.findAll(
118+
Pageable.from(page, pageSize, Sort.of(Sort.Order.asc("id"))));
119+
List<Service> services = requestedPage.getContent();
120+
121+
if (!services.isEmpty()) {
122+
List<String> serviceIds = requestedPage.getContent().stream()
123+
.map(Service::getId)
124+
.toList();
125+
List<ScopeEntry> allScopes = serviceRepository.findScopesByServiceIds(serviceIds);
126+
Map<String, List<ScopeEntry>> scopesByServiceId = allScopes.stream()
127+
.collect(Collectors.groupingBy(scope -> scope.getService().getId()));
128+
services.forEach(service -> service.setOidcScopes(scopesByServiceId.get(service.getId())));
129+
}
130+
return HttpResponse.ok(
131+
new ServicesVO()
132+
.total((int) requestedPage.getTotalSize())
133+
.pageNumber(page)
134+
.pageSize(services.size())
135+
.services(services.stream().map(serviceMapper::map).toList()));
136+
}
137+
138+
@Transactional
125139
@Override
126140
public HttpResponse<ServiceVO> updateService(@NonNull String id, @NonNull ServiceVO serviceVO) {
127141
if (serviceVO.getId() != null && !id.equals(serviceVO.getId())) {
@@ -143,85 +157,85 @@ public HttpResponse<ServiceVO> updateService(@NonNull String id, @NonNull Servic
143157
Service toBeUpdated = serviceMapper.map(serviceVO);
144158
toBeUpdated.setId(id);
145159
Service updatedService = serviceRepository.update(toBeUpdated);
146-
160+
147161
return HttpResponse.ok(serviceMapper.map(updatedService));
148162
}
149163

150-
// validate a service vo, e.g. check forbidden null values
151-
private void validateServiceVO(ServiceVO serviceVO) {
152-
if (serviceVO.getDefaultOidcScope() == null) {
153-
throw new IllegalArgumentException("Default OIDC scope cannot be null.");
154-
}
155-
if (serviceVO.getOidcScopes() == null) {
156-
throw new IllegalArgumentException("OIDC scopes cannot be null.");
157-
}
158-
159-
String defaultOidcScope = serviceVO.getDefaultOidcScope();
160-
ServiceScopesEntryVO serviceScopesEntryVO = serviceVO
161-
.getOidcScopes()
162-
.get(defaultOidcScope);
163-
if (serviceScopesEntryVO == null) {
164-
throw new IllegalArgumentException("Default OIDC scope must exist in OIDC scopes array.");
165-
}
166-
167-
Optional<CredentialVO> nullType = Optional.ofNullable(serviceScopesEntryVO
168-
.getCredentials())
169-
.orElse(List.of())
170-
.stream()
171-
.filter(cvo -> cvo.getType() == null)
172-
.findFirst();
173-
if (nullType.isPresent()) {
174-
throw new IllegalArgumentException("Type of a credential cannot be null.");
175-
}
176-
177-
serviceVO
178-
.getOidcScopes()
179-
.values()
180-
.forEach(this::validateKeyMappings);
181-
}
182-
183-
private void validateKeyMappings(ServiceScopesEntryVO scopeEntry) {
184-
if (scopeEntry.getFlatClaims()) {
185-
List<String> includedKeys = Optional.ofNullable(scopeEntry.getCredentials())
186-
.orElse(List.of())
187-
.stream()
188-
.filter(cvo -> cvo.getJwtInclusion().getEnabled())
189-
.flatMap(credentialVO ->
190-
Optional.ofNullable(credentialVO.getJwtInclusion()
191-
.getClaimsToInclude())
192-
.orElse(List.of())
193-
.stream()
194-
.map(claim -> {
195-
if (claim.getNewKey() != null && !claim.getNewKey().isEmpty()) {
196-
return claim.getNewKey();
197-
}
198-
return claim.getOriginalKey();
199-
})
200-
).toList();
201-
if (includedKeys.size() != new HashSet(includedKeys).size()) {
202-
throw new IllegalArgumentException("Configuration contains duplicate claim keys.");
203-
}
204-
} else {
205-
Optional.ofNullable(scopeEntry.getCredentials())
206-
.orElse(List.of())
207-
.stream()
208-
.filter(cvo -> cvo.getJwtInclusion().getEnabled())
209-
.forEach(cvo -> {
210-
List<String> claimKeys = Optional.ofNullable(cvo.getJwtInclusion()
211-
.getClaimsToInclude())
212-
.orElse(List.of())
213-
.stream()
214-
.map(claim -> {
215-
if (claim.getNewKey() != null && !claim.getNewKey().isEmpty()) {
216-
return claim.getNewKey();
217-
}
218-
return claim.getOriginalKey();
219-
})
220-
.toList();
221-
if (claimKeys.size() != new HashSet(claimKeys).size()) {
222-
throw new IllegalArgumentException("Configuration contains duplicate claim keys.");
223-
}
224-
});
225-
}
226-
}
164+
// validate a service vo, e.g. check forbidden null values
165+
private void validateServiceVO(ServiceVO serviceVO) {
166+
if (serviceVO.getDefaultOidcScope() == null) {
167+
throw new IllegalArgumentException("Default OIDC scope cannot be null.");
168+
}
169+
if (serviceVO.getOidcScopes() == null) {
170+
throw new IllegalArgumentException("OIDC scopes cannot be null.");
171+
}
172+
173+
String defaultOidcScope = serviceVO.getDefaultOidcScope();
174+
ServiceScopesEntryVO serviceScopesEntryVO = serviceVO
175+
.getOidcScopes()
176+
.get(defaultOidcScope);
177+
if (serviceScopesEntryVO == null) {
178+
throw new IllegalArgumentException("Default OIDC scope must exist in OIDC scopes array.");
179+
}
180+
181+
Optional<CredentialVO> nullType = Optional.ofNullable(serviceScopesEntryVO
182+
.getCredentials())
183+
.orElse(List.of())
184+
.stream()
185+
.filter(cvo -> cvo.getType() == null)
186+
.findFirst();
187+
if (nullType.isPresent()) {
188+
throw new IllegalArgumentException("Type of a credential cannot be null.");
189+
}
190+
191+
serviceVO
192+
.getOidcScopes()
193+
.values()
194+
.forEach(this::validateKeyMappings);
195+
}
196+
197+
private void validateKeyMappings(ServiceScopesEntryVO scopeEntry) {
198+
if (scopeEntry.getFlatClaims()) {
199+
List<String> includedKeys = Optional.ofNullable(scopeEntry.getCredentials())
200+
.orElse(List.of())
201+
.stream()
202+
.filter(cvo -> cvo.getJwtInclusion().getEnabled())
203+
.flatMap(credentialVO ->
204+
Optional.ofNullable(credentialVO.getJwtInclusion()
205+
.getClaimsToInclude())
206+
.orElse(List.of())
207+
.stream()
208+
.map(claim -> {
209+
if (claim.getNewKey() != null && !claim.getNewKey().isEmpty()) {
210+
return claim.getNewKey();
211+
}
212+
return claim.getOriginalKey();
213+
})
214+
).toList();
215+
if (includedKeys.size() != new HashSet(includedKeys).size()) {
216+
throw new IllegalArgumentException("Configuration contains duplicate claim keys.");
217+
}
218+
} else {
219+
Optional.ofNullable(scopeEntry.getCredentials())
220+
.orElse(List.of())
221+
.stream()
222+
.filter(cvo -> cvo.getJwtInclusion().getEnabled())
223+
.forEach(cvo -> {
224+
List<String> claimKeys = Optional.ofNullable(cvo.getJwtInclusion()
225+
.getClaimsToInclude())
226+
.orElse(List.of())
227+
.stream()
228+
.map(claim -> {
229+
if (claim.getNewKey() != null && !claim.getNewKey().isEmpty()) {
230+
return claim.getNewKey();
231+
}
232+
return claim.getOriginalKey();
233+
})
234+
.toList();
235+
if (claimKeys.size() != new HashSet(claimKeys).size()) {
236+
throw new IllegalArgumentException("Configuration contains duplicate claim keys.");
237+
}
238+
});
239+
}
240+
}
227241
}

0 commit comments

Comments
 (0)