refactor: optimize plugin status updates in plugin reconciler (#4403)

#### What type of PR is this?
/kind improvement
/area core
/milestone 2.9.x

#### What this PR does / why we need it:
优化插件 Reconciler 中对 status 的更新

how to test it?
测试插件启动和停止没有问题即可，着重看一下 status 中是否会存在 stylesheet 和 entry 期望有值但却没有值的情况是否会发生

#### Does this PR introduce a user-facing change?
```release-note
None
```

application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java

@@ -22,6 +22,7 @@ import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.function.UnaryOperator;
 import lombok.AllArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.BooleanUtils;
@@ -139,34 +140,36 @@ public class PluginReconciler implements Reconciler<Request> {
                 }
                 createInitialReverseProxyIfNotPresent(plugin);
 
-                // filled logo path
-                generateAccessibleLogoUrl(plugin);
+                updateStatus(name, status -> {
+                    String logoUrl = generateAccessibleLogoUrl(plugin);
+                    status.setLogo(logoUrl);
 
-                // update phase
-                Plugin.PluginStatus status = plugin.statusNonNull();
-                PluginWrapper pluginWrapper = getPluginWrapper(name);
-                status.setPhase(pluginWrapper.getPluginState());
-                updateStatus(plugin.getMetadata().getName(), status);
+                    // Synchronize to plugin state in manager based on the phase of database
+                    // to avoid the plugin state in manager is inconsistent with the database
+                    // It is possible that the in-memory plugin has successfully started,
+                    // but the status update of the database has failed.
+                    // The status in the database will prevail
+                    getPluginWrapper(name).setPluginState(status.getPhase());
+                    return status;
+                });
                 return false;
             })
             .orElse(false);
     }
 
