diff --git a/model/map-file/src/main/java/org/keycloak/models/map/storage/file/FileMapKeycloakTransaction.java b/model/map-file/src/main/java/org/keycloak/models/map/storage/file/FileMapKeycloakTransaction.java index 9850d89e08..cd067d54a4 100644 --- a/model/map-file/src/main/java/org/keycloak/models/map/storage/file/FileMapKeycloakTransaction.java +++ b/model/map-file/src/main/java/org/keycloak/models/map/storage/file/FileMapKeycloakTransaction.java @@ -17,6 +17,7 @@ package org.keycloak.models.map.storage.file; +import org.jboss.logging.Logger; import org.keycloak.models.map.common.AbstractEntity; import org.keycloak.models.map.common.DeepCloner; import org.keycloak.models.map.common.StringKeyConverter; @@ -32,15 +33,18 @@ import org.keycloak.storage.ReadOnlyException; import org.keycloak.storage.SearchableModelField; import java.io.IOException; import java.io.UncheckedIOException; -import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileTime; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.function.Function; /** @@ -51,8 +55,12 @@ import java.util.function.Function; public class FileMapKeycloakTransaction extends ConcurrentHashMapKeycloakTransaction { + private static final Logger LOG = Logger.getLogger(FileMapKeycloakTransaction.class); + + private final List createdPaths = new LinkedList<>(); private final List pathsToDelete = new LinkedList<>(); private final Map renameOnCommit = new HashMap<>(); + private final Map lastModified = new HashMap<>(); private final String txId = StringKey.INSTANCE.yieldNewUniqueKey(); @@ -77,16 +85,30 @@ public class FileMapKeycloakTransaction allChangedPaths = new HashSet<>(); + allChangedPaths.addAll(this.renameOnCommit.values()); + allChangedPaths.addAll(this.pathsToDelete); + allChangedPaths.forEach(this::checkIsSafeToModify); + try { + this.renameOnCommit.forEach(FileMapKeycloakTransaction::move); + this.pathsToDelete.forEach(FileMapKeycloakTransaction::silentDelete); + // TODO: catch exception thrown by move and try to restore any previously completed moves. + } finally { + // ensure all temp files are removed. + this.renameOnCommit.keySet().forEach(FileMapKeycloakTransaction::silentDelete); + // remove any created files that may have been left empty. + this.createdPaths.forEach(path -> silenteDelete(path, true)); + } } private static void move(Path from, Path to) { @@ -98,18 +120,24 @@ public class FileMapKeycloakTransaction { diff --git a/model/map-file/src/main/java/org/keycloak/models/map/storage/file/FileMapStorage.java b/model/map-file/src/main/java/org/keycloak/models/map/storage/file/FileMapStorage.java index 6b49943e7b..1d2ac1691b 100644 --- a/model/map-file/src/main/java/org/keycloak/models/map/storage/file/FileMapStorage.java +++ b/model/map-file/src/main/java/org/keycloak/models/map/storage/file/FileMapStorage.java @@ -45,6 +45,7 @@ import java.io.UncheckedIOException; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.FileTime; import java.util.Arrays; import java.util.IdentityHashMap; import java.util.List; @@ -207,6 +208,7 @@ public class FileMapStorage imple } protected V parse(Path fileName) { + getLastModifiedTime(fileName); final V parsedObject = YamlParser.parse(fileName, new MapEntityContext<>(entityClass)); if (parsedObject == null) { LOG.debugf("Could not parse %s%s", fileName, StackUtil.getShortStackTrace()); @@ -368,7 +370,7 @@ public class FileMapStorage imple if (sp == null) { throw new IllegalArgumentException("Invalid path: " + escapedId); } - + checkIsSafeToModify(sp); // TODO: improve locking synchronized (FileMapStorageProviderFactory.class) { writeYamlContents(sp, value); @@ -430,6 +432,22 @@ public class FileMapStorage imple protected abstract String getTxId(); + /** + * Hook to obtain the last modified time of the file identified by the supplied {@link Path}. + * + * @param path the {@link Path} to the file whose last modified time it to be obtained. + * @return the {@link FileTime} corresponding to the file's last modified time. + */ + protected abstract FileTime getLastModifiedTime(final Path path); + + /** + * Hook to validate that it is safe to modify the file identified by the supplied {@link Path}. The primary + * goal is to identify if other transactions have modified the file after it was read by the current transaction, + * preventing updates to a stale entity. + * + * @param path the {@link Path} to the file that is to be modified. + */ + protected abstract void checkIsSafeToModify(final Path path); } }