fix(store): reject incomplete restore files and move corrupt ones aside
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
d010c51e2a
commit
f29728cb9f
3 files changed
+83
-9
No files matched your search
@@ -0,0 +1,23 @@
|
||||
package home.gateway.adapters.store;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Path;
|
||||
|
||||
/**
|
||||
* The restore file cannot be trusted. It was renamed to {@link #quarantined()}, so the pending
|
||||
* restores it held are not lost; starting without them is the caller's explicit decision.
|
||||
*/
|
||||
public class CorruptRestoreFileException extends IOException {
|
||||
|
||||
private final Path quarantined;
|
||||
|
||||
CorruptRestoreFileException(Path file, Path quarantined, Throwable cause) {
|
||||
super("restore file " + file + " is corrupt and was moved to " + quarantined + ": "
|
||||
+ cause.getMessage(), cause);
|
||||
this.quarantined = quarantined;
|
||||
}
|
||||
|
||||
public Path quarantined() {
|
||||
return quarantined;
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,8 @@
|
||||
package home.gateway.adapters.store;
|
||||
|
||||
import com.fasterxml.jackson.core.type.TypeReference;
|
||||
import com.fasterxml.jackson.databind.DeserializationFeature;
|
||||
import com.fasterxml.jackson.databind.MapperFeature;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import com.fasterxml.jackson.databind.SerializationFeature;
|
||||
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
|
||||
@@ -22,25 +24,41 @@ public class JsonRestoreStore implements RestoreStore {
|
||||
private final Path file;
|
||||
private final ObjectMapper json = new ObjectMapper()
|
||||
.registerModule(new JavaTimeModule())
|
||||
.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
|
||||
.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
|
||||
// Jackson would otherwise turn a missing or null number into 0, i.e. a restore of register 0.
|
||||
.enable(DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES)
|
||||
.enable(DeserializationFeature.FAIL_ON_NULL_CREATOR_PROPERTIES)
|
||||
.enable(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES)
|
||||
.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
|
||||
.disable(MapperFeature.ALLOW_COERCION_OF_SCALARS);
|
||||
|
||||
public JsonRestoreStore(Path file) {
|
||||
this.file = file;
|
||||
}
|
||||
|
||||
/** @throws CorruptRestoreFileException after moving an unusable file aside */
|
||||
@Override
|
||||
public List<Restore> load() throws IOException {
|
||||
if (!Files.exists(file)) {
|
||||
return List.of();
|
||||
}
|
||||
try {
|
||||
return json.readValue(file.toFile(), new TypeReference<List<Restore>>() { });
|
||||
var restores = json.readValue(file.toFile(), new TypeReference<List<Restore>>() { });
|
||||
if (restores == null || restores.contains(null)) {
|
||||
throw new IOException("not a list of restores");
|
||||
}
|
||||
return restores;
|
||||
} catch (IOException e) {
|
||||
throw new IOException("restore file " + file + " is unreadable, fix or remove it by hand: "
|
||||
+ e.getMessage(), e);
|
||||
throw quarantine(e);
|
||||
}
|
||||
}
|
||||
|
||||
private CorruptRestoreFileException quarantine(IOException cause) throws IOException {
|
||||
var aside = file.resolveSibling(file.getFileName() + ".bad-" + System.currentTimeMillis());
|
||||
Files.move(file, aside, StandardCopyOption.ATOMIC_MOVE);
|
||||
return new CorruptRestoreFileException(file, aside, cause);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void save(List<Restore> restores) throws IOException {
|
||||
var temp = file.resolveSibling(file.getFileName() + ".tmp");
|
||||
|
||||
@@ -11,6 +11,7 @@ import java.time.Instant;
|
||||
import java.util.List;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
@@ -59,15 +60,47 @@ class JsonRestoreStoreTest {
|
||||
assertEquals(List.of(restore), store.load());
|
||||
}
|
||||
|
||||
private static String restoreJson(String register, String original) {
|
||||
return "[{\"setpoint\":\"x\",\"register\":" + register + ",\"originalValue\":" + original
|
||||
+ ",\"appliedValue\":9,\"due\":\"2026-10-24T11:00:00Z\"}]";
|
||||
}
|
||||
|
||||
@Test
|
||||
void corruptFileFailsWithAClearMessageAndIsKept() throws IOException {
|
||||
void anUnusableFileIsMovedAsideAndReportedNeverSilentlyDropped() throws IOException {
|
||||
var cases = List.of("{not json", "", "garbage \u0000\u0001", "null", "{}", "[null]",
|
||||
"[{\"setpoint\":\"x\"}]", restoreJson("4144", "\"5\""), restoreJson("null", "5"),
|
||||
restoreJson("4144", "5").replace("}]", ",\"extra\":1}]"));
|
||||
for (var content : cases) {
|
||||
var file = dir.resolve("restores.json");
|
||||
Files.writeString(file, content);
|
||||
|
||||
var error = assertThrows(CorruptRestoreFileException.class,
|
||||
() -> new JsonRestoreStore(file).load(), content);
|
||||
|
||||
assertTrue(error.getMessage().contains(file.toString()), content);
|
||||
assertFalse(Files.exists(file), content);
|
||||
assertTrue(error.quarantined().getFileName().toString().startsWith("restores.json.bad-"));
|
||||
assertEquals(content, Files.readString(error.quarantined()));
|
||||
Files.delete(error.quarantined());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void validFileWithAllFieldsStillLoads() throws IOException {
|
||||
var file = dir.resolve("restores.json");
|
||||
Files.writeString(file, restoreJson("4144", "450"));
|
||||
|
||||
assertEquals(1, new JsonRestoreStore(file).load().size());
|
||||
}
|
||||
|
||||
@Test
|
||||
void afterTheRenameTheNextLoadIsEmpty() throws IOException {
|
||||
var file = dir.resolve("restores.json");
|
||||
Files.writeString(file, "{not json");
|
||||
var store = new JsonRestoreStore(file);
|
||||
assertThrows(CorruptRestoreFileException.class, store::load);
|
||||
|
||||
var error = assertThrows(IOException.class, () -> new JsonRestoreStore(file).load());
|
||||
|
||||
assertTrue(error.getMessage().contains(file.toString()));
|
||||
assertEquals("{not json", Files.readString(file));
|
||||
assertEquals(List.of(), store.load());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
Reference in new issue
Block a user