Skip to content

[ISSUE #6619] Publish create events for imported plugins - #7189

Open
dengliming wants to merge 1 commit into
apache:masterfrom
dengliming:fix-6619-publish-plugin-created-event
Open

dengliming wants to merge 1 commit into
apache:masterfrom
dengliming:fix-6619-publish-plugin-created-event

Conversation

@dengliming

Copy link
Copy Markdown
Member

Publish the plugin-created event after a config import successfully inserts a new plugin. This keeps imported plugins on the same initialization path as plugins created through the normal API, including resource and permission setup.

The unit test now verifies that a successful import invokes PluginEventPublisher.onCreated.

Tested with:
./mvnw -q -pl shenyu-admin -am -DskipTests=false -Dcheckstyle.skip=false -Dtest=PluginServiceTest -DfailIfNoTests=false test

Fixes #6619

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the asymmetry. importData published a create event for the update path but not for the insert path, so plugins that arrived through config import were persisted without ever going through onCreated — which is what initialises the plugin's resources/permissions. The imported plugin therefore existed but was inert until something else happened to publish an event for it. Adding the publish on successful insert makes the two branches symmetric.

Verified:

  • pluginEventPublisher.onCreated(pluginDO) is called only when insertSelective returns a positive row count, matching how successCount is incremented — so a failed insert doesn't fan out a spurious event.
  • It's placed before successCount++ but after the insert, so ordering relative to the DB write is correct.
  • The test change asserts onCreated is invoked with a PluginDO, which directly pins the new behaviour.

Non-blocking note: this now means an import can trigger resource creation for many plugins in a loop (see the transaction work in #7192). If import is ever made transactional end-to-end, publishing inside the loop would fire events for rows not yet committed — worth keeping in mind, but correct as-is since importData is not transactional today.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] PluginServiceImpl.importData never publishes the create event (resource init skipped for imported plugins)

2 participants