-    void generateAccessibleLogoUrl(Plugin plugin) {
+    String generateAccessibleLogoUrl(Plugin plugin) {
         String logo = plugin.getSpec().getLogo();
         if (StringUtils.isBlank(logo)) {
-            return;
+            return null;
         }
-        Plugin.PluginStatus status = plugin.statusNonNull();
-        if (PathUtils.isAbsoluteUri(logo)) {
-            status.setLogo(logo);
-        } else {
+        if (!PathUtils.isAbsoluteUri(logo)) {
             String assetsPrefix =
                 PluginConst.assertsRoutePrefix(plugin.getMetadata().getName());
             String versionedLogo =
                 applyVersioningToStaticResource(logo, plugin.getSpec().getVersion());
-            status.setLogo(PathUtils.combinePath(assetsPrefix, versionedLogo));
+            return PathUtils.combinePath(assetsPrefix, versionedLogo);
         }
+        return logo;
     }
 
     Optional<Setting> lookupPluginSetting(String name, String settingName) {
@@ -231,18 +234,19 @@ public class PluginReconciler implements Reconciler<Request> {
         // Fix gh-3224
         // Maybe Setting is being created and cannot be queried. so try again.
         if (settingOption.isEmpty()) {
-            Plugin.PluginStatus status = plugin.statusNonNull();
-            status.setPhase(PluginState.FAILED);
-            var condition = Condition.builder()
-                .type("BackOff")
-                .reason("BackOff")
-                .message("Wait for setting [" + settingName + "] creation")
-                .status(ConditionStatus.FALSE)
-                .lastTransitionTime(Instant.now())
-                .build();
-            Plugin.PluginStatus.nullSafeConditions(status)
-                .addAndEvictFIFO(condition);
-            updateStatus(plugin.getMetadata().getName(), status);
+            updateStatus(plugin.getMetadata().getName(), status -> {
+                status.setPhase(PluginState.FAILED);
+                var condition = Condition.builder()
+                    .type("BackOff")
+                    .reason("BackOff")
+                    .message("Wait for setting [" + settingName + "] creation")
+                    .status(ConditionStatus.FALSE)
+                    .lastTransitionTime(Instant.now())
+                    .build();
+                Plugin.PluginStatus.nullSafeConditions(status)
+                    .addAndEvictFIFO(condition);
+                return status;
+            });
             // need requeue
             return true;
         }
@@ -316,9 +320,7 @@ public class PluginReconciler implements Reconciler<Request> {
     }
 
     void persistenceFailureStatus(String pluginName, Throwable e) {
-        client.fetch(Plugin.class, pluginName).ifPresent(plugin -> {
-            Plugin.PluginStatus status = plugin.statusNonNull();
-
+        updateStatus(pluginName, status -> {
             PluginWrapper pluginWrapper = haloPluginManager.getPlugin(pluginName);
             if (pluginWrapper != null) {
                 pluginWrapper.setPluginState(PluginState.FAILED);
@@ -327,7 +329,6 @@ public class PluginReconciler implements Reconciler<Request> {
 
             status.setPhase(PluginState.FAILED);
 
-            Plugin.PluginStatus oldStatus = JsonUtils.deepCopy(status);
             Condition condition = Condition.builder()
                 .type(PluginState.FAILED.toString())
                 .reason("UnexpectedState")
@@ -337,9 +338,7 @@ public class PluginReconciler implements Reconciler<Request> {
                 .build();
             Plugin.PluginStatus.nullSafeConditions(status)
                 .addAndEvictFIFO(condition);
-            if (!Objects.equals(oldStatus, status)) {
-                client.update(plugin);
-            }
+            return status;
         });
     }
 
@@ -352,33 +351,35 @@ public class PluginReconciler implements Reconciler<Request> {
         }
 
         if (pluginWrapper == null) {
-            Plugin.PluginStatus status = new Plugin.PluginStatus();
-            status.setPhase(PluginState.FAILED);
-
             String errorMsg = "Plugin " + name + " not found in plugin manager.";
-            Condition condition = Condition.builder()
-                .type(PluginState.FAILED.toString())
-                .reason("PluginNotFound")
-                .message(errorMsg)
-                .status(ConditionStatus.FALSE)
-                .lastTransitionTime(Instant.now())
-                .build();
-            Plugin.PluginStatus.nullSafeConditions(status)
-                .addAndEvictFIFO(condition);
-            updateStatus(name, status);
+            updateStatus(name, status -> {
+                status.setPhase(PluginState.FAILED);
+
+                Condition condition = Condition.builder()
+                    .type(PluginState.FAILED.toString())
+                    .reason("PluginNotFound")
+                    .message(errorMsg)
+                    .status(ConditionStatus.FALSE)
+                    .lastTransitionTime(Instant.now())
+                    .build();
+                Plugin.PluginStatus.nullSafeConditions(status)
+                    .addAndEvictFIFO(condition);
+                return status;
+            });
             throw new DoNotRetryException(errorMsg);
         }
         return pluginWrapper;
     }
 
-    void updateStatus(String name, Plugin.PluginStatus status) {
-        if (status == null) {
-            return;
-        }
+    void updateStatus(String name, UnaryOperator<Plugin.PluginStatus> operator) {
         client.fetch(Plugin.class, name).ifPresent(plugin -> {
             Plugin.PluginStatus oldStatus = JsonUtils.deepCopy(plugin.statusNonNull());
-            plugin.setStatus(status);
-            URI loadLocation = status.getLoadLocation();
+            Plugin.PluginStatus newStatus =
+                Optional.ofNullable(operator.apply(plugin.statusNonNull()))
+                    .orElse(new Plugin.PluginStatus());
+            plugin.setStatus(newStatus);
+
+            URI loadLocation = newStatus.getLoadLocation();
             if (loadLocation == null) {
                 String pluginPath = nullSafeAnnotations(plugin).get(PLUGIN_PATH);
                 if (StringUtils.isNotBlank(pluginPath)) {
@@ -387,9 +388,9 @@ public class PluginReconciler implements Reconciler<Request> {
                 } else {
                     loadLocation = getPluginWrapper(name).getPluginPath().toUri();
                 }
-                status.setLoadLocation(loadLocation);
+                newStatus.setLoadLocation(loadLocation);
             }
-            if (!Objects.equals(oldStatus, status)) {
+            if (!Objects.equals(oldStatus, newStatus)) {
                 client.update(plugin);
             }
         });
@@ -411,20 +412,19 @@ public class PluginReconciler implements Reconciler<Request> {
                 "The plugin is disabled for some reason and cannot be started.");
         }
 
-        client.fetch(Plugin.class, name).ifPresent(plugin -> {
-            final Plugin.PluginStatus status = plugin.statusNonNull();
-            final Plugin.PluginStatus oldStatus = JsonUtils.deepCopy(status);
-
+        String pluginVersion = pluginWrapper.getDescriptor().getVersion();
+        updateStatus(name, status -> {
             PluginState currentState = haloPluginManager.startPlugin(name);
             if (!PluginState.STARTED.equals(currentState)) {
                 PluginStartingError staringErrorInfo = getStaringErrorInfo(name);
-                log.debug("Failed to start plugin: " + staringErrorInfo.getDevMessage());
-                throw new IllegalStateException(staringErrorInfo.getMessage());
+                log.debug("Failed to start plugin: " + staringErrorInfo.getDevMessage(),
+                    pluginWrapper.getFailedException());
+                throw new IllegalStateException(staringErrorInfo.getMessage(),
+                    pluginWrapper.getFailedException());
             }
 
-            plugin.statusNonNull().setLastStartTime(Instant.now());
+            status.setLastStartTime(Instant.now());
 
-            final String pluginVersion = plugin.getSpec().getVersion();
             String jsBundlePath =
                 BundleResourceUtils.getJsBundlePath(haloPluginManager, name);
             jsBundlePath = applyVersioningToStaticResource(jsBundlePath, pluginVersion);
@@ -445,9 +445,7 @@ public class PluginReconciler implements Reconciler<Request> {
                 .build();
             Plugin.PluginStatus.nullSafeConditions(status)
                 .addAndEvictFIFO(condition);
-            if (!Objects.equals(oldStatus, status)) {
-                client.update(plugin);
-            }
+            return status;
         });
     }
 
@@ -470,10 +468,7 @@ public class PluginReconciler implements Reconciler<Request> {
     }
 
     void doStop(String name) {
-        client.fetch(Plugin.class, name).ifPresent(plugin -> {
-            final Plugin.PluginStatus status = plugin.statusNonNull();
-            final Plugin.PluginStatus oldStatus = JsonUtils.deepCopy(status);
-
+        updateStatus(name, status -> {
             PluginState currentState = haloPluginManager.stopPlugin(name);
             if (!PluginState.STOPPED.equals(currentState)) {
                 throw new IllegalStateException("Failed to stop plugin: " + name);
@@ -492,9 +487,7 @@ public class PluginReconciler implements Reconciler<Request> {
                 .build();
             Plugin.PluginStatus.nullSafeConditions(status)
                 .addAndEvictFIFO(condition);
-            if (!Objects.equals(oldStatus, status)) {
-                client.update(plugin);
-            }
+            return status;
         });
     }
 
@@ -640,16 +633,24 @@ public class PluginReconciler implements Reconciler<Request> {
         return Paths.get(pathString).toUri();
     }
 
-    private boolean shouldReconcileStartState(Plugin plugin) {
+    boolean shouldReconcileStartState(Plugin plugin) {
         PluginWrapper pluginWrapper = getPluginWrapper(plugin.getMetadata().getName());
-        return BooleanUtils.isTrue(plugin.getSpec().getEnabled())
-            && !PluginState.STARTED.equals(pluginWrapper.getPluginState());
+        if (BooleanUtils.isNotTrue(plugin.getSpec().getEnabled())) {
+            return false;
+        }
+        // phase is not started or plugin state is not started should start
+        return !PluginState.STARTED.equals(plugin.statusNonNull().getPhase())
+            || !PluginState.STARTED.equals(pluginWrapper.getPluginState());
     }
 
-    private boolean shouldReconcileStopState(Plugin plugin) {
+    boolean shouldReconcileStopState(Plugin plugin) {
         PluginWrapper pluginWrapper = getPluginWrapper(plugin.getMetadata().getName());
-        return BooleanUtils.isFalse(plugin.getSpec().getEnabled())
-            && PluginState.STARTED.equals(pluginWrapper.getPluginState());
+        if (BooleanUtils.isNotFalse(plugin.getSpec().getEnabled())) {
+            return false;
+        }
+        // phase is not stopped or plugin state is not stopped should stop
+        return !PluginState.STOPPED.equals(plugin.statusNonNull().getPhase())
+            || !PluginState.STOPPED.equals(pluginWrapper.getPluginState());
     }
 
     private void addFinalizerIfNecessary(Plugin oldPlugin) {

application/src/test/java/run/halo/app/core/extension/reconciler/PluginReconcilerTest.java

@@ -37,6 +37,7 @@ import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.stubbing.Answer;
+import org.pf4j.PluginDescriptor;
 import org.pf4j.PluginState;
 import org.pf4j.PluginWrapper;
 import org.pf4j.RuntimeMode;
@@ -91,6 +92,10 @@ class PluginReconcilerTest {
     void reconcileOkWhenPluginManagerStartSuccessfully() {
         Plugin plugin = need2ReconcileForStartupState();
         when(pluginWrapper.getPluginState()).thenReturn(PluginState.STOPPED);
+        var pluginDescriptor = mock(PluginDescriptor.class);
+        when(pluginWrapper.getDescriptor()).thenReturn(pluginDescriptor);
+        when(pluginDescriptor.getVersion()).thenReturn("1.0.0");
+
         when(extensionClient.fetch(eq(Plugin.class), eq("apples"))).thenReturn(Optional.of(plugin));
         when(haloPluginManager.startPlugin(any())).thenAnswer((Answer<PluginState>) invocation -> {
             // mock plugin real state is started
@@ -119,6 +124,9 @@ class PluginReconcilerTest {
 
         // mock plugin real state is started
         when(pluginWrapper.getPluginState()).thenReturn(PluginState.STOPPED);
+        var pluginDescriptor = mock(PluginDescriptor.class);
+        when(pluginWrapper.getDescriptor()).thenReturn(pluginDescriptor);
+        when(pluginDescriptor.getVersion()).thenReturn("1.0.0");
 
         PluginStartingError pluginStartingError =
             PluginStartingError.of("apples", "error message", "dev message");
@@ -309,9 +317,8 @@ class PluginReconcilerTest {
             plugin.setSpec(new Plugin.PluginSpec());
             plugin.getSpec().setLogo("https://example.com/logo.png");
             plugin.getSpec().setVersion("1.0.0");
-            pluginReconciler.generateAccessibleLogoUrl(plugin);
-            assertThat(plugin.statusNonNull().getLogo())
-                .isEqualTo("https://example.com/logo.png");
+            String logo = pluginReconciler.generateAccessibleLogoUrl(plugin);
+            assertThat(logo).isEqualTo("https://example.com/logo.png");
         }
 
         @Test
@@ -320,8 +327,7 @@ class PluginReconcilerTest {
             plugin.setSpec(new Plugin.PluginSpec());
             plugin.getSpec().setLogo("https://example.com/logo.png?hello=world");
             plugin.getSpec().setVersion("1.0.0");
-            pluginReconciler.generateAccessibleLogoUrl(plugin);
-            assertThat(plugin.statusNonNull().getLogo())
+            assertThat(pluginReconciler.generateAccessibleLogoUrl(plugin))
                 .isEqualTo("https://example.com/logo.png?hello=world");
         }
 
@@ -331,8 +337,7 @@ class PluginReconcilerTest {
             plugin.setSpec(new Plugin.PluginSpec());
             plugin.getSpec().setLogo(null);
             plugin.getSpec().setVersion("1.0.0");
-            pluginReconciler.generateAccessibleLogoUrl(plugin);
-            assertThat(plugin.statusNonNull().getLogo()).isNull();
+            assertThat(pluginReconciler.generateAccessibleLogoUrl(plugin)).isNull();
         }
 
         @Test
@@ -341,8 +346,7 @@ class PluginReconcilerTest {
             plugin.setSpec(new Plugin.PluginSpec());
             plugin.getSpec().setLogo("");
             plugin.getSpec().setVersion("1.0.0");
-            pluginReconciler.generateAccessibleLogoUrl(plugin);
-            assertThat(plugin.statusNonNull().getLogo()).isNull();
+            assertThat(pluginReconciler.generateAccessibleLogoUrl(plugin)).isNull();
         }
 
         @Test
@@ -353,8 +357,7 @@ class PluginReconcilerTest {
             plugin.getMetadata().setName("fake-plugin");
             plugin.getSpec().setLogo("/static/logo.jpg");
             plugin.getSpec().setVersion("1.0.0");
-            pluginReconciler.generateAccessibleLogoUrl(plugin);
-            assertThat(plugin.statusNonNull().getLogo())
+            assertThat(pluginReconciler.generateAccessibleLogoUrl(plugin))
                 .isEqualTo("/plugins/fake-plugin/assets/static/logo.jpg?version=1.0.0");
         }
 
@@ -366,8 +369,7 @@ class PluginReconcilerTest {
             plugin.getMetadata().setName("fake-plugin");
             plugin.getSpec().setLogo("data:image/gif;base64,R0lGODfake");
             plugin.getSpec().setVersion("2.0.0");
-            pluginReconciler.generateAccessibleLogoUrl(plugin);
-            assertThat(plugin.statusNonNull().getLogo())
+            assertThat(pluginReconciler.generateAccessibleLogoUrl(plugin))
                 .isEqualTo("data:image/gif;base64,R0lGODfake");
         }
     }
@@ -498,10 +500,13 @@ class PluginReconcilerTest {
         String name = "fake-plugin";
         Plugin plugin = new Plugin();
         Plugin.PluginStatus status = new Plugin.PluginStatus();
+        plugin.setMetadata(new Metadata());
+        plugin.getMetadata().setName(name);
         plugin.setStatus(status);
         when(extensionClient.fetch(eq(Plugin.class), eq(name)))
             .thenReturn(Optional.of(plugin));
         PluginWrapper pluginWrapper = mock(PluginWrapper.class);
+        when(pluginWrapper.getPluginPath()).thenReturn(Paths.get("/path/to/plugin.jar"));
         when(haloPluginManager.getPlugin(eq(name)))
             .thenReturn(pluginWrapper);
         Throwable error = mock(Throwable.class);
@@ -516,6 +521,56 @@ class PluginReconcilerTest {
         verify(pluginWrapper).setFailedException(eq(error));
     }
 
+    @Test
+    void shouldReconcileStartState() {
+        Plugin plugin = new Plugin();
+        plugin.setMetadata(new Metadata());
+        plugin.getMetadata().setName("fake-plugin");
+        plugin.setSpec(new Plugin.PluginSpec());
+
+        PluginWrapper pluginWrapper = mock(PluginWrapper.class);
+        when(haloPluginManager.getPlugin(eq("fake-plugin"))).thenReturn(pluginWrapper);
+
+        plugin.getSpec().setEnabled(false);
+        assertThat(pluginReconciler.shouldReconcileStartState(plugin)).isFalse();
+
+        plugin.getSpec().setEnabled(true);
+        plugin.statusNonNull().setPhase(PluginState.RESOLVED);
+        assertThat(pluginReconciler.shouldReconcileStartState(plugin)).isTrue();
+
+        when(pluginWrapper.getPluginState()).thenReturn(PluginState.STOPPED);
+        assertThat(pluginReconciler.shouldReconcileStartState(plugin)).isTrue();
+
+        plugin.statusNonNull().setPhase(PluginState.STARTED);
+        when(pluginWrapper.getPluginState()).thenReturn(PluginState.STARTED);
+        assertThat(pluginReconciler.shouldReconcileStartState(plugin)).isFalse();
+    }
+
+    @Test
+    void shouldReconcileStopState() {
+        Plugin plugin = new Plugin();
+        plugin.setMetadata(new Metadata());
+        plugin.getMetadata().setName("fake-plugin");
+        plugin.setSpec(new Plugin.PluginSpec());
+
+        PluginWrapper pluginWrapper = mock(PluginWrapper.class);
+        when(haloPluginManager.getPlugin(eq("fake-plugin"))).thenReturn(pluginWrapper);
+
+        plugin.getSpec().setEnabled(true);
+        assertThat(pluginReconciler.shouldReconcileStopState(plugin)).isFalse();
+
+        plugin.getSpec().setEnabled(false);
+        plugin.statusNonNull().setPhase(PluginState.RESOLVED);
+        assertThat(pluginReconciler.shouldReconcileStopState(plugin)).isTrue();
+
+        when(pluginWrapper.getPluginState()).thenReturn(PluginState.STOPPED);
+        assertThat(pluginReconciler.shouldReconcileStopState(plugin)).isTrue();
+
+        plugin.statusNonNull().setPhase(PluginState.STOPPED);
+        when(pluginWrapper.getPluginState()).thenReturn(PluginState.STOPPED);
+        assertThat(pluginReconciler.shouldReconcileStopState(plugin)).isFalse();
+    }
+
     private ArgumentCaptor<Plugin> doReconcileNeedRequeue() {
         ArgumentCaptor<Plugin> pluginCaptor = ArgumentCaptor.forClass(Plugin.class);
         doNothing().when(extensionClient).update(pluginCaptor.capture());