Skip to content

Commit 6ebe33c

Browse files
vramperezVíctor Rampérez Martín
vramperez
and
Víctor Rampérez Martín
authored
fix(scope-entry): fix scope update and delete bugs (#24)
Co-authored-by: Víctor Rampérez Martín <vramperez@MacBook-Air-de-Victor.local>
1 parent b808dc4 commit 6ebe33c

File tree

7 files changed

+141
-61
lines changed

7 files changed

+141
-61
lines changed

‎src/main/java/org/fiware/iam/ServiceMapper.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ default Service map(ServiceVO serviceVO) {
3535
.map(e -> {
3636
ScopeEntry scopeEntry = new ScopeEntry();
3737
scopeEntry.setScopeKey(e.getKey());
38+
scopeEntry.setService(service);
3839
scopeEntry.setCredentials(Optional.ofNullable(e.getValue().getCredentials()).
3940
orElse(List.of()).stream().map(this::map).toList());
4041
scopeEntry.setPresentationDefinition(this.map(e.getValue().getPresentationDefinition()));

‎src/main/java/org/fiware/iam/repository/H2ScopeEntryRepository.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package org.fiware.iam.repository;
2+
3+
import io.micronaut.context.annotation.Requires;
4+
import io.micronaut.data.jdbc.annotation.JdbcRepository;
5+
import io.micronaut.data.model.query.builder.sql.Dialect;
6+
7+
/**
8+
* Extension of the {@link ScopeEntryRepository} for the MySql-dialect
9+
*/
10+
@Requires(property = "datasources.default.dialect", value = "H2")
11+
@JdbcRepository(dialect = Dialect.H2)
12+
public interface H2ScopeEntryRepository extends ScopeEntryRepository {
13+
}

‎src/main/java/org/fiware/iam/repository/MySqlScopeEntryRepository.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package org.fiware.iam.repository;
2+
3+
import io.micronaut.context.annotation.Requires;
4+
import io.micronaut.data.jdbc.annotation.JdbcRepository;
5+
import io.micronaut.data.model.query.builder.sql.Dialect;
6+
7+
/**
8+
* Extension of the {@link ScopeEntryRepository} for the MySql-dialect
9+
*/
10+
@Requires(property = "datasources.default.dialect", value = "MYSQL")
11+
@JdbcRepository(dialect = Dialect.MYSQL)
12+
public interface MySqlScopeEntryRepository extends ScopeEntryRepository {
13+
}

‎src/main/java/org/fiware/iam/repository/PostgresSqlScopeEntryRepository.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package org.fiware.iam.repository;
2+
3+
import io.micronaut.context.annotation.Requires;
4+
import io.micronaut.data.jdbc.annotation.JdbcRepository;
5+
import io.micronaut.data.model.query.builder.sql.Dialect;
6+
7+
/**
8+
* Extension of the {@link ScopeEntryRepository} for the MySql-dialect
9+
*/
10+
@Requires(property = "datasources.default.dialect", value = "POSTGRES")
11+
@JdbcRepository(dialect = Dialect.POSTGRES)
12+
public interface PostgresSqlScopeEntryRepository extends ScopeEntryRepository {
13+
}

‎src/main/java/org/fiware/iam/repository/ScopeEntryRepository.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package org.fiware.iam.repository;
2+
3+
import io.micronaut.data.repository.CrudRepository;
4+
5+
/**
6+
* Extension of the base repository to support {@link ScopeEntry}
7+
*/
8+
public interface ScopeEntryRepository extends CrudRepository<ScopeEntry, Long> {
9+
10+
/**
11+
* Deletes all scope entries associated with a specific service.
12+
* Micronaut will generate: DELETE FROM scope_entry WHERE service_id = ?
13+
*/
14+
void deleteByService(Service service);
15+
}

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

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.fiware.iam.ccs.model.*;
1616
import org.fiware.iam.exception.ConflictException;
1717
import org.fiware.iam.repository.ScopeEntry;
18+
import org.fiware.iam.repository.ScopeEntryRepository;
1819
import org.fiware.iam.repository.Service;
1920
import org.fiware.iam.repository.ServiceRepository;
2021

@@ -34,6 +35,7 @@
3435
public class ServiceApiController implements ServiceApi {
3536

3637
private final ServiceRepository serviceRepository;
38+
private final ScopeEntryRepository scopeEntryRepository;
3739
private final ServiceMapper serviceMapper;
3840

3941
@Override
@@ -54,14 +56,20 @@ public HttpResponse<Object> createService(@NonNull ServiceVO serviceVO) {
5456
"{id}", savedService.getId())));
5557
}
5658

57-
@Override
58-
public HttpResponse<Object> deleteServiceById(@NonNull String id) {
59-
if (!serviceRepository.existsById(id)) {
60-
return HttpResponse.notFound();
61-
}
62-
serviceRepository.deleteById(id);
63-
return HttpResponse.noContent();
64-
}
59+
@Transactional
60+
@Override
61+
public HttpResponse<Object> deleteServiceById(@NonNull String id) {
62+
Optional<Service> service = serviceRepository.findById(id);
63+
64+
if (service.isEmpty()) {
65+
return HttpResponse.notFound();
66+
}
67+
68+
scopeEntryRepository.deleteByService(service.get());
69+
serviceRepository.deleteById(id);
70+
71+
return HttpResponse.noContent();
72+
}
6573

6674
@Override
6775
public HttpResponse<List<String>> getScopeForService(@NonNull String id, @Nullable String oidcScope) {
@@ -113,39 +121,31 @@ public HttpResponse<ServicesVO> getServices(@Nullable Integer nullablePageSize,
113121
.services(requestedPage.getContent().stream().map(serviceMapper::map).toList()));
114122
}
115123

116-
@Transactional
117-
@Override
118-
public HttpResponse<ServiceVO> updateService(@NonNull String id, @NonNull ServiceVO serviceVO) {
119-
if (serviceVO.getId() != null && !id.equals(serviceVO.getId())) {
120-
throw new IllegalArgumentException("The id of a service cannot be updated.");
121-
}
122-
validateServiceVO(serviceVO);
123-
Optional<Service> optionalOrginalService = serviceRepository.findById(id);
124-
if (optionalOrginalService.isEmpty()) {
125-
return HttpResponse.notFound();
126-
}
127-
128-
Service originalService = optionalOrginalService.get();
129-
Service toBeUpdated = serviceMapper.map(serviceVO);
130-
Map<String, ScopeEntry> scopeEntryMap = originalService.getOidcScopes()
131-
.stream().collect(Collectors.toMap(ScopeEntry::getScopeKey, sE -> sE));
132-
133-
List<ScopeEntry> scopeEntries = toBeUpdated.getOidcScopes().stream()
134-
.map(scopeEntry -> {
135-
if (scopeEntryMap.containsKey(scopeEntry.getScopeKey())) {
136-
Long originalId = scopeEntryMap.get(scopeEntry.getScopeKey()).getId();
137-
scopeEntry.setId(originalId);
138-
}
139-
return scopeEntry;
140-
}).toList();
141-
142-
// just in case none is set in the object
143-
toBeUpdated.setId(id);
144-
toBeUpdated.setOidcScopes(scopeEntries);
145-
146-
Service updatedService = serviceRepository.update(toBeUpdated);
147-
return HttpResponse.ok(serviceMapper.map(updatedService));
148-
}
124+
@Transactional
125+
@Override
126+
public HttpResponse<ServiceVO> updateService(@NonNull String id, @NonNull ServiceVO serviceVO) {
127+
if (serviceVO.getId() != null && !id.equals(serviceVO.getId())) {
128+
throw new IllegalArgumentException("The id of a service cannot be updated.");
129+
}
130+
validateServiceVO(serviceVO);
131+
132+
Optional<Service> optionalOriginalService = serviceRepository.findById(id);
133+
if (optionalOriginalService.isEmpty()) {
134+
return HttpResponse.notFound();
135+
}
136+
137+
Service originalService = optionalOriginalService.get();
138+
139+
// Delete all existing scope entries for this service to avoid duplicates/orphans
140+
// This is necessary because Micronaut Data JDBC doesn't automatically handle orphan removal in updates
141+
scopeEntryRepository.deleteByService(originalService);
142+
143+
Service toBeUpdated = serviceMapper.map(serviceVO);
144+
toBeUpdated.setId(id);
145+
Service updatedService = serviceRepository.update(toBeUpdated);
146+
147+
return HttpResponse.ok(serviceMapper.map(updatedService));
148+
}
149149

150150
// validate a service vo, e.g. check forbidden null values
151151
private void validateServiceVO(ServiceVO serviceVO) {

‎src/test/java/org/fiware/iam/rest/ServiceApiControllerTest.java‎

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -649,26 +649,51 @@ public void updateService404() throws Exception {
649649
"Only existing services can be updated.");
650650
}
651651

652-
private void assertServiceVOsEqual(ServiceVO service0, ServiceVO service1) {
653-
assertEquals(service0.getId(), service1.getId(), "ID should be equal.");
654-
assertEquals(service0.getDefaultOidcScope(), service1.getDefaultOidcScope(), "Services should have the equal default scope.");
655-
assertEquals(service0.getOidcScopes().keySet(), service1.getOidcScopes().keySet(), "The services should have the same scopes.");
656-
657-
Set<String> scopeKeys = service0.getOidcScopes().keySet();
658-
for (String scope : scopeKeys) {
659-
assertScopeEntryEquals(service0.getOidcScopes().get(scope), service1.getOidcScopes().get(scope));
660-
}
661-
662-
}
663-
664-
private void assertScopeEntryEquals(ServiceScopesEntryVO entryVO1, ServiceScopesEntryVO entryVO2) {
665-
assertEquals(entryVO1.getCredentials().size(), entryVO2.getCredentials().size(), "The scopes should have the same number of entries.");
666-
667-
// we don't care about order
668-
Set<OrderIgnoringCredential> entries1 = entryVO1.getCredentials().stream().map(this::fromCredentialVO).collect(Collectors.toSet());
669-
Set<OrderIgnoringCredential> entries2 = entryVO2.getCredentials().stream().map(this::fromCredentialVO).collect(Collectors.toSet());
670-
assertEquals(entries1, entries2, "Credentials in the scope should be equal.");
671-
}
652+
@Test
653+
public void updateServiceRemoveScope200() throws Exception {
654+
ServiceScopesEntryVO entry1 = ServiceScopesEntryVOTestExample.build();
655+
ServiceScopesEntryVO entry2 = ServiceScopesEntryVOTestExample.build();
656+
CredentialVO credential1 = CredentialVOTestExample.build();
657+
CredentialVO credential2 = CredentialVOTestExample.build();
658+
addToScopeEntry(entry1, credential1);
659+
addToScopeEntry(entry2, credential2);
660+
661+
Map<String, ServiceScopesEntryVO> scopes = new HashMap<>();
662+
scopes.put("scope1", entry1);
663+
scopes.put("scope2", entry2);
664+
665+
ServiceVO service = ServiceVOTestExample.build().id("service-remove-scope").oidcScopes(scopes);
666+
service.setDefaultOidcScope("scope1");
667+
668+
assertEquals(HttpStatus.CREATED, testClient.createService(service).getStatus(), "Initial creation should succeed.");
669+
670+
// Remove scope2 and update
671+
service.setOidcScopes(Map.of("scope1", entry1));
672+
673+
HttpResponse<ServiceVO> updatedResponse = testClient.updateService(service.getId(), service);
674+
assertEquals(HttpStatus.OK, updatedResponse.getStatus(), "The service should be updated successfully.");
675+
676+
// Fetch all services and compare the stored service by id
677+
HttpResponse<ServicesVO> servicesResponse = testClient.getServices(null, null);
678+
assertEquals(HttpStatus.OK, servicesResponse.getStatus(), "Fetching services should succeed.");
679+
ServicesVO servicesVO = servicesResponse.body();
680+
assertNotNull(servicesVO, "Services response body should not be null.");
681+
Optional<ServiceVO> opt = servicesVO.getServices().stream()
682+
.filter(s -> Objects.equals(s.getId(), service.getId()))
683+
.findFirst();
684+
assertTrue(opt.isPresent(), "Updated service should be present in the services list.");
685+
ServiceVO updatedFromList = opt.get();
686+
assertNotNull(updatedFromList);
687+
assertEquals(1, updatedFromList.getOidcScopes().size(), "Only one scope should remain after update.");
688+
assertTrue(updatedFromList.getOidcScopes().containsKey("scope1"), "Remaining scope should be scope1.");
689+
}
690+
691+
private void assertServiceVOsEqual(ServiceVO expected, ServiceVO actual) {
692+
assertEquals(expected.getId(), actual.getId());
693+
assertEquals(expected.getDefaultOidcScope(), actual.getDefaultOidcScope());
694+
assertEquals(expected.getOidcScopes().size(), actual.getOidcScopes().size());
695+
assertTrue(actual.getOidcScopes().keySet().containsAll(expected.getOidcScopes().keySet()));
696+
}
672697

673698
public OrderIgnoringCredential fromCredentialVO(CredentialVO credentialVO) {
674699
OrderIgnoringCredential orderIgnoringCredential = new OrderIgnoringCredential();

0 commit comments

Comments
 (0)