Skip to content

[FEAT] delete synthesizer logic#3360

Open
Alanxtl wants to merge 2 commits into
apache:developfrom
Alanxtl:delete_synthesizer
Open

[FEAT] delete synthesizer logic#3360
Alanxtl wants to merge 2 commits into
apache:developfrom
Alanxtl:delete_synthesizer

Conversation

@Alanxtl
Copy link
Copy Markdown
Contributor

@Alanxtl Alanxtl commented Jun 1, 2026

Description

这个pr删了synthesizer相关的逻辑

  1. synthesizer相关的逻辑根本就没有被使用
  2. synthesizer相关的逻辑只有rest协议实现了

它最早是为应用级服务发现提供 fallback:当 consumer 通过应用实例找不到 provider exported URL metadata 时,对 REST 这类可以从实例地址推导 URL 的协议,临时合成接口级 provider URL 继续通知下游。
但现在看,它已经不符合当前架构了。当前架构的权威来源是 MetadataInfo.ServiceInfo -> instance.ToURLs()。

现在真正生效的是:

service instance -> metadata service -> MetadataInfo.Services -> instance.ToURLs(serviceInfo)

也就是用 provider 暴露出来的 ServiceInfo 重建 URL。

而 synthesizer 这边目前主要是 REST 有一个实现:根据订阅 URL 和实例地址拼:

rest://host:port/interface

但它拿不到 provider 真实导出的很多参数。更关键的是,当前代码里没有被服务发现主流程调用。

所以可以这么定性:

正常路径不该用 synthesizer。
有 metadata 时,metadata 才是权威来源。

synthesizer 最多是 fallback。
比如老 provider 没 metadata、metadata 拉取失败,但协议又能安全地从实例地址推导 URL,这时可以临时合一个。

REST synthesizer 特别鸡肋。
REST 调用真正关键的是 path/query/header/body 映射,而这些 synthesizer 推不出来。它最多能补地址,不能补完整语义。

Checklist

  • I confirm the target branch is develop
  • Code has passed local testing
  • I have added tests that prove my fix is effective or that my feature works

@Alanxtl Alanxtl changed the title delete synthesizer logic [FEAT] delete synthesizer logic Jun 1, 2026
@Alanxtl Alanxtl added ✏️ Feature 3.3.2 version 3.3.2 labels Jun 1, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 1, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 52.53%. Comparing base (60d1c2a) to head (afe1cde).
⚠️ Report is 814 commits behind head on develop.

Files with missing lines Patch % Lines
cluster/router/chain/chain.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3360      +/-   ##
===========================================
+ Coverage    46.76%   52.53%   +5.76%     
===========================================
  Files          295      490     +195     
  Lines        17172    37844   +20672     
===========================================
+ Hits          8031    19880   +11849     
- Misses        8287    16358    +8071     
- Partials       854     1606     +752     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the (currently unused) Service Discovery “subscribed URL synthesizer” mechanism—including the REST synthesizer implementation—and does a small cleanup around REST config reader registration and a router-chain error message.

Changes:

  • Deleted the SubscribedURLsSynthesizer interface, its factory/registry, and the REST synthesizer implementation + its unit test.
  • Switched REST config reader registration to use constant.RESTProtocol instead of a local "rest" constant.
  • Minor tweak to a router-chain error message punctuation (but there’s still a typo to fix: “exits” → “exists”).

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
registry/servicediscovery/synthesizer/subscribed_urls_synthesizer.go Removes the synthesizer interface (feature deletion).
registry/servicediscovery/synthesizer/subscribed_urls_synthesizer_factory.go Removes synthesizer registration factory (feature deletion).
registry/servicediscovery/synthesizer/rest/rest_subscribed_urls_synthesizer.go Removes REST-specific synthesizer implementation (feature deletion).
registry/servicediscovery/synthesizer/rest/rest_subscribed_urls_synthesizer_test.go Removes tests for the deleted REST synthesizer.
protocol/rest/config/reader/rest_config_reader.go Uses shared constant.RESTProtocol for config reader registration.
cluster/router/chain/chain.go Adjusts error message text (needs a small typo fix).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cluster/router/chain/chain.go Outdated
Comment thread protocol/rest/config/reader/rest_config_reader.go
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 2, 2026

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants