lymerin opened a new pull request, #7446:
URL: https://github.com/apache/shenyu/pull/7446

   Fixes #6525
   
   ### Cause
   
   ZooKeeper registration and discovery used inconsistent paths. The client 
writes instances under `/shenyu/register/instance/{appName}`, but 
`serviceExists` and `watchInstances` used the supplied key directly, while 
`selectInstances` prepended either `watchPath` or the instance root.
   
   The client also sent its configured `registerPath` to Admin as the listener 
node. For example, `/shenyu/discovery/http_example` did not identify the 
directory containing the registered instances. This could reject selector setup 
or leave discovery watching a different subtree from the one receiving 
registrations.
   
   Correcting the watch path also exposes downstream processing problems:
   
   - Native instance JSON contains `host` and `port`, but no `url`. Forwarding 
it unchanged leaves Admin without the required `upstream_url`.
   - Native events contain neither a record ID nor an update timestamp. The 
UPDATE statement used `#{id}` to match `discovery_handler_id`, so these updates 
did not match an existing row. After correcting that parameter, writing a null 
timestamp would violate the `date_updated` NOT NULL constraint.
   
   ### Changes
   
   - Add one ZooKeeper path resolver for instance lookup, existence checks, and 
watches. Bare service names resolve to 
`/shenyu/register/instance/{serviceName}`; absolute paths are used directly. 
Missing services return `false`.
   - After ZooKeeper registration, send the actual instance parent path to 
Admin through the existing discovery configuration listener.
   - Remove the ZooKeeper `watchPath` field and its property lookup, and stop 
injecting that property for ZooKeeper clients. Renew child watches using the 
resolved path when the event path is null.
   - Complete `url` in native instance events from `host:port`, preserving the 
existing `address` fallback. Instance data stored in ZooKeeper is unchanged.
   - Key Admin listener lookup by both discovery ID and watched service key, 
preventing applications sharing a discovery configuration from attaching to 
another application’s listener.
   - Bind `discoveryHandlerId` in the upstream UPDATE condition and populate 
`dateUpdated` before execution, allowing native weight and status changes to be 
persisted.
   - Publish the current database upstream list after each processed change 
instead of only the changed instance. Preserve each upstream’s `namespaceId` 
during conversion.
   - Add `shenyu-client-core` to Admin with **test scope only** for the 
integration test; it does not enter Admin’s production classpath.
   - Update the HTTP example to include both `enable` and `register`, provide 
`props.name`, and remove the obsolete discovery path.
   
   ### Compatibility notes
   
   ZooKeeper **no longer reads the `watchPath` property at all**. Configured 
values are ignored. Configure the complete absolute instance parent path as the 
listener node instead. Other registry types retain their existing `watchPath` 
handling.
   
   Existing bindings using `/shenyu/discovery/...` must be updated to the 
actual instance directory. This PR does not automatically migrate existing 
database configuration.
   
   URL completion applies only beneath `/shenyu/register/instance/`. Event 
payloads outside that subtree retain their existing format.
   
   ### Validation
   
   Targeted module tests and reactor Checkstyle checks passed using the Maven 
wrapper:
   
   - `ZookeeperInstanceRegisterRepositoryTest`: **4 tests**, including real 
ZooKeeper verification using serialized `InstanceEntity` data for lookup and 
ADDED/UPDATED/DELETED events.
   - `DiscoveryDataChangedEventSyncListenerTest`: **3 existing tests**, 
confirming the event-processing changes remain compatible with the existing 
tests.
   - `ZookeeperClientDiscoveryIntegrationTest`: **1 test**, using the real 
client HTTP sender, Admin, ZooKeeper, and WebSocket synchronization to verify 
registration, application isolation, weight updates in the database, deletion 
snapshots, and namespace preservation.
   
   All **8 tests** passed. 
   
   Make sure that:
   
   - [x] You have read the [[contribution 
guidelines](https://shenyu.apache.org/community/contributor-guide)](https://shenyu.apache.org/community/contributor-guide).
   - [x] You submit test cases (unit or integration tests) that back your 
changes.
   - [x] Your local test passed `./mvnw clean install 
-Dmaven.javadoc.skip=true`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to