0verEngineer / InlineProblems

Plugin to show problems inside the text editor for IDEs based on the IntelliJ Platform
GNU General Public License v3.0
61 stars 7 forks source link

Fix `SettingsState` serialization issues #10

Closed heyuch closed 1 year ago

heyuch commented 1 year ago

Hi,

In this PR, I've added a custom com.intellij.util.xmlb.Converter to deal with the java.awt.Color's serialization issues:

The java.awt.Color#value property is package level, which can't be involved during serialization. As show in the persistent setting file OverEngineer_InlineProblems.xml:

<option name="errorBackgroundColor">
      <Color />
</option>

This leads the custom error color settings can not be persisted.

Besides, the java.awt.Color does not have a no-argument constructor, deserialize with the above persisted settings during plugin startup causes an exception, which is reported in #7.

With this PR, the java.awt.Color would serialized into string, and the persisted setting file OverEngineer_InlineProblems.xml would looks like:

<application>
  <component name="org.overengineer.inlineproblems.settings.SettingsState">
    <option name="errorBackgroundColor" value="#1e4164" />
  </component>
</application>
0verEngineer commented 1 year ago

Hey @heyuch, thanks a lot for your work.

If i install the Plugin with your fix in IntelliJ Ultimate 2021.3 and open the settings i get following error (does not happen with the runPlugin task): java.lang.AssertionError at com.intellij.util.xmlb.OptionTagBinding.deserialize(OptionTagBinding.java:107) at com.intellij.util.xmlb.BasePrimitiveBinding.deserializeUnsafe(BasePrimitiveBinding.java:55) at com.intellij.util.xmlb.BeanBinding.deserializeInto(BeanBinding.java:246) at com.intellij.util.xmlb.BeanBinding.deserializeInto(BeanBinding.java:200) at com.intellij.util.xmlb.BeanBinding.deserialize(BeanBinding.java:143) at com.intellij.configurationStore.JdomSerializerImpl.deserialize(xmlSerializer.kt:91) at com.intellij.configurationStore.DefaultStateSerializerKt.deserializeState(DefaultStateSerializer.kt:29) at com.intellij.configurationStore.StateStorageBase.deserializeState(StateStorageBase.kt:35) at com.intellij.configurationStore.StateGetterImpl.getState(StorageBaseEx.kt:57) at com.intellij.configurationStore.ComponentStoreImpl.doInitComponent(ComponentStoreImpl.kt:421) at com.intellij.configurationStore.ComponentStoreImpl.initComponent(ComponentStoreImpl.kt:371) at com.intellij.configurationStore.ComponentStoreImpl.initComponent(ComponentStoreImpl.kt:122) at com.intellij.configurationStore.ComponentStoreWithExtraComponents.initComponent(ComponentStoreWithExtraComponents.kt:48) at com.intellij.serviceContainer.ComponentManagerImpl.initializeComponent$intellij_platform_serviceContainer(ComponentManagerImpl.kt:521) at com.intellij.serviceContainer.ServiceComponentAdapter.createAndInitialize(ServiceComponentAdapter.kt:51) at com.intellij.serviceContainer.ServiceComponentAdapter.doCreateInstance(ServiceComponentAdapter.kt:37) at com.intellij.serviceContainer.BaseComponentAdapter.getInstanceUncached(BaseComponentAdapter.kt:113) at com.intellij.serviceContainer.BaseComponentAdapter.getInstance(BaseComponentAdapter.kt:67) at com.intellij.serviceContainer.BaseComponentAdapter.getInstance$default(BaseComponentAdapter.kt:60) at com.intellij.serviceContainer.ComponentManagerImpl.doGetService(ComponentManagerImpl.kt:595) at com.intellij.serviceContainer.ComponentManagerImpl.getService(ComponentManagerImpl.kt:569) at com.intellij.openapi.client.ClientAwareComponentManager.getFromSelfOrCurrentSession(ClientAwareComponentManager.kt:37) at com.intellij.openapi.client.ClientAwareComponentManager.getService(ClientAwareComponentManager.kt:22) at org.overengineer.inlineproblems.settings.SettingsState.getInstance(SettingsState.java:80) at org.overengineer.inlineproblems.settings.SettingsComponent.<init>(SettingsComponent.java:47) at org.overengineer.inlineproblems.settings.SettingsConfigurable.createComponent(SettingsConfigurable.java:35) at com.intellij.openapi.options.ex.ConfigurableWrapper.createComponent(ConfigurableWrapper.java:172) at com.intellij.openapi.options.ex.ConfigurableCardPanel.lambda$createConfigurableComponent$4(ConfigurableCardPanel.java:116) at com.intellij.openapi.application.impl.ApplicationImpl.runReadAction(ApplicationImpl.java:865) at com.intellij.openapi.application.ReadAction.compute(ReadAction.java:61) at com.intellij.openapi.options.ex.ConfigurableCardPanel.createConfigurableComponent(ConfigurableCardPanel.java:112) at com.intellij.openapi.options.ex.ConfigurableCardPanel.create(ConfigurableCardPanel.java:60) at com.intellij.openapi.options.newEditor.ConfigurableEditor$1.create(ConfigurableEditor.java:57) at com.intellij.openapi.options.newEditor.ConfigurableEditor$1.create(ConfigurableEditor.java:54) at com.intellij.ui.CardLayoutPanel.createValue(CardLayoutPanel.java:73) at com.intellij.ui.CardLayoutPanel.select(CardLayoutPanel.java:101) at com.intellij.ui.CardLayoutPanel.lambda$selectLater$0(CardLayoutPanel.java:117) at com.intellij.openapi.application.TransactionGuardImpl.runWithWritingAllowed(TransactionGuardImpl.java:214) at com.intellij.openapi.application.TransactionGuardImpl.access$200(TransactionGuardImpl.java:21) at com.intellij.openapi.application.TransactionGuardImpl$2.run(TransactionGuardImpl.java:196) at com.intellij.openapi.application.impl.ApplicationImpl.runIntendedWriteActionOnCurrentThread(ApplicationImpl.java:805) at com.intellij.openapi.application.impl.ApplicationImpl.lambda$invokeLater$4(ApplicationImpl.java:348) at com.intellij.openapi.application.impl.FlushQueue.doRun(FlushQueue.java:82) at com.intellij.openapi.application.impl.FlushQueue.runNextEvent(FlushQueue.java:131) at com.intellij.openapi.application.impl.FlushQueue.flushNow(FlushQueue.java:47) at com.intellij.openapi.application.impl.FlushQueue$FlushNow.run(FlushQueue.java:187) at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:313) at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:776) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:727) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85) at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:746) at com.intellij.ide.IdeEventQueue.defaultDispatchEvent(IdeEventQueue.java:891) at com.intellij.ide.IdeEventQueue._dispatchEvent(IdeEventQueue.java:760) at com.intellij.ide.IdeEventQueue.lambda$dispatchEvent$6(IdeEventQueue.java:447) at com.intellij.openapi.progress.impl.CoreProgressManager.computePrioritized(CoreProgressManager.java:818) at com.intellij.ide.IdeEventQueue.lambda$dispatchEvent$7(IdeEventQueue.java:446) at com.intellij.openapi.application.impl.ApplicationImpl.runIntendedWriteActionOnCurrentThread(ApplicationImpl.java:805) at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.java:492) at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203) at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124) at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:117) at java.desktop/java.awt.WaitDispatchSupport$2.run(WaitDispatchSupport.java:190) at java.desktop/java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:235) at java.desktop/java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:233) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.desktop/java.awt.WaitDispatchSupport.enter(WaitDispatchSupport.java:233) at java.desktop/java.awt.Dialog.show(Dialog.java:1070) at com.intellij.openapi.ui.impl.DialogWrapperPeerImpl$MyDialog.show(DialogWrapperPeerImpl.java:701) at com.intellij.openapi.ui.impl.DialogWrapperPeerImpl.show(DialogWrapperPeerImpl.java:437) at com.intellij.openapi.ui.DialogWrapper.doShow(DialogWrapper.java:1671) at com.intellij.openapi.ui.DialogWrapper.show(DialogWrapper.java:1629) at com.intellij.ide.actions.ShowSettingsUtilImpl.showSettingsDialog(ShowSettingsUtilImpl.java:90) at com.intellij.ide.actions.ShowSettingsAction.perform(ShowSettingsAction.java:50) at com.intellij.ide.actions.ShowSettingsAction.actionPerformed(ShowSettingsAction.java:37) at com.intellij.openapi.keymap.impl.ActionProcessor.performAction(ActionProcessor.java:65) at com.intellij.openapi.keymap.impl.IdeKeyEventDispatcher$1.performAction(IdeKeyEventDispatcher.java:573) at com.intellij.openapi.keymap.impl.IdeKeyEventDispatcher.lambda$doPerformActionInner$10(IdeKeyEventDispatcher.java:706) at com.intellij.openapi.actionSystem.ex.ActionUtil.performDumbAwareWithCallbacks(ActionUtil.java:260) at com.intellij.openapi.keymap.impl.IdeKeyEventDispatcher.doPerformActionInner(IdeKeyEventDispatcher.java:702) at com.intellij.openapi.keymap.impl.IdeKeyEventDispatcher.processAction(IdeKeyEventDispatcher.java:645) at com.intellij.openapi.keymap.impl.IdeKeyEventDispatcher.processAction(IdeKeyEventDispatcher.java:584) at com.intellij.openapi.keymap.impl.IdeKeyEventDispatcher.processActionOrWaitSecondStroke(IdeKeyEventDispatcher.java:467) at com.intellij.openapi.keymap.impl.IdeKeyEventDispatcher.inInitState(IdeKeyEventDispatcher.java:456) at com.intellij.openapi.keymap.impl.IdeKeyEventDispatcher.dispatchKeyEvent(IdeKeyEventDispatcher.java:224) at com.intellij.ide.IdeEventQueue.dispatchKeyEvent(IdeEventQueue.java:804) at com.intellij.ide.IdeEventQueue._dispatchEvent(IdeEventQueue.java:754) at com.intellij.ide.IdeEventQueue.lambda$dispatchEvent$6(IdeEventQueue.java:447) at com.intellij.openapi.progress.impl.CoreProgressManager.computePrioritized(CoreProgressManager.java:818) at com.intellij.ide.IdeEventQueue.lambda$dispatchEvent$7(IdeEventQueue.java:446) at com.intellij.openapi.application.impl.ApplicationImpl.runIntendedWriteActionOnCurrentThread(ApplicationImpl.java:805) at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.java:498) at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203) at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124) at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

