Skip to content

Commit

Permalink
feat: do not return default settings if requested name is not exist (#23
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@grigoriev
grigoriev authored Jun 4, 2024
1 parent 87cff1a commit 18905bb
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 50 deletions.
115 changes: 75 additions & 40 deletions app/src/main/java/ch/sbb/polarion/extension/generic/settings/GenericNamedSettings.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package ch.sbb.polarion.extension.generic.settings;

import ch.sbb.polarion.extension.generic.exception.ObjectNotFoundException;
import ch.sbb.polarion.extension.generic.util.ContextUtils;
import ch.sbb.polarion.extension.generic.util.ScopeUtils;
import com.polarion.alm.shared.util.Pair;
Expand All @@ -8,6 +9,7 @@
import com.polarion.subterra.base.location.ILocation;
import lombok.Getter;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.Collection;
import java.util.Comparator;
Expand Down Expand Up @@ -56,12 +58,13 @@ protected GenericNamedSettings(String featureName, SettingsService settingsServi
}

@SuppressWarnings("unused")
public T load(String project, SettingId id) {
public T load(@Nullable String project, @NotNull SettingId id) {
try {
return read(ScopeUtils.getScopeFromProject(project), id, null);
} catch (ObjectNotFoundException e) {
throw e;
} catch (Exception e) {
logger.error("Cannot load saved model for project '" + project + "': " + e.getMessage(), e);
return fromString("");
throw new IllegalStateException("Cannot load saved model for project '" + project + "': " + e.getMessage(), e);
}
}

Expand All @@ -78,7 +81,7 @@ public Collection<SettingName> readNames(@NotNull String scope) {
if (names.isEmpty()) {
// If there are no settings persisted - try to create default one
try {
createDefaultSettings();
saveDefaultSettingsInGlobalRepo();
} catch (Exception e) { // If it's not possible to create the settings in read only transaction, so just ignore it
logger.warn("Cannot create the settings in read only transaction, creation will be skipped: " + e.getMessage(), e);
}
Expand All @@ -90,31 +93,46 @@ public Collection<SettingName> readNames(@NotNull String scope) {
}

@Override
public T read(@NotNull String scope, @NotNull SettingId id, String revisionName) {
String settingPath = String.format(LOCATION_MASK, settingsFolder, getFileName(scope, false, id), SETTINGS_FILE_EXTENSION);
final ILocation location = ScopeUtils.getContextLocation(scope).append(settingPath);
String value = settingsService.read(location, revisionName);
if (value == null) {
if (!StringUtils.isEmpty(revisionName)) {
return null;
}
ILocation defaultLocation = ScopeUtils.getDefaultLocation().append(settingPath);
if (!settingsService.exists(defaultLocation)) {
try {
return createDefaultSettings();
} catch (Exception e) {
logger.warn("Cannot create the settings in read only transaction, default values will be used: " + e.getMessage(), e);
return defaultValues();
}
public @NotNull T read(@NotNull String scope, @NotNull SettingId id, @Nullable String revisionName) {
@Nullable String fileName = getFileName(scope, false, id);
@Nullable String value = readFileContent(scope, fileName, revisionName);

if (value == null && StringUtils.isEmpty(revisionName)) {
value = readFileContent(DEFAULT_SCOPE, fileName, null);
}

return value == null ? handleMissingValue(id) : fromString(value);
}

private @Nullable String readFileContent(@NotNull String scope, @Nullable String fileName, @Nullable String revisionName) {
if (fileName == null) {
return null;
}
String settingPath = String.format(LOCATION_MASK, settingsFolder, fileName, SETTINGS_FILE_EXTENSION);
ILocation location = ScopeUtils.getContextLocation(scope).append(settingPath);
return settingsService.read(location, revisionName);
}

private @NotNull T handleMissingValue(@NotNull SettingId id) {
if (DEFAULT_NAME.equals(id.getIdentifier())) {
try {
return saveDefaultSettingsInGlobalRepo();
} catch (Exception e) {
logger.warn("Cannot create the settings in read-only transaction, default values will be used: " + e.getMessage(), e);
return defaultValues();
}
value = settingsService.read(defaultLocation, null);
}
return fromString(value);
throw new ObjectNotFoundException("Setting '%s' not found".formatted(id.getIdentifier()));
}

@Override
public T save(@NotNull String scope, @NotNull SettingId id, @NotNull T what) {
String settingPath = String.format(LOCATION_MASK, settingsFolder, getFileName(scope, true, id), SETTINGS_FILE_EXTENSION);
public @NotNull T save(@NotNull String scope, @NotNull SettingId id, @NotNull T what) {
@Nullable String fileName = getFileName(scope, true, id);
if (StringUtils.isEmpty(fileName)) {
throw new IllegalArgumentException("Provided filename is empty");
}

String settingPath = String.format(LOCATION_MASK, settingsFolder, fileName, SETTINGS_FILE_EXTENSION);
final ILocation location = ScopeUtils.getContextLocation(scope).append(settingPath);
what.setBundleTimestamp(currentBundleTimestamp());
beforeSave(what);
Expand All @@ -136,13 +154,21 @@ public void afterSave(@NotNull T what) {

@Override
public void delete(@NotNull String scope, @NotNull SettingId id) {
String settingPath = String.format(LOCATION_MASK, settingsFolder, getFileName(scope, true, id), SETTINGS_FILE_EXTENSION);
@Nullable String fileName = getFileName(scope, true, id);
if (StringUtils.isEmpty(fileName)) {
throw new ObjectNotFoundException("Setting '%s' not found and that's why can not be deleted".formatted(id.getIdentifier()));
}
String settingPath = String.format(LOCATION_MASK, settingsFolder, fileName, SETTINGS_FILE_EXTENSION);
final ILocation location = ScopeUtils.getContextLocation(scope).append(settingPath);
settingsService.delete(location);
}

public @NotNull List<Revision> listRevisions(String scope, SettingId id) {
String settingPath = String.format(LOCATION_MASK, settingsFolder, getFileName(scope, false, id), SETTINGS_FILE_EXTENSION);
@Nullable String fileName = getFileName(scope, false, id);
if (StringUtils.isEmpty(fileName)) {
throw new ObjectNotFoundException("Setting '%s' not found".formatted(id.getIdentifier()));
}
String settingPath = String.format(LOCATION_MASK, settingsFolder, fileName, SETTINGS_FILE_EXTENSION);
final ILocation location = ScopeUtils.getContextLocation(scope).append(settingPath);
final String projectId = ScopeUtils.getProjectFromScope(scope);
return settingsService.listRevisions(location, projectId);
Expand All @@ -164,14 +190,16 @@ public int hashCode() {
return Objects.hash(getFeatureName());
}

public String getIdByName(String scope, boolean limitToScope, String settingName) {
return readNames(scope).stream().filter(
n -> Objects.equals(n.getName(), settingName) && (!limitToScope || Objects.equals(n.getScope(), scope))
).map(SettingName::getId).findFirst().orElse(null);
public @Nullable String getIdByName(String scope, boolean limitToScope, String settingName) {
return readNames(scope).stream()
.filter(n -> Objects.equals(n.getName(), settingName) && (!limitToScope || Objects.equals(n.getScope(), scope)))
.map(SettingName::getId)
.findFirst()
.orElse(null);
}

private String getFileName(String scope, boolean limitToScope, SettingId settingId) {
return settingId.useName ? getIdByName(scope, limitToScope, settingId.identifier) : settingId.identifier;
private @Nullable String getFileName(String scope, boolean limitToScope, SettingId settingId) {
return settingId.isUseName() ? getIdByName(scope, limitToScope, settingId.getIdentifier()) : settingId.getIdentifier();
}

private Set<SettingName> getSettingNamesFromLocation(String scope) {
Expand All @@ -183,21 +211,28 @@ private Set<SettingName> getSettingNamesFromLocation(String scope) {
Pair<String, Set<SettingName>> cached = SETTING_NAMES_CACHE.get(targetLocation);
if (cached == null || !Objects.equals(lastRevision, cached.left())) {
Set<SettingName> settingNames = settingsService.getPersistedSettingFileNames(targetLocation).stream()
.map(fileName -> SettingName.builder()
.id(fileName)
//Legacy items (which were created before 'id' introduction) don't have 'name' model field - in this case we reuse their file names.
//Proper 'name' field value will appear only after explicit settings save by user.
.name(Optional.ofNullable(read(scope, SettingId.fromId(fileName), null).getName()).orElse(fileName))
.scope(scope)
.build())
.map(fileNameWithoutExtension -> buildSettingName(scope, fileNameWithoutExtension))
.collect(Collectors.toSet());
cached = Pair.of(lastRevision, settingNames);
SETTING_NAMES_CACHE.put(targetLocation, cached);
}
return cached.right();
}

private T createDefaultSettings() {
private SettingName buildSettingName(String scope, String id) {
//Legacy items (which were created before 'id' introduction) don't have 'name' model field - in this case we reuse their file names.
//Proper 'name' field value will appear only after explicit settings save by user.
T settingsModel = read(scope, SettingId.fromId(id), null);
String name = Optional.ofNullable(settingsModel.getName()).orElse(id);

return SettingName.builder()
.id(id)
.name(name)
.scope(scope)
.build();
}

private @NotNull T saveDefaultSettingsInGlobalRepo() {
T defaultModel = defaultValues();
defaultModel.setName(DEFAULT_NAME);
return save(DEFAULT_SCOPE, SettingId.fromId(DEFAULT_NAME), defaultModel);
Expand Down
13 changes: 4 additions & 9 deletions app/src/main/java/ch/sbb/polarion/extension/generic/settings/SettingId.java
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package ch.sbb.polarion.extension.generic.settings;

import lombok.Getter;

import java.util.Objects;

/**
* Settings can be queried/processed using either NAME or ID (fileName).
*/
@Getter
public final class SettingId {

/**
Expand All @@ -27,14 +30,6 @@ public static SettingId fromId(String id) {
return new SettingId(id, false);
}

public String getIdentifier() {
return identifier;
}

@SuppressWarnings("unused")
public boolean isUseName() {
return useName;
}

@Override
public boolean equals(Object o) {
Expand All @@ -43,7 +38,7 @@ public boolean equals(Object o) {
return false;
}
SettingId settingId = (SettingId) o;
return useName == settingId.useName && Objects.equals(identifier, settingId.identifier);
return useName == settingId.isUseName() && Objects.equals(identifier, settingId.getIdentifier());
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/resources/js/common.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ const SbbCommon = {
}
if (revisions.length === 0) {
let cell = tableBody.insertRow().insertCell();
cell.setAttribute("colspan", "5")
cell.setAttribute("colspan", "6")
cell.className = 'empty-message';
cell.appendChild(document.createTextNode("No saved revisions yet."));
}
Expand Down

0 comments on commit 18905bb

Please sign in to comment.