You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by al...@apache.org on 2026/06/13 11:23:06 UTC
(dubbo-go) branch develop updated: fix(metadata): harden remote MetadataInfo loading with fallback, nil guards and error chain (#3395)
This is an automated email from the ASF dual-hosted git repository.
AlexStocks pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git
The following commit(s) were added to refs/heads/develop by this push:
new 4d93c115a fix(metadata): harden remote MetadataInfo loading with fallback, nil guards and error chain (#3395)
4d93c115a is described below
commit 4d93c115a2ac2b75d104dc6c93e40448b0955185
Author: Nene7ko_ <14...@users.noreply.github.com>
AuthorDate: Sat Jun 13 19:23:00 2026 +0800
fix(metadata): harden remote MetadataInfo loading with fallback, nil guards and error chain (#3395)
* test(metadata): update cache key format and add GetMetadataInfo tests
* fix(metadata): add fallback and nil URL guards for remote MetadataInfo loading
* fix(metadata): address review issues in fallback and nil URL guard changes
* fix(metadata): address PR review issues — error chain, context, comments, tests
- Wrap combined fallback error with perrors.Wrapf instead of Errorf so the
rpcErr chain is preserved for errors.Is/As callers (issue #1)
- Annotate the reportErr==nil fallback failure path with [Metadata-Fallback]
context instead of returning a bare RPC error (issue #2)
- Wrap local/default path RPC error with app/registry/revision context,
consistent with the remote path (issue #3)
- Mark unused ctx parameters as _ with TODO(context-propagation) comments
in both getMetadataInfo implementations and the interface (issue #4)
- Update GetMetadataInfo doc comment to accurately describe cache, fallback,
and nil guard behavior (issue #5)
- Align 'treat as local' comment and 'defaulting to RPC' log to use consistent
'local' storage-type vocabulary (issue #6)
- Clarify '// local / default storage path' comment to say RPC is used (issue #7)
- Fix listenerMockMetadataReport.GetAppMetadata to use comma-ok assertion,
preventing panic when Return(nil, err) is used in future tests (issue #8)
- Add TestGetMetadataInfo_ReportReturnsNil_RPCSucceeds covering the
report-(nil,nil) -> RPC fallback path (gap #9)
- Add TestGetMetadataInfo_RPCReturnsNilMetadata covering the nil-meta
guard in the RPC result path (gap #10)
- Add TestGetMetadataInfo_NilMetadataMap covering the nil-Metadata-map
guard at line 264, reachable only via direct call not OnEvent (gap #11)
* fix ci fail
* fix(metadata): isolate metadata cache by provider app and add port guard
Address PR review feedback:
- Scope the metadata cache key by provider application instead of the
subscribing consumer app. OnEvent keyed revisionToInstances,
revisionToMetadata and the disk cache on revision alone (or consumer
app + revision), so two provider apps sharing a revision could
overwrite each other. Key on metadataCacheKey(providerApp, registryId,
revision) throughout; pass instance.GetServiceName() to GetMetadataInfo.
- buildStandardMetadataServiceURL now returns (*common.URL, error) and
reports missing protocol and missing port at the URL layer, instead of
only guarding a nil URL in the caller.
- Align test names/comments with what they actually cover and fix error
assertions to use require (testifylint require-error).
* fix ci fail
---------
Co-authored-by: xnlemon <xi...@gmail.com>
---
metadata/client.go | 53 ++--
metadata/client_test.go | 61 ++++-
.../service_instances_changed_listener_impl.go | 98 ++++++--
...service_instances_changed_listener_impl_test.go | 274 ++++++++++++++++++++-
4 files changed, 436 insertions(+), 50 deletions(-)
diff --git a/metadata/client.go b/metadata/client.go
index e3f09aa5d..bfd23c803 100644
--- a/metadata/client.go
+++ b/metadata/client.go
@@ -47,11 +47,18 @@ func GetMetadataFromMetadataReport(revision string, instance registry.ServiceIns
if report == nil {
return nil, perrors.Errorf("no metadata report instance found for registryId=%s, please check metadata-report configuration", registryId)
}
- return report.GetAppMetadata(instance.GetServiceName(), revision)
+ meta, err := report.GetAppMetadata(instance.GetServiceName(), revision)
+ if err != nil {
+ return nil, perrors.Wrapf(err, "failed to get app metadata app=%s revision=%s", instance.GetServiceName(), revision)
+ }
+ return meta, nil
}
func GetMetadataFromRpc(revision string, instance registry.ServiceInstance) (*info.MetadataInfo, error) {
- url := buildStandardMetadataServiceURL(instance)
+ url, err := buildStandardMetadataServiceURL(instance)
+ if err != nil {
+ return nil, err
+ }
url.SetParam(constant.TimeoutKey, defaultTimeout)
p := extension.GetProtocol(url.Protocol)
invoker := p.Refer(url)
@@ -70,23 +77,27 @@ func GetMetadataFromRpc(revision string, instance registry.ServiceInstance) (*in
return remoteService.getMetadataInfo(context.Background(), revision)
}
+// remoteMetadataService is the internal interface for fetching MetadataInfo via RPC.
+// The context parameter is accepted for future cancellation support but is not yet propagated.
type remoteMetadataService interface {
- getMetadataInfo(context context.Context, revision string) (*info.MetadataInfo, error)
+ getMetadataInfo(_ context.Context, revision string) (*info.MetadataInfo, error)
}
type triMetadataServiceV2 struct {
invoker base.Invoker
}
-func (m *triMetadataServiceV2) getMetadataInfo(ctx context.Context, revision string) (*info.MetadataInfo, error) {
+// getMetadataInfo fetches metadata via RPC using the Triple protocol (Protobuf).
+// TODO(context-propagation): ctx is not yet forwarded to the invoker; cancellation is not respected.
+func (m *triMetadataServiceV2) getMetadataInfo(_ context.Context, revision string) (*info.MetadataInfo, error) {
const methodName = "GetMetadataInfo"
req := &tripleapi.MetadataRequest{Revision: revision}
metadataInfo := &tripleapi.MetadataInfoV2{}
inv, _ := generateInvocation(m.invoker.GetURL(), methodName, req, metadataInfo, constant.CallUnary)
res := m.invoker.Invoke(context.Background(), inv)
if res.Error() != nil {
- logger.Errorf("[Metadata] could not get the metadata info from remote provider, err=%v", res.Error())
- return nil, res.Error()
+ logger.Errorf("[Metadata][RPC] could not get the metadata info from remote provider, err=%v", res.Error())
+ return nil, perrors.Wrapf(res.Error(), "remote metadata call failed")
}
return convertMetadataInfoV2(metadataInfo), nil
}
@@ -148,7 +159,9 @@ type remoteMetadataServiceV1 struct {
invoker base.Invoker
}
-func (m *remoteMetadataServiceV1) getMetadataInfo(ctx context.Context, revision string) (*info.MetadataInfo, error) {
+// getMetadataInfo fetches metadata via RPC using the dubbo:// protocol (Hessian2 serialization).
+// TODO(context-propagation): ctx is not yet forwarded to the invoker; cancellation is not respected.
+func (m *remoteMetadataServiceV1) getMetadataInfo(_ context.Context, revision string) (*info.MetadataInfo, error) {
const methodName = "getMetadataInfo"
// Use interface{} as reply parameter to accept any type (MetadataInfo or string)
// This avoids panic when Java returns String instead of MetadataInfo
@@ -157,15 +170,15 @@ func (m *remoteMetadataServiceV1) getMetadataInfo(ctx context.Context, revision
res := m.invoker.Invoke(context.Background(), inv)
if res.Error() != nil {
- logger.Errorf("[Metadata] RPC call failed to %s, err=%v", m.invoker.GetURL().Location, res.Error())
- return nil, res.Error()
+ logger.Errorf("[Metadata][RPC] RPC call failed to %s, err=%v", m.invoker.GetURL().Location, res.Error())
+ return nil, perrors.Wrapf(res.Error(), "RPC call failed to %s", m.invoker.GetURL().Location)
}
// rawResult now contains the deserialized value - could be *MetadataInfo, string, or nil
// Handle nil response (e.g., Java service not fully initialized)
if rawResult == nil {
- logger.Warnf("[Metadata] Provider %s returned nil metadata (service may not be ready), revision=%s",
+ logger.Warnf("[Metadata][RPC] Provider %s returned nil metadata (service may not be ready), revision=%s",
m.invoker.GetURL().Location, revision)
return nil, perrors.Errorf("metadata is nil from %s, revision: %s", m.invoker.GetURL().Location, revision)
}
@@ -178,18 +191,18 @@ func (m *remoteMetadataServiceV1) getMetadataInfo(ctx context.Context, revision
} else if strValue, ok := rawResult.(string); ok {
// Old Java Dubbo version returns JSON string instead of MetadataInfo object
// Try to parse it as JSON for backward compatibility
- logger.Warnf("[Metadata] Provider %s returned string type (old Dubbo version), attempting JSON parse", m.invoker.GetURL().Location)
+ logger.Warnf("[Metadata][RPC] Provider %s returned string type (old Dubbo version), attempting JSON parse", m.invoker.GetURL().Location)
metadataInfo = &info.MetadataInfo{}
if err := json.Unmarshal([]byte(strValue), metadataInfo); err != nil {
- logger.Errorf("[Metadata] failed to parse JSON string from provider %s, err=%v", m.invoker.GetURL().Location, err)
- logger.Errorf("[Metadata] - String content: %s", truncateString(strValue, 1000))
+ logger.Errorf("[Metadata][RPC] failed to parse JSON string from provider %s, err=%v", m.invoker.GetURL().Location, err)
+ logger.Errorf("[Metadata][RPC] - String content: %s", truncateString(strValue, 1000))
return nil, perrors.Errorf("failed to parse metadata JSON from %s: %v", m.invoker.GetURL().Location, err)
}
} else {
// Neither MetadataInfo nor String - this is unexpected
- logger.Errorf("[Metadata] unexpected metadata type from %s: got %T, expected *info.MetadataInfo or string",
+ logger.Errorf("[Metadata][RPC] unexpected metadata type from %s: got %T, expected *info.MetadataInfo or string",
m.invoker.GetURL().Location, rawResult)
return nil, perrors.Errorf("unexpected metadata type from %s: got %T, expected *info.MetadataInfo or string",
m.invoker.GetURL().Location, rawResult)
@@ -207,10 +220,14 @@ func truncateString(s string, maxLen int) string {
}
// buildStandardMetadataServiceURL will use standard format to build the metadata service url.
-func buildStandardMetadataServiceURL(ins registry.ServiceInstance) *common.URL {
+// Returns an error if required params (protocol or port) are missing.
+func buildStandardMetadataServiceURL(ins registry.ServiceInstance) (*common.URL, error) {
ps := getMetadataServiceUrlParams(ins)
if ps[constant.ProtocolKey] == "" {
- return nil
+ return nil, perrors.New("metadata service URL params missing: protocol is empty")
+ }
+ if ps[constant.PortKey] == "" {
+ return nil, perrors.New("metadata service URL params missing: port is empty")
}
sn := ins.GetServiceName()
@@ -243,7 +260,7 @@ func buildStandardMetadataServiceURL(ins registry.ServiceInstance) *common.URL {
}
}
- return u
+ return u, nil
}
// getMetadataServiceUrlParams this will convertV2 the metadata service url parameters to map structure
@@ -255,7 +272,7 @@ func getMetadataServiceUrlParams(ins registry.ServiceInstance) map[string]string
if str, ok := ps[constant.MetadataServiceURLParamsPropertyName]; ok && len(str) > 0 {
err := json.Unmarshal([]byte(str), &res)
if err != nil {
- logger.Errorf("[Metadata] could not parse the metadata service url parameters to map, err=%v", err)
+ logger.Errorf("[Metadata][URL] could not parse the metadata service url parameters to map, err=%v", err)
}
}
diff --git a/metadata/client_test.go b/metadata/client_test.go
index aa538a918..a2c192e0f 100644
--- a/metadata/client_test.go
+++ b/metadata/client_test.go
@@ -186,14 +186,43 @@ func TestGetMetadataFromRpc(t *testing.T) {
})
}
+func TestGetMetadataFromRpc_MissingURLParams(t *testing.T) {
+ t.Run("missing protocol", func(t *testing.T) {
+ insNoProto := ®istry.DefaultServiceInstance{
+ ID: "2",
+ ServiceName: "dubbo-app",
+ Host: "dubbo.io",
+ Metadata: map[string]string{},
+ }
+ _, err := GetMetadataFromRpc("1", insNoProto)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "protocol is empty")
+ })
+
+ t.Run("missing port", func(t *testing.T) {
+ insNoPort := ®istry.DefaultServiceInstance{
+ ID: "3",
+ ServiceName: "dubbo-app",
+ Host: "dubbo.io",
+ Metadata: map[string]string{
+ constant.MetadataServiceURLParamsPropertyName: `{"protocol":"dubbo"}`,
+ },
+ }
+ _, err := GetMetadataFromRpc("1", insNoPort)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "port is empty")
+ })
+}
+
func Test_buildMetadataServiceURL(t *testing.T) {
type args struct {
ins registry.ServiceInstance
}
tests := []struct {
- name string
- args args
- want *common.URL
+ name string
+ args args
+ want *common.URL
+ wantErr string
}{
{
name: "normal",
@@ -239,12 +268,34 @@ func Test_buildMetadataServiceURL(t *testing.T) {
Metadata: map[string]string{},
},
},
- want: nil,
+ wantErr: "protocol is empty",
+ },
+ {
+ name: "no port",
+ args: args{
+ ®istry.DefaultServiceInstance{
+ ServiceName: "dubbo-app",
+ Host: "dubbo.io",
+ Metadata: map[string]string{
+ constant.MetadataServiceURLParamsPropertyName: `{
+ "protocol":"dubbo"
+ }`,
+ },
+ },
+ },
+ wantErr: "port is empty",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- assert.Equalf(t, tt.want, buildStandardMetadataServiceURL(tt.args.ins), "buildMetadataServiceURL(%v)", tt.args.ins)
+ got, err := buildStandardMetadataServiceURL(tt.args.ins)
+ if tt.wantErr != "" {
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), tt.wantErr)
+ return
+ }
+ require.NoError(t, err)
+ assert.Equalf(t, tt.want, got, "buildMetadataServiceURL(%v)", tt.args.ins)
})
}
}
diff --git a/registry/servicediscovery/service_instances_changed_listener_impl.go b/registry/servicediscovery/service_instances_changed_listener_impl.go
index af18900b2..317cff056 100644
--- a/registry/servicediscovery/service_instances_changed_listener_impl.go
+++ b/registry/servicediscovery/service_instances_changed_listener_impl.go
@@ -28,6 +28,8 @@ import (
gxset "github.com/dubbogo/gost/container/set"
"github.com/dubbogo/gost/gof/observer"
"github.com/dubbogo/gost/log/logger"
+
+ perrors "github.com/pkg/errors"
)
import (
@@ -113,14 +115,21 @@ func (lstn *ServiceInstancesChangedListenerImpl) OnEvent(e observer.Event) error
logger.Infof("[Registry][ServiceDiscovery] find instance without valid service metadata, host=%s", instance.GetHost())
continue
}
- subInstances := revisionToInstances[revision]
+ // MetadataInfo belongs to the provider application, so isolate every cache
+ // dimension by provider app. Two provider apps that happen to share a revision
+ // (e.g. same interface set exported under different application names) must not
+ // collide on a revision-only key. instance.GetServiceName() is the provider app.
+ providerApp := instance.GetServiceName()
+ key := metadataCacheKey(providerApp, lstn.registryId, revision)
+
+ subInstances := revisionToInstances[key]
if subInstances == nil {
subInstances = make([]registry.ServiceInstance, 0, 8)
}
- revisionToInstances[revision] = append(subInstances, instance)
- metadataInfo := lstn.revisionToMetadata[revision]
+ revisionToInstances[key] = append(subInstances, instance)
+ metadataInfo := lstn.revisionToMetadata[key]
if metadataInfo == nil {
- meta, err := GetMetadataInfo(lstn.app, instance, revision, lstn.registryId)
+ meta, err := GetMetadataInfo(providerApp, instance, revision, lstn.registryId)
if err != nil {
// Skip this instance if metadata fetch fails (e.g., old Java Dubbo version)
// Try next instance with same revision
@@ -141,22 +150,22 @@ func (lstn *ServiceInstancesChangedListenerImpl) OnEvent(e observer.Event) error
if serviceToRevisionServices[matchKey] == nil {
serviceToRevisionServices[matchKey] = make(map[string]*info.ServiceInfo)
}
- serviceToRevisionServices[matchKey][revision] = service
+ serviceToRevisionServices[matchKey][key] = service
}
- newRevisionToMetadata[revision] = metadataInfo
+ newRevisionToMetadata[key] = metadataInfo
}
}
lstn.revisionToMetadata = newRevisionToMetadata
- for revision, metadataInfo := range newRevisionToMetadata {
- cacheKey := lstn.app + ":" + lstn.registryId + ":" + revision
- metaCache.Set(cacheKey, metadataInfo)
+ for key, metadataInfo := range newRevisionToMetadata {
+ // key is already provider-app scoped and matches the disk cache key format.
+ metaCache.Set(key, metadataInfo)
}
for serviceKey, revisionServices := range serviceToRevisionServices {
urls := make([]*common.URL, 0, 8)
- for revision, serviceInfo := range revisionServices {
- for _, i := range revisionToInstances[revision] {
+ for key, serviceInfo := range revisionServices {
+ for _, i := range revisionToInstances[key] {
if i != nil {
urls = append(urls, toInstanceServiceURLs(i, serviceInfo)...)
}
@@ -244,12 +253,25 @@ func (lstn *ServiceInstancesChangedListenerImpl) GetEventType() reflect.Type {
return reflect.TypeOf(®istry.ServiceInstancesChangedEvent{})
}
-// GetMetadataInfo get metadata info when MetadataStorageTypePropertyName is null
+// metadataCacheKey builds the cache key that isolates MetadataInfo by provider
+// application, registry, and revision. MetadataInfo is owned by the provider app,
+// so app must be the provider application name (instance.GetServiceName()), never
+// the subscribing consumer app. Keying on revision alone would let two provider
+// apps that share a revision overwrite each other's metadata.
+func metadataCacheKey(app, registryId, revision string) string {
+ return app + ":" + registryId + ":" + revision
+}
+
+// GetMetadataInfo retrieves the MetadataInfo for a service instance by revision.
+// Results are cached by app+registryId+revision, where app must be the provider
+// application name. For "remote" storage type, it fetches from the metadata report
+// and falls back to RPC if the report fails or returns nil. For all other storage
+// types (including absent), it uses RPC directly.
func GetMetadataInfo(app string, instance registry.ServiceInstance, revision string, registryId string) (*info.MetadataInfo, error) {
cacheOnce.Do(func() {
initCache(app)
})
- cacheKey := app + ":" + registryId + ":" + revision
+ cacheKey := metadataCacheKey(app, registryId, revision)
if metadataInfo, ok := metaCache.Get(cacheKey); ok {
return metadataInfo.(*info.MetadataInfo), nil
}
@@ -258,20 +280,60 @@ func GetMetadataInfo(app string, instance registry.ServiceInstance, revision str
var metadataInfo *info.MetadataInfo
var err error
if instance.GetMetadata() == nil {
+ // No metadata map at all; treat as default (local/RPC) storage type.
metadataStorageType = constant.DefaultMetadataStorageType
} else {
metadataStorageType = instance.GetMetadata()[constant.MetadataStorageTypePropertyName]
+ if metadataStorageType == "" {
+ // MetadataStorageTypePropertyName absent (e.g. old Java provider); default to local storage type.
+ logger.Warnf("[Metadata] MetadataStorageType not set for instance %s, defaulting to local", instance.GetID())
+ metadataStorageType = constant.DefaultMetadataStorageType
+ }
}
+
if metadataStorageType == constant.RemoteMetadataStorageType {
- metadataInfo, err = metadata.GetMetadataFromMetadataReport(revision, instance, registryId)
- if err != nil {
- return nil, err
+ var reportErr error
+ metadataInfo, reportErr = metadata.GetMetadataFromMetadataReport(revision, instance, registryId)
+ if reportErr != nil {
+ logger.Errorf("[Metadata][Fallback] report failed, fallback to RPC app=%s registry=%s revision=%s err=%v",
+ app, registryId, revision, reportErr)
+ } else if metadataInfo == nil {
+ logger.Warnf("[Metadata][Fallback] report returned nil metadata, fallback to RPC app=%s registry=%s revision=%s",
+ app, registryId, revision)
+ } else {
+ metaCache.Set(cacheKey, metadataInfo)
+ return metadataInfo, nil
}
- } else {
+
metadataInfo, err = metadata.GetMetadataFromRpc(revision, instance)
if err != nil {
- return nil, err
+ if reportErr != nil {
+ // Wrap rpcErr so callers can use errors.Is/As on the primary failure;
+ // reportErr is annotated as context since it triggered the fallback.
+ return nil, perrors.Wrapf(err,
+ "both paths failed, reportErr: %v", reportErr)
+ }
+ // reportErr was nil — the report returned nil metadata and RPC also failed.
+ return nil, perrors.Wrapf(err,
+ "RPC fallback failed after report returned nil metadata")
}
+ if metadataInfo == nil {
+ return nil, perrors.Errorf("got nil metadata from RPC app=%s registry=%s revision=%s",
+ app, registryId, revision)
+ }
+ metaCache.Set(cacheKey, metadataInfo)
+ return metadataInfo, nil
+ }
+
+ // Non-remote storage type ("local" or absent): fetch metadata via RPC directly.
+ metadataInfo, err = metadata.GetMetadataFromRpc(revision, instance)
+ if err != nil {
+ return nil, perrors.Wrapf(err,
+ "failed app=%s registry=%s revision=%s", app, registryId, revision)
+ }
+ if metadataInfo == nil {
+ return nil, perrors.Errorf("got nil metadata from RPC app=%s registry=%s revision=%s",
+ app, registryId, revision)
}
metaCache.Set(cacheKey, metadataInfo)
return metadataInfo, nil
diff --git a/registry/servicediscovery/service_instances_changed_listener_impl_test.go b/registry/servicediscovery/service_instances_changed_listener_impl_test.go
index 24a09a642..22f7582ae 100644
--- a/registry/servicediscovery/service_instances_changed_listener_impl_test.go
+++ b/registry/servicediscovery/service_instances_changed_listener_impl_test.go
@@ -93,7 +93,8 @@ func TestServiceInstancesChangedListenerRefreshesAndClearsEnvironmentWhenRevisio
listener.AddListenerAndNotify(common.MatchKey(testInterface, constant.TriProtocol), notify)
revision := "rev-20001-same-environment-change"
- metaCache.Set(testApp+":"+constant.DefaultKey+":"+revision, newTestMetadataInfo(t, revision, 20001, "pre"))
+ cacheKey := testApp + ":" + constant.DefaultKey + ":" + revision
+ metaCache.Set(cacheKey, newTestMetadataInfo(t, revision, 20001, "pre"))
pre := newTestServiceInstanceOnly(20001, "pre", revision)
require.NoError(t, listener.OnEvent(registry.NewServiceInstancesChangedEvent(testApp, []registry.ServiceInstance{
@@ -129,11 +130,11 @@ func TestServiceInstancesChangedListenerSkipsNilMetadataWithoutPanic(t *testing.
listener.AddListenerAndNotify(common.MatchKey(testInterface, constant.TriProtocol), notify)
revision := "rev-20003-nil-metadata"
+ nilCacheKey := testApp + ":" + constant.DefaultKey + ":" + revision
var metadataInfo *info.MetadataInfo
- cacheKey := testApp + ":" + constant.DefaultKey + ":" + revision
- metaCache.Set(cacheKey, metadataInfo)
+ metaCache.Set(nilCacheKey, metadataInfo)
t.Cleanup(func() {
- metaCache.Delete(cacheKey)
+ metaCache.Delete(nilCacheKey)
})
instance := newTestServiceInstanceOnly(20003, "pre", revision)
@@ -208,7 +209,7 @@ func TestListenerUsesRegistryIdToFetchRemoteMetadata(t *testing.T) {
// Remove the cache entry after the test so it doesn't bleed into other tests.
t.Cleanup(func() {
if metaCache != nil {
- metaCache.Delete(listenerRegistryId + ":" + revision)
+ metaCache.Delete(testApp + ":" + listenerRegistryId + ":" + revision)
}
})
@@ -236,7 +237,8 @@ func (m *listenerMockMetadataReport) CreateMetadataReport(*common.URL) metadatar
func (m *listenerMockMetadataReport) GetAppMetadata(string, string) (*info.MetadataInfo, error) {
args := m.Called()
- return args.Get(0).(*info.MetadataInfo), args.Error(1)
+ result, _ := args.Get(0).(*info.MetadataInfo)
+ return result, args.Error(1)
}
func (m *listenerMockMetadataReport) PublishAppMetadata(string, string, *info.MetadataInfo) error {
@@ -268,9 +270,9 @@ func newTestServiceInstance(t *testing.T, port int, environment string) registry
func newTestServiceInstanceWithRevision(t *testing.T, port int, environment string, revision string) registry.ServiceInstance {
t.Helper()
- // Pre-populate the cache under the composite key used by GetMetadataInfo.
- // All test listeners use constant.DefaultKey as their registryId.
- metaCache.Set(testApp+":"+constant.DefaultKey+":"+revision, newTestMetadataInfo(t, revision, port, environment))
+ cacheKey := testApp + ":" + constant.DefaultKey + ":" + revision
+ metaCache.Set(cacheKey, newTestMetadataInfo(t, revision, port, environment))
+ t.Cleanup(func() { metaCache.Delete(cacheKey) })
return newTestServiceInstanceOnly(port, environment, revision)
}
@@ -372,3 +374,257 @@ func (c *capturingNotifyListener) NotifyAll(events []*registry.ServiceEvent, cal
callback()
}
}
+
+func TestGetMetadataInfo_CacheKeyFormat(t *testing.T) {
+ // Ensure cache is initialized (normally done by NewServiceInstancesChangedListener)
+ _ = NewServiceInstancesChangedListener(testApp, constant.DefaultKey, gxset.NewSet(testApp))
+
+ revision := "rev-cache-key-test"
+ // Pre-populate cache with the expected composite key
+ expectedKey := testApp + ":" + constant.DefaultKey + ":" + revision
+ expectedMeta := newTestMetadataInfo(t, revision, 20000, "dev")
+ metaCache.Set(expectedKey, expectedMeta)
+ t.Cleanup(func() {
+ metaCache.Delete(expectedKey)
+ })
+
+ instance := ®istry.DefaultServiceInstance{
+ ID: "127.0.0.1:20000",
+ ServiceName: testApp,
+ Host: "127.0.0.1",
+ Port: 20000,
+ Enable: true,
+ Healthy: true,
+ Metadata: map[string]string{
+ constant.ExportedServicesRevisionPropertyName: revision,
+ constant.ServiceInstanceEndpoints: `[{"port":20000,"protocol":"tri"}]`,
+ },
+ }
+
+ // Should hit the cache and return the pre-populated metadata
+ meta, err := GetMetadataInfo(testApp, instance, revision, constant.DefaultKey)
+ require.NoError(t, err)
+ assert.Equal(t, expectedMeta, meta)
+}
+
+func TestGetMetadataInfo_LocalStorageGoesDirectlyToRPC(t *testing.T) {
+ // Ensure cache is initialized
+ _ = NewServiceInstancesChangedListener(testApp, constant.DefaultKey, gxset.NewSet(testApp))
+
+ // Instance with no MetadataStorageTypePropertyName (i.e. local/default path)
+ // should go directly to RPC without touching the metadata report.
+ // RPC will fail with a URL error because there are no URL params — that's
+ // enough to confirm the correct branch was taken.
+ instance := ®istry.DefaultServiceInstance{
+ ID: "127.0.0.1:20003",
+ ServiceName: testApp,
+ Host: "127.0.0.1",
+ Port: 20003,
+ Enable: true,
+ Healthy: true,
+ Metadata: map[string]string{
+ constant.ExportedServicesRevisionPropertyName: "rev-local-rpc",
+ // MetadataStorageTypePropertyName intentionally absent → local path
+ constant.ServiceInstanceEndpoints: `[{"port":20003,"protocol":"tri"}]`,
+ },
+ }
+
+ _, err := GetMetadataInfo(testApp, instance, "rev-local-rpc", constant.DefaultKey)
+ require.Error(t, err)
+ // Must be a URL/RPC error, not a report error, confirming the local path
+ // skips the report entirely and goes straight to RPC.
+ assert.Contains(t, err.Error(), "metadata service URL params missing",
+ "local storage path should go directly to RPC, not touch the metadata report")
+}
+
+func TestGetMetadataInfo_FallbackToRPC(t *testing.T) {
+ // Ensure cache is initialized
+ _ = NewServiceInstancesChangedListener(testApp, constant.DefaultKey, gxset.NewSet(testApp))
+
+ // remote storage type without a report registered → report will fail
+ // should fall through to RPC, which will fail with url error (no URL params)
+ instance := ®istry.DefaultServiceInstance{
+ ID: "127.0.0.1:20002",
+ ServiceName: testApp,
+ Host: "127.0.0.1",
+ Port: 20002,
+ Enable: true,
+ Healthy: true,
+ Metadata: map[string]string{
+ constant.ExportedServicesRevisionPropertyName: "rev-fallback-to-rpc",
+ constant.MetadataStorageTypePropertyName: constant.RemoteMetadataStorageType,
+ constant.ServiceInstanceEndpoints: `[{"port":20002,"protocol":"tri"}]`,
+ },
+ }
+
+ _, err := GetMetadataInfo(testApp, instance, "rev-fallback-to-rpc", constant.DefaultKey)
+ require.Error(t, err)
+ // Both report and RPC fail: the combined error proves the fallback path was taken
+ // and includes the RPC/URL failure as the wrapped cause.
+ assert.Contains(t, err.Error(), "both paths failed",
+ "fallback path should produce a combined error mentioning both failures")
+ assert.Contains(t, err.Error(), "metadata service URL params missing",
+ "fallback error should include the RPC/URL failure cause")
+}
+
+// TestGetMetadataInfo_ReportReturnsNil_FallsBackToRPC verifies the path where the metadata
+// report returns (nil, nil) — no error but no data — which must trigger the RPC fallback.
+// Here the instance has no URL params, so RPC fails; the test asserts the resulting error
+// comes from the RPC fallback (not the report), proving the nil-result branch was taken.
+func TestGetMetadataInfo_ReportReturnsNil_FallsBackToRPC(t *testing.T) {
+ const regID = "report-nil-rpc-ok"
+ const revision = "rev-report-nil-rpc-ok"
+
+ // Register a mock report that returns (nil, nil) — success with no data.
+ mockReport := new(listenerMockMetadataReport)
+ extension.SetMetadataReportFactory(regID, func() metadatareport.MetadataReportFactory {
+ return mockReport
+ })
+ opts := metadata.NewReportOptions(
+ metadata.WithRegistryId(regID),
+ metadata.WithProtocol(regID),
+ metadata.WithAddress("127.0.0.1"),
+ )
+ require.NoError(t, opts.Init())
+ t.Cleanup(metadata.ClearMetadataReportInstances)
+ t.Cleanup(func() {
+ if metaCache != nil {
+ metaCache.Delete(testApp + ":" + regID + ":" + revision)
+ }
+ })
+
+ mockReport.On("GetAppMetadata").Return((*info.MetadataInfo)(nil), nil).Once()
+
+ // The instance has no URL params, so the RPC fallback fails at URL construction.
+ // That failure is the observable proof that the report's nil result triggered fallback.
+ instance := ®istry.DefaultServiceInstance{
+ ID: "127.0.0.1:20098",
+ ServiceName: testApp,
+ Host: "127.0.0.1",
+ Port: 20098,
+ Enable: true,
+ Healthy: true,
+ Metadata: map[string]string{
+ constant.ExportedServicesRevisionPropertyName: revision,
+ constant.MetadataStorageTypePropertyName: constant.RemoteMetadataStorageType,
+ // No URL params — RPC will fail at the URL-construction stage
+ },
+ }
+ _ = NewServiceInstancesChangedListener(testApp, regID, gxset.NewSet(testApp))
+
+ _, err := GetMetadataInfo(testApp, instance, revision, regID)
+ require.Error(t, err)
+ // The report returned nil (no error), so the fallback was triggered and then RPC
+ // failed at URL construction. The error must reflect the RPC-after-nil-report path.
+ assert.Contains(t, err.Error(), "RPC fallback failed after report returned nil metadata",
+ "nil report result should trigger fallback and surface an RPC error")
+ assert.Contains(t, err.Error(), "metadata service URL params missing",
+ "fallback error should include the RPC/URL failure cause")
+ mockReport.AssertExpectations(t)
+}
+
+// TestGetMetadataInfo_CacheTypedNilNoPanic verifies that when the cache holds a typed nil
+// *info.MetadataInfo entry, GetMetadataInfo does not panic and returns (nil, nil).
+// This covers a defensive edge case (e.g., a previous store of typed nil) rather than
+// the production nil guard in the RPC path. The RPC nil guard is exercised indirectly
+// through TestGetMetadataInfo_ReportReturnsNil_FallsBackToRPC.
+func TestGetMetadataInfo_CacheTypedNilNoPanic(t *testing.T) {
+ // This path is exercised via the cache: pre-seed with a typed nil *info.MetadataInfo.
+ // GetMetadataInfo hits the cache fast-path and returns the typed nil without calling RPC.
+ // Asserts that this does NOT panic, giving the caller a (nil, nil) to skip gracefully.
+ _ = NewServiceInstancesChangedListener(testApp, constant.DefaultKey, gxset.NewSet(testApp))
+
+ revision := "rev-rpc-nil-meta-guard"
+ cacheKey := testApp + ":" + constant.DefaultKey + ":" + revision
+ var nilMeta *info.MetadataInfo
+ metaCache.Set(cacheKey, nilMeta)
+ t.Cleanup(func() { metaCache.Delete(cacheKey) })
+
+ instance := newTestServiceInstanceOnly(20098, "dev", revision)
+
+ // Must not panic; the typed nil in the cache is returned as (nil, nil).
+ var result *info.MetadataInfo
+ require.NotPanics(t, func() {
+ result, _ = GetMetadataInfo(testApp, instance, revision, constant.DefaultKey)
+ })
+ assert.Nil(t, result, "typed nil from cache should be returned as nil MetadataInfo")
+}
+
+// TestGetMetadataInfo_NilMetadataMap verifies that an instance with a nil Metadata map
+// is handled gracefully by GetMetadataInfo and takes the local/RPC path.
+// The nil-map guard in GetMetadataInfo is unreachable from OnEvent (which has its own
+// nil check), so it must be tested by calling GetMetadataInfo directly.
+func TestGetMetadataInfo_NilMetadataMap(t *testing.T) {
+ _ = NewServiceInstancesChangedListener(testApp, constant.DefaultKey, gxset.NewSet(testApp))
+
+ instance := ®istry.DefaultServiceInstance{
+ ID: "127.0.0.1:20097",
+ ServiceName: testApp,
+ Host: "127.0.0.1",
+ Port: 20097,
+ Enable: true,
+ Healthy: true,
+ Metadata: nil, // nil map — triggers the nil-map guard
+ }
+
+ _, err := GetMetadataInfo(testApp, instance, "rev-nil-map", constant.DefaultKey)
+ // With no URL params the RPC call fails, but it must be an RPC/URL error —
+ // not a panic and not a report error — proving the nil-map guard worked and
+ // the local path was taken.
+ require.Error(t, err)
+ assert.NotPanics(t, func() {
+ _, _ = GetMetadataInfo(testApp, instance, "rev-nil-map-nopanic", constant.DefaultKey)
+ })
+}
+
+// TestGetMetadataInfo_ProviderAppIsolatesSharedRevision verifies that two different
+// provider applications sharing the same revision do NOT collide in the cache.
+// The cache key is scoped by provider app, so each app's MetadataInfo is isolated
+// even when their revision strings are identical.
+func TestGetMetadataInfo_ProviderAppIsolatesSharedRevision(t *testing.T) {
+ _ = NewServiceInstancesChangedListener(testApp, constant.DefaultKey, gxset.NewSet(testApp))
+
+ const sharedRevision = "rev-shared-across-apps"
+ const providerA = "order-service"
+ const providerB = "payment-service"
+
+ keyA := providerA + ":" + constant.DefaultKey + ":" + sharedRevision
+ keyB := providerB + ":" + constant.DefaultKey + ":" + sharedRevision
+ metaA := info.NewMetadataInfo(providerA, sharedRevision)
+ metaB := info.NewMetadataInfo(providerB, sharedRevision)
+ metaCache.Set(keyA, metaA)
+ metaCache.Set(keyB, metaB)
+ t.Cleanup(func() {
+ metaCache.Delete(keyA)
+ metaCache.Delete(keyB)
+ })
+
+ instanceA := ®istry.DefaultServiceInstance{
+ ID: "127.0.0.1:21001",
+ ServiceName: providerA,
+ Host: "127.0.0.1",
+ Port: 21001,
+ Metadata: map[string]string{
+ constant.ExportedServicesRevisionPropertyName: sharedRevision,
+ },
+ }
+ instanceB := ®istry.DefaultServiceInstance{
+ ID: "127.0.0.1:21002",
+ ServiceName: providerB,
+ Host: "127.0.0.1",
+ Port: 21002,
+ Metadata: map[string]string{
+ constant.ExportedServicesRevisionPropertyName: sharedRevision,
+ },
+ }
+
+ // Each provider app must resolve to its own MetadataInfo, not the other's,
+ // despite sharing the same revision.
+ gotA, err := GetMetadataInfo(instanceA.GetServiceName(), instanceA, sharedRevision, constant.DefaultKey)
+ require.NoError(t, err)
+ assert.Equal(t, providerA, gotA.App, "provider A must get its own metadata")
+
+ gotB, err := GetMetadataInfo(instanceB.GetServiceName(), instanceB, sharedRevision, constant.DefaultKey)
+ require.NoError(t, err)
+ assert.Equal(t, providerB, gotB.App, "provider B must get its own metadata, not provider A's")
+}