heyuch commented 1 year ago

Hey @heyuch, thanks a lot for your work.

If i install the Plugin with your fix in IntelliJ Ultimate 2021.3 and open the settings i get following error (does not happen with the runPlugin task): `java.lang.AssertionError at com.intellij.util.xmlb.OptionTagBinding.deserialize(OptionTagBinding.java:107)

Hi @0verEngineer, this problem is caused by the @OptionTag annotated SettingState class is incompatible with the existing setting file during unserialize:

By the newly added @OptionTag annotation and the custom Color Converter, the Color property serialization would from:

<option name="errorBackgroundColor">
      <Color />
</option>

to this:

<option name="errorBackgroundColor" value="#1e4164" />

When starting IDE with the old format xml setting file, the <Color /> tag could not be resolved, which caused the AssertionError exception.

I'll trying to solve this issue.

0verEngineer commented 1 year ago

Hi @heyuch, I have already suspected that.

I can only think of three possible solutions that do not require code to be run everytime at plugin start / at settings deserialization.

  1. We could simply rename the color variables for example like that: errorBackgroundColor -> errorBackgroundCol
  2. We could somehow delete the settings file but only once on the update from a version lower or equal to 0.2.1 to 0.2.2 or higher
  3. Parse and modify the xml file to the default values on the update

If you don't find a better solution then i think we should simply rename the variables.

heyuch commented 1 year ago

@0verEngineer I've tried this migrating-persisted-values to implement a ProjectConverter to convert the xml file during IDE startup if it's the old format. But I'm stuck in getting the settings file directory via the plugin-sdk.

If you don't find a better solution then i think we should simply rename the variables.

I agree with your solution, just do it.

heyuch commented 1 year ago

We could somehow delete the settings file but only once on the update from a version lower or equal to 0.2.1 to 0.2.2 or higher

@0verEngineer what would you think just change the @State's name to achieve this.

from now:

@State(
        name = "org.overengineer.inlineproblems.settings.SettingsState",
        storages = @Storage(SettingsState.STORAGE_FILE)
)

to something like this:

@State(
        name = "org.overengineer.inlineproblems.settings.SettingsState.v2",
        storages = @Storage(SettingsState.STORAGE_FILE)
)
0verEngineer commented 1 year ago

@heyuch I did not know about the migration thing, it does also sound very handy for future changes. Have you tried getting the settings directory via the idea.config.path Prop like mentioned here

If it is not working then we rename the variables, but if we rename the name of the whole @state then all settings are reset to default, i think we should avoid this.

heyuch commented 1 year ago

If it is not working then we rename the variables

@0verEngineer All right, let's deal with it in the simplest way. There should be no need for me to do anything?

0verEngineer commented 1 year ago

@heyuch You could rename the variables so i do not have to check the branch out later today and can merge it directly. errorBackgroundColor to errorBackgroundCol

heyuch commented 1 year ago

@heyuch You could rename the variables so i do not have to check the branch out later today and can merge it directly. errorBackgroundColor to errorBackgroundCol

@0verEngineer done.

0verEngineer commented 1 year ago

fixes #7