intellij-community icon indicating copy to clipboard operation
intellij-community copied to clipboard

IDEA-218688 Refactor UpdateChecker for flexibility

Open Kyzderp opened this issue 6 years ago • 3 comments

  • Separate the patch downloading logic into PatchDownloadTask, which UpdateInfoDialog calls
  • Add a "callback container" so callers to UpdateChecker.updateAndShowResult can do as they choose with the callback's fields

Previously, a call into UpdateChecker.updateAndShowResult would check for updates to the platform, check for updates to plugins, and then show the UpdateInfoDialog prompting the user to accept or decline the update.

With this change, the caller to updateAndShowResult passes in its own UpdateResultCallbackProvider, which creates a custom UpdateResultCallback with the results of platform/plugin updates. Then, the UpdateResultCallback is run inside showUpdateResult. In the regular flow, this callback shows UpdateInfoDialog, and clicking on Update would start the PatchDownloadTask. In custom usages, the UpdateInfoDialog could be skipped and directly start PatchDownloadTask.

Kyzderp avatar Jul 19 '19 21:07 Kyzderp

https://youtrack.jetbrains.com/issue/IDEA-218688

trespasserw avatar Jul 26 '19 17:07 trespasserw

If we do need to extend the API, why do we need two separate entities (UpdateResultCallbackProvider and UpdateResultCallback)? Can't we simply define an UpdateResultConsumer that is called directly?

public interface UpdateResultConsumer {
  void showUpdateResult(@NotNull CheckForUpdateResult checkForUpdateResult,
                                      @Nullable Collection<PluginDownloader> updatedPlugins,
                                      @Nullable Collection<IdeaPluginDescriptor> incompatiblePlugins,
                                      @NotNull UpdateSettings updateSettings,
                                      boolean enableLink);
}

  @JvmStatic
  fun updateAndShowResult(project: Project?, customSettings: UpdateSettings?, resultConsumer: UpdateResultConsumer) { ... }

yole avatar Oct 09 '20 10:10 yole

If we do need to extend the API, why do we need two separate entities (UpdateResultCallbackProvider and UpdateResultCallback)? Can't we simply define an UpdateResultConsumer that is called directly?

public interface UpdateResultConsumer {
  void showUpdateResult(@NotNull CheckForUpdateResult checkForUpdateResult,
                                      @Nullable Collection<PluginDownloader> updatedPlugins,
                                      @Nullable Collection<IdeaPluginDescriptor> incompatiblePlugins,
                                      @NotNull UpdateSettings updateSettings,
                                      boolean enableLink);
}

  @JvmStatic
  fun updateAndShowResult(project: Project?, customSettings: UpdateSettings?, resultConsumer: UpdateResultConsumer) { ... }

My original intention was for it to be like a Runnable, with the fields initialized beforehand so that run() could be called at any time. But that also meant requiring the fields to be initialized before calling run(), so I added the Provider to ensure that happens. It should be possible to reduce it to the Consumer as you said, though it may require passing some additional parameters into showUpdateResult() from doUpdateAndShowResult(). If that sounds fine, I will try it out.

Kyzderp avatar Oct 15 '20 01:10 Kyzderp

I assume this is no longer relevant; please let me know if it is.

yole avatar Dec 29 '22 13:12 yole