From 50f8e40d99c8940166e17a3d56f00a80f01f0f1a Mon Sep 17 00:00:00 2001 From: crschnick Date: Thu, 4 Jul 2024 14:06:50 +0000 Subject: [PATCH] More error fixes --- .../io/xpipe/app/ext/DataStoreProviders.java | 8 +-- .../xpipe/app/storage/DataStorageParser.java | 20 ------ .../xpipe/app/storage/DataStorageWriter.java | 14 ---- .../io/xpipe/app/storage/DataStoreEntry.java | 70 ++++++------------- .../io/xpipe/app/storage/StandardStorage.java | 14 +++- .../io/xpipe/core/util/CoreJacksonModule.java | 21 +++--- 6 files changed, 45 insertions(+), 102 deletions(-) delete mode 100644 app/src/main/java/io/xpipe/app/storage/DataStorageParser.java delete mode 100644 app/src/main/java/io/xpipe/app/storage/DataStorageWriter.java diff --git a/app/src/main/java/io/xpipe/app/ext/DataStoreProviders.java b/app/src/main/java/io/xpipe/app/ext/DataStoreProviders.java index 3b36cca63..85827c093 100644 --- a/app/src/main/java/io/xpipe/app/ext/DataStoreProviders.java +++ b/app/src/main/java/io/xpipe/app/ext/DataStoreProviders.java @@ -64,17 +64,11 @@ public class DataStoreProviders { @SuppressWarnings("unchecked") public static T byStore(DataStore store) { - return (T) byStoreClass(store.getClass()).orElseThrow(); - } - - @SuppressWarnings("unchecked") - public static Optional byStoreClass(Class c) { if (ALL == null) { throw new IllegalStateException("Not initialized"); } - return (Optional) - ALL.stream().filter(d -> d.getStoreClasses().contains(c)).findAny(); + return (T) ALL.stream().filter(d -> d.getStoreClasses().contains(store.getClass())).findAny().orElseThrow(() -> new IllegalArgumentException("Unknown store class")); } public static List getAll() { diff --git a/app/src/main/java/io/xpipe/app/storage/DataStorageParser.java b/app/src/main/java/io/xpipe/app/storage/DataStorageParser.java deleted file mode 100644 index eed8c8f95..000000000 --- a/app/src/main/java/io/xpipe/app/storage/DataStorageParser.java +++ /dev/null @@ -1,20 +0,0 @@ -package io.xpipe.app.storage; - -import io.xpipe.app.issue.ErrorEvent; -import io.xpipe.core.store.DataStore; -import io.xpipe.core.util.JacksonMapper; - -import com.fasterxml.jackson.databind.JsonNode; - -public class DataStorageParser { - - public static DataStore storeFromNode(JsonNode node) { - var mapper = JacksonMapper.getDefault(); - try { - return mapper.treeToValue(node, DataStore.class); - } catch (Throwable e) { - ErrorEvent.fromThrowable(e).handle(); - return null; - } - } -} diff --git a/app/src/main/java/io/xpipe/app/storage/DataStorageWriter.java b/app/src/main/java/io/xpipe/app/storage/DataStorageWriter.java deleted file mode 100644 index e018a68f0..000000000 --- a/app/src/main/java/io/xpipe/app/storage/DataStorageWriter.java +++ /dev/null @@ -1,14 +0,0 @@ -package io.xpipe.app.storage; - -import io.xpipe.core.store.DataStore; -import io.xpipe.core.util.JacksonMapper; - -import com.fasterxml.jackson.databind.JsonNode; - -public class DataStorageWriter { - - public static JsonNode storeToNode(DataStore store) { - var mapper = JacksonMapper.getDefault(); - return mapper.valueToTree(store); - } -} diff --git a/app/src/main/java/io/xpipe/app/storage/DataStoreEntry.java b/app/src/main/java/io/xpipe/app/storage/DataStoreEntry.java index f3d1aaed7..35ceb6fee 100644 --- a/app/src/main/java/io/xpipe/app/storage/DataStoreEntry.java +++ b/app/src/main/java/io/xpipe/app/storage/DataStoreEntry.java @@ -82,6 +82,7 @@ public class DataStoreEntry extends StorageElement { String name, Instant lastUsed, Instant lastModified, + DataStore store, JsonNode storeNode, boolean dirty, Validity validity, @@ -93,7 +94,7 @@ public class DataStoreEntry extends StorageElement { Order explicitOrder) { super(directory, uuid, name, lastUsed, lastModified, dirty); this.categoryUuid = categoryUuid; - this.store = DataStorageParser.storeFromNode(storeNode); + this.store = store; this.storeNode = storeNode; this.validity = validity; this.configuration = configuration; @@ -101,7 +102,7 @@ public class DataStoreEntry extends StorageElement { this.color = color; this.explicitOrder = explicitOrder; this.provider = store != null - ? DataStoreProviders.byStoreClass(store.getClass()).orElse(null) + ? DataStoreProviders.byStore(store) : null; this.storePersistentStateNode = storePersistentState; this.notes = notes; @@ -149,8 +150,9 @@ public class DataStoreEntry extends StorageElement { @SneakyThrows public static DataStoreEntry createNew( @NonNull UUID uuid, @NonNull UUID categoryUuid, @NonNull String name, @NonNull DataStore store) { - var node = DataStorageWriter.storeToNode(store); - var validity = DataStorageParser.storeFromNode(node) == null + var node = JacksonMapper.getDefault().valueToTree(store); + var storeFromNode = JacksonMapper.getDefault().treeToValue(node, DataStore.class); + var validity = storeFromNode == null ? Validity.LOAD_FAILED : store.isComplete() ? Validity.COMPLETE : Validity.INCOMPLETE; var entry = new DataStoreEntry( @@ -160,6 +162,7 @@ public class DataStoreEntry extends StorageElement { name, Instant.now(), Instant.now(), + storeFromNode, node, true, validity, @@ -172,39 +175,6 @@ public class DataStoreEntry extends StorageElement { return entry; } - @SneakyThrows - private static DataStoreEntry createExisting( - @NonNull Path directory, - @NonNull UUID uuid, - @NonNull UUID categoryUuid, - @NonNull String name, - @NonNull Instant lastUsed, - @NonNull Instant lastModified, - JsonNode storeNode, - Configuration configuration, - JsonNode storePersistentState, - boolean expanded, - DataStoreColor color, - String notes, - Order order) { - return new DataStoreEntry( - directory, - uuid, - categoryUuid, - name, - lastUsed, - lastModified, - storeNode, - false, - Validity.INCOMPLETE, - configuration, - storePersistentState, - expanded, - color, - notes, - order); - } - public static Optional fromDirectory(Path dir) throws Exception { ObjectMapper mapper = JacksonMapper.getDefault(); @@ -277,20 +247,19 @@ public class DataStoreEntry extends StorageElement { } // Store loading is prone to errors. - JsonNode storeNode = null; - try { - storeNode = DataStorageEncryption.readPossiblyEncryptedNode(mapper.readTree(storeFile.toFile())); - } catch (Exception e) { - ErrorEvent.fromThrowable(e).handle(); - } - return Optional.of(createExisting( + JsonNode storeNode = DataStorageEncryption.readPossiblyEncryptedNode(mapper.readTree(storeFile.toFile())); + var store = JacksonMapper.getDefault().treeToValue(storeNode, DataStore.class); + return Optional.of(new DataStoreEntry( dir, uuid, categoryUuid, name, lastUsed, lastModified, + store, storeNode, + false, + Validity.INCOMPLETE, configuration, persistentState, expanded, @@ -482,7 +451,7 @@ public class DataStoreEntry extends StorageElement { } this.store = store; - this.storeNode = DataStorageWriter.storeToNode(store); + this.storeNode = JacksonMapper.getDefault().valueToTree(store); if (updateTime) { lastModified = Instant.now(); } @@ -491,7 +460,7 @@ public class DataStoreEntry extends StorageElement { } public void reassignStore() { - this.storeNode = DataStorageWriter.storeToNode(store); + this.storeNode = JacksonMapper.getDefault().valueToTree(store); dirty = true; } @@ -528,7 +497,14 @@ public class DataStoreEntry extends StorageElement { return; } - var newStore = DataStorageParser.storeFromNode(storeNode); + DataStore newStore; + try { + newStore = JacksonMapper.getDefault().treeToValue(storeNode, DataStore.class); + } catch (Throwable e) { + ErrorEvent.fromThrowable(e).handle(); + newStore = null; + } + if (newStore == null) { store = null; validity = Validity.LOAD_FAILED; diff --git a/app/src/main/java/io/xpipe/app/storage/StandardStorage.java b/app/src/main/java/io/xpipe/app/storage/StandardStorage.java index 051c487c8..c10f196b1 100644 --- a/app/src/main/java/io/xpipe/app/storage/StandardStorage.java +++ b/app/src/main/java/io/xpipe/app/storage/StandardStorage.java @@ -1,5 +1,6 @@ package io.xpipe.app.storage; +import com.fasterxml.jackson.core.JacksonException; import io.xpipe.app.ext.DataStorageExtensionProvider; import io.xpipe.app.issue.ErrorEvent; import io.xpipe.app.issue.TrackEvent; @@ -7,6 +8,7 @@ import io.xpipe.app.prefs.AppPrefs; import io.xpipe.core.process.OsType; import io.xpipe.core.store.LocalStore; +import io.xpipe.core.util.JacksonMapper; import lombok.Getter; import org.apache.commons.io.FileUtils; @@ -129,6 +131,16 @@ public class StandardStorage extends DataStorage { } storeEntries.put(entry.get(), entry.get()); + } catch (JacksonException ex) { + // Data corruption and schema changes are expected + + // We only keep invalid entries in developer mode as there's no point in keeping them in + // production. + if (AppPrefs.get().isDevelopmentEnvironment()) { + directoriesToKeep.add(path); + } + + ErrorEvent.fromThrowable(ex).expected().omit().build().handle(); } catch (IOException ex) { // IO exceptions are not expected exception.set(new IOException("Unable to load data from " + path + ". Is it corrupted?", ex)); @@ -221,7 +233,7 @@ public class StandardStorage extends DataStorage { .filter(entry -> entry.getValidity() != DataStoreEntry.Validity.LOAD_FAILED) .forEach(entry -> { entry.dirty = true; - entry.setStoreNode(DataStorageWriter.storeToNode(entry.getStore())); + entry.setStoreNode(JacksonMapper.getDefault().valueToTree(entry.getStore())); }); save(false); } diff --git a/core/src/main/java/io/xpipe/core/util/CoreJacksonModule.java b/core/src/main/java/io/xpipe/core/util/CoreJacksonModule.java index 5228abc54..734e91329 100644 --- a/core/src/main/java/io/xpipe/core/util/CoreJacksonModule.java +++ b/core/src/main/java/io/xpipe/core/util/CoreJacksonModule.java @@ -1,5 +1,13 @@ package io.xpipe.core.util; +import com.fasterxml.jackson.annotation.JsonIdentityInfo; +import com.fasterxml.jackson.annotation.ObjectIdGenerators; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.fasterxml.jackson.databind.jsontype.NamedType; +import com.fasterxml.jackson.databind.module.SimpleModule; import io.xpipe.core.dialog.BaseQueryElement; import io.xpipe.core.dialog.BusyElement; import io.xpipe.core.dialog.ChoiceElement; @@ -11,18 +19,7 @@ import io.xpipe.core.store.FilePath; import io.xpipe.core.store.LocalStore; import io.xpipe.core.store.StorePath; -import com.fasterxml.jackson.annotation.JsonIdentityInfo; -import com.fasterxml.jackson.annotation.ObjectIdGenerators; -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.*; -import com.fasterxml.jackson.databind.annotation.JsonSerialize; -import com.fasterxml.jackson.databind.jsontype.NamedType; -import com.fasterxml.jackson.databind.module.SimpleModule; -import com.fasterxml.jackson.databind.type.ArrayType; - import java.io.IOException; -import java.lang.reflect.WildcardType; import java.nio.charset.Charset; import java.nio.file.Path; import java.util.List; @@ -35,8 +32,6 @@ public class CoreJacksonModule extends SimpleModule { context.registerSubtypes( new NamedType(InPlaceSecretValue.class), new NamedType(LocalStore.class), - new NamedType(ArrayType.class), - new NamedType(WildcardType.class), new NamedType(BaseQueryElement.class), new NamedType(ChoiceElement.class), new NamedType(BusyElement.class),