Skip to content

Commit

Permalink
🎨 properly handling the potential lack of service configuration struc…
Browse files Browse the repository at this point in the history
…ture via Optionals instead of null values

Signed-off-by: dseurotech <[email protected]>
  • Loading branch information
dseurotech committed Aug 1, 2024
1 parent 7192338 commit 7ece681
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public KapuaTocd getConfigMetadata(KapuaId scopeId) throws KapuaException {
//Temporary, use Optional instead
return new EmptyTocd();
}
return serviceConfigurationManager.getConfigMetadata(scopeId, true);
return serviceConfigurationManager.getConfigMetadata(scopeId, true).orElse(null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ default boolean isServiceEnabled(TxContext txContext, KapuaId scopeId) {

Map<String, Object> getConfigValues(KapuaId scopeId, boolean excludeDisabled) throws KapuaException;

KapuaTocd getConfigMetadata(KapuaId scopeId, boolean excludeDisabled) throws KapuaException;
Optional<KapuaTocd> getConfigMetadata(KapuaId scopeId, boolean excludeDisabled) throws KapuaException;

ServiceComponentConfiguration extractServiceComponentConfiguration(KapuaId scopeId) throws KapuaException;
Optional<ServiceComponentConfiguration> extractServiceComponentConfiguration(KapuaId scopeId) throws KapuaException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,24 +88,24 @@ public Map<String, Object> getConfigValues(TxContext txContext, KapuaId scopeId,
}

@Override
public KapuaTocd getConfigMetadata(KapuaId scopeId, boolean excludeDisabled) throws KapuaException {
public Optional<KapuaTocd> getConfigMetadata(KapuaId scopeId, boolean excludeDisabled) throws KapuaException {
// Argument validation
ArgumentValidator.notNull(scopeId, "scopeId");

// Get the Tocd
// Keep distinct values for service PID, Scope ID and disabled properties included/excluded from AD
Pair<KapuaId, Boolean> cacheKey = Pair.of(scopeId, excludeDisabled);
final Pair<KapuaId, Boolean> cacheKey = Pair.of(scopeId, excludeDisabled);
try {
// Check if the OCD is already in cache, but not in the "empty" cache
KapuaTocd tocd;
tocd = kapuaTocdLocalCache.get(cacheKey);
if (tocd == null && !kapuaTocdEmptyLocalCache.get(cacheKey)) {
Optional<KapuaTocd> tocd;
tocd = Optional.ofNullable(kapuaTocdLocalCache.get(cacheKey));
if (!tocd.isPresent() && !kapuaTocdEmptyLocalCache.get(cacheKey)) {
// If not, read metadata and process it
tocd = wrapped.getConfigMetadata(scopeId, excludeDisabled);
// If null, put it in the "empty" ocd cache, else put it in the "standard" cache
if (tocd != null) {
if (tocd.isPresent()) {
// If the value is not null, put it in "standard" cache and remove the entry from the "empty" cache if present
kapuaTocdLocalCache.put(cacheKey, tocd);
kapuaTocdLocalCache.put(cacheKey, tocd.get());
kapuaTocdEmptyLocalCache.remove(cacheKey);
} else {
// If the value is null, just remember we already read it from file at least once
Expand All @@ -119,7 +119,7 @@ public KapuaTocd getConfigMetadata(KapuaId scopeId, boolean excludeDisabled) thr
}

@Override
public ServiceComponentConfiguration extractServiceComponentConfiguration(KapuaId scopeId) throws KapuaException {
public Optional<ServiceComponentConfiguration> extractServiceComponentConfiguration(KapuaId scopeId) throws KapuaException {
return wrapped.extractServiceComponentConfiguration(scopeId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.io.IOException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -32,7 +33,6 @@
import org.eclipse.kapua.KapuaIllegalArgumentException;
import org.eclipse.kapua.KapuaIllegalNullArgumentException;
import org.eclipse.kapua.commons.security.KapuaSecurityUtils;
import org.eclipse.kapua.commons.service.internal.KapuaServiceDisabledException;
import org.eclipse.kapua.commons.util.ArgumentValidator;
import org.eclipse.kapua.commons.util.ResourceUtils;
import org.eclipse.kapua.commons.util.StringUtil;
Expand Down Expand Up @@ -131,11 +131,21 @@ public void checkAllowedEntities(TxContext txContext, KapuaId scopeId, String en
@Override
public void setConfigValues(KapuaId scopeId, Optional<KapuaId> parentId, Map<String, Object> values) throws KapuaException {
txManager.<Void>execute(tx -> {
KapuaTocd ocd = doGetConfigMetadata(tx, scopeId, false);
Optional<KapuaTocd> maybeOcd = doGetConfigMetadata(tx, scopeId, false);
if (!maybeOcd.isPresent()) {
return null;
//throw?
}

final Optional<KapuaTocd> maybeKapuaTocd = doGetConfigMetadata(tx, scopeId, true);
if (!maybeKapuaTocd.isPresent()) {
return null;
//throw?
}

Map<String, Object> originalValues = doGetConfigValues(tx, scopeId, doGetConfigMetadata(tx, scopeId, true));
Map<String, Object> originalValues = doGetConfigValues(tx, scopeId, maybeKapuaTocd.get());

for (KapuaTad ad : ocd.getAD()) {
for (KapuaTad ad : maybeOcd.get().getAD()) {
boolean allowSelfEdit = Boolean.parseBoolean(ad.getOtherAttributes().getOrDefault(new QName("allowSelfEdit"), "false"));

final KapuaId currentUserId = KapuaSecurityUtils.getSession().getUserId();
Expand All @@ -155,7 +165,7 @@ public void setConfigValues(KapuaId scopeId, Optional<KapuaId> parentId, Map<Str
}
}

validateConfigurations(tx, ocd, values, scopeId, parentId);
validateConfigurations(tx, maybeOcd.get(), values, scopeId, parentId);

ServiceConfigQueryImpl query = new ServiceConfigQueryImpl(scopeId);
query.setPredicate(
Expand Down Expand Up @@ -361,8 +371,14 @@ public Map<String, Object> getConfigValues(TxContext txContext, KapuaId scopeId,
}

protected Map<String, Object> doGetConfigValues(TxContext txContext, KapuaId scopeId, boolean excludeDisabled) throws KapuaException {
final KapuaTocd metadata = doGetConfigMetadata(txContext, scopeId, excludeDisabled);
return doGetConfigValues(txContext, scopeId, metadata);
final Optional<KapuaTocd> metadata = doGetConfigMetadata(txContext, scopeId, excludeDisabled);
return metadata.map(m -> {
try {
return doGetConfigValues(txContext, scopeId, m);
} catch (KapuaException e) {
throw new RuntimeException(e);
}
}).orElse(Collections.emptyMap());
}

protected Map<String, Object> doGetConfigValues(TxContext txContext, KapuaId scopeId, KapuaTocd metadata) throws KapuaException {
Expand Down Expand Up @@ -414,16 +430,16 @@ private static Map<String, Object> toValues(@NotNull KapuaTocd ocd, Properties p
* @since 1.3.0
*/
@Override
public KapuaTocd getConfigMetadata(KapuaId scopeId, boolean excludeDisabled) throws KapuaException {
public Optional<KapuaTocd> getConfigMetadata(KapuaId scopeId, boolean excludeDisabled) throws KapuaException {
return txManager.execute(txContext -> doGetConfigMetadata(txContext, scopeId, excludeDisabled));
}

protected KapuaTocd doGetConfigMetadata(TxContext txContext, KapuaId scopeId, boolean excludeDisabled) throws KapuaException {
protected Optional<KapuaTocd> doGetConfigMetadata(TxContext txContext, KapuaId scopeId, boolean excludeDisabled) throws KapuaException {
// Argument validation
ArgumentValidator.notNull(scopeId, "scopeId");
// Check disabled service
if (!isServiceEnabled(txContext, scopeId)) {
throw new KapuaServiceDisabledException(pid);
return Optional.empty();
}
// Get the Tocd
try {
Expand All @@ -434,15 +450,19 @@ protected KapuaTocd doGetConfigMetadata(TxContext txContext, KapuaId scopeId, bo
}

@Override
public ServiceComponentConfiguration extractServiceComponentConfiguration(KapuaId scopeId) throws KapuaException {
public Optional<ServiceComponentConfiguration> extractServiceComponentConfiguration(KapuaId scopeId) throws KapuaException {
return txManager.execute(txContext -> {
final KapuaTocd metadata = this.doGetConfigMetadata(txContext, scopeId, true);
final Optional<KapuaTocd> maybeMetadata = this.doGetConfigMetadata(txContext, scopeId, true);
if (!maybeMetadata.isPresent()) {
return Optional.empty();
}
final KapuaTocd metadata = maybeMetadata.get();
final Map<String, Object> values = this.doGetConfigValues(txContext, scopeId, metadata);
final ServiceComponentConfiguration res = new ServiceComponentConfiguration(metadata.getId());
res.setDefinition(metadata);
res.setName(metadata.getName());
res.setProperties(values);
return res;
return Optional.of(res);
});
}

Expand All @@ -456,16 +476,16 @@ public ServiceComponentConfiguration extractServiceComponentConfiguration(KapuaI
* @return The processed {@link KapuaTocd}.
* @since 1.3.0
*/
private KapuaTocd processMetadata(TxContext txContext, KapuaTmetadata metadata, KapuaId scopeId, boolean excludeDisabled) {
private Optional<KapuaTocd> processMetadata(TxContext txContext, KapuaTmetadata metadata, KapuaId scopeId, boolean excludeDisabled) {
if (metadata != null && metadata.getOCD() != null && !metadata.getOCD().isEmpty()) {
for (KapuaTocd ocd : metadata.getOCD()) {
if (ocd.getId() != null && ocd.getId().equals(pid) && isServiceEnabled(txContext, scopeId)) {
ocd.getAD().removeIf(ad -> excludeDisabled && !isPropertyEnabled(ad, scopeId));
return ocd;
return Optional.of(ocd);
}
}
}
return null;
return Optional.empty();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import javax.inject.Inject;

Expand All @@ -31,6 +32,7 @@
public class ServiceConfigurationsFacadeImpl implements ServiceConfigurationsFacade {

private final Map<Class<?>, ServiceConfigurationManager> serviceConfigurationManagersByServiceClass;
private final Map<String, ServiceConfigurationManager> serviceConfigurationManagersByServiceClassName;
protected final AuthorizationService authorizationService;
protected final PermissionFactory permissionFactory;
protected final AccountService accountService;
Expand All @@ -39,6 +41,8 @@ public class ServiceConfigurationsFacadeImpl implements ServiceConfigurationsFac
public ServiceConfigurationsFacadeImpl(Map<Class<?>, ServiceConfigurationManager> serviceConfigurationManagersByServiceClass, AuthorizationService authorizationService,
PermissionFactory permissionFactory, AccountService accountService) {
this.serviceConfigurationManagersByServiceClass = serviceConfigurationManagersByServiceClass;
this.serviceConfigurationManagersByServiceClassName =
serviceConfigurationManagersByServiceClass.entrySet().stream().collect(Collectors.toMap(kv -> kv.getKey().getName(), kv -> kv.getValue()));
this.authorizationService = authorizationService;
this.permissionFactory = permissionFactory;
this.accountService = accountService;
Expand All @@ -52,38 +56,38 @@ public ServiceConfiguration fetchAllConfigurations(KapuaId scopeId) throws Kapua
if (!authorizationService.isPermitted(permissionFactory.newPermission(configurableService.getDomain(), Actions.read, scopeId))) {
continue;
}
res.getComponentConfigurations().add(configurableService.extractServiceComponentConfiguration(scopeId));
configurableService.extractServiceComponentConfiguration(scopeId).ifPresent(res.getComponentConfigurations()::add);
}
return res;
}

@Override
public ServiceComponentConfiguration fetchConfiguration(KapuaId scopeId, String serviceId) throws KapuaException {
final ServiceConfigurationManager serviceConfigurationManager = serviceConfigurationManagersByServiceClass.get(serviceId);
final ServiceConfigurationManager serviceConfigurationManager = serviceConfigurationManagersByServiceClassName.get(serviceId);
if (serviceConfigurationManager == null) {
throw new KapuaIllegalArgumentException("service.pid", serviceId);
}
authorizationService.checkPermission(permissionFactory.newPermission(serviceConfigurationManager.getDomain(), Actions.read, scopeId));
return serviceConfigurationManager.extractServiceComponentConfiguration(scopeId);
return serviceConfigurationManager.extractServiceComponentConfiguration(scopeId).orElse(null);
}

@Override
public void update(KapuaId scopeId, ServiceConfiguration newServiceConfiguration) throws KapuaException {
final Account account = accountService.find(scopeId);

for (ServiceComponentConfiguration newServiceComponentConfiguration : newServiceConfiguration.getComponentConfigurations()) {
doUpdateServiceComponentConfiguration(account, scopeId, newServiceComponentConfiguration);
doUpdateServiceComponentConfiguration(account, scopeId, newServiceComponentConfiguration.getId(), newServiceComponentConfiguration);
}
}

@Override
public void update(KapuaId scopeId, String serviceId, ServiceComponentConfiguration newServiceComponentConfiguration) throws KapuaException {
final Account account = accountService.find(scopeId);
doUpdateServiceComponentConfiguration(account, scopeId, newServiceComponentConfiguration);
doUpdateServiceComponentConfiguration(account, scopeId, serviceId, newServiceComponentConfiguration);
}

private void doUpdateServiceComponentConfiguration(Account account, KapuaId scopeId, ServiceComponentConfiguration newServiceComponentConfiguration) throws KapuaException {
final ServiceConfigurationManager serviceConfigurationManager = serviceConfigurationManagersByServiceClass.get(newServiceComponentConfiguration.getId());
private void doUpdateServiceComponentConfiguration(Account account, KapuaId scopeId, String serviceId, ServiceComponentConfiguration newServiceComponentConfiguration) throws KapuaException {
final ServiceConfigurationManager serviceConfigurationManager = serviceConfigurationManagersByServiceClassName.get(serviceId);
if (serviceConfigurationManager == null) {
throw new KapuaIllegalArgumentException("serviceConfiguration.componentConfiguration.id", newServiceComponentConfiguration.getId());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ public DeviceConnectionServiceConfigurationManager(
* @since 2.0.0
*/
@Override
public KapuaTocd doGetConfigMetadata(TxContext txContext, KapuaId scopeId, boolean excludeDisabled) throws KapuaException {
protected Optional<KapuaTocd> doGetConfigMetadata(TxContext txContext, KapuaId scopeId, boolean excludeDisabled) throws KapuaException {

KapuaTocd deviceConnectionServiceConfigDefinition = super.doGetConfigMetadata(txContext, scopeId, excludeDisabled);
Optional<KapuaTocd> deviceConnectionServiceConfigDefinition = super.doGetConfigMetadata(txContext, scopeId, excludeDisabled);

// Find the 'deviceConnectionAuthenticationType' KapuaTad
Optional<KapuaTad> authenticationTypeConfigDefinition = findAuthenticationTypeTad(deviceConnectionServiceConfigDefinition);
Optional<KapuaTad> authenticationTypeConfigDefinition = deviceConnectionServiceConfigDefinition.flatMap(this::findAuthenticationTypeTad);

// Add the KapuaToption to the KapuaTad
authenticationTypeConfigDefinition.ifPresent(tad -> {
Expand Down

0 comments on commit 7ece681

Please sign in to comment.