diff --git a/pkg/transaction/metric/metricscollector.go b/pkg/transaction/metric/metricscollector.go index e990da384..295ef89ae 100644 --- a/pkg/transaction/metric/metricscollector.go +++ b/pkg/transaction/metric/metricscollector.go @@ -659,7 +659,7 @@ func (c *collector) createAPIDetail(api models.APIDetails) *models.APIResourceRe Name: api.Name, } cacheManager := agent.GetCacheManager() - svc := cacheManager.GetAPIServiceWithAPIID(strings.TrimPrefix(api.ID, transutil.SummaryEventProxyIDPrefix)) + svc := cacheManager.GetAPIServiceWithAPIID(transutil.StripSummaryEventPrefix(api.ID)) ref.APIServiceID = unknown if svc != nil { ref.APIServiceID = svc.Metadata.ID diff --git a/pkg/transaction/metric/metricscollector_test.go b/pkg/transaction/metric/metricscollector_test.go index 132b71581..fcbc0f147 100644 --- a/pkg/transaction/metric/metricscollector_test.go +++ b/pkg/transaction/metric/metricscollector_test.go @@ -24,6 +24,7 @@ import ( "github.com/Axway/agent-sdk/pkg/config" "github.com/Axway/agent-sdk/pkg/traceability" "github.com/Axway/agent-sdk/pkg/transaction/models" + transutil "github.com/Axway/agent-sdk/pkg/transaction/util" "github.com/Axway/agent-sdk/pkg/util/healthcheck" "github.com/Axway/agent-sdk/pkg/util/log" ) @@ -1208,6 +1209,65 @@ func TestCollectorCreateOrUpdateHistogramIDResolution(t *testing.T) { } } +func TestCreateAPIDetail(t *testing.T) { + cases := map[string]struct { + apiServiceRI *apiv1.ResourceInstance + apiID string + apiName string + wantOwnType string + wantOwnGUID string + wantSvcID string + }{ + "real ID prefix resolves owner and apiServiceId from cache": { + apiServiceRI: withMetaID(makeAPIServiceRI("api-1", &apiv1.Owner{Type: apiv1.TeamOwner, ID: testTeamGUID1}), "svc-meta-api-1"), + apiID: transutil.SummaryEventProxyIDPrefix + "api-1", + apiName: "api-one", + wantOwnType: "team", + wantOwnGUID: testTeamGUID1, + wantSvcID: "svc-meta-api-1", + }, + "name-fallback prefix resolves owner and apiServiceId from cache": { + apiServiceRI: withMetaID(makeAPIServiceRI("api-2", &apiv1.Owner{Type: apiv1.TeamOwner, ID: testTeamGUID1}), "svc-meta-api-2"), + apiID: transutil.SummaryEventAPINamePrefix + "api-2", + apiName: "api-two", + wantOwnType: "team", + wantOwnGUID: testTeamGUID1, + wantSvcID: "svc-meta-api-2", + }, + "api not found in cache returns unknown owner and apiServiceId": { + apiServiceRI: nil, + apiID: transutil.SummaryEventProxyIDPrefix + "missing-api", + apiName: "missing", + wantOwnType: "unknown", + wantSvcID: unknown, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + agent.InitializeForTest(nil, agent.TestWithCentralConfig(newUtilCacheConfig())) + if tc.apiServiceRI != nil { + agent.GetCacheManager().AddAPIService(tc.apiServiceRI) + } + + c := &collector{logger: log.NewFieldLogger()} + ref := c.createAPIDetail(models.APIDetails{ID: tc.apiID, Name: tc.apiName}) + if ref == nil { + t.Fatal("expected non-nil API resource reference") + } + assert.Equal(t, tc.apiID, ref.ID) + assert.Equal(t, tc.apiName, ref.Name) + assert.Equal(t, tc.wantSvcID, ref.APIServiceID) + if ref.Owner == nil { + t.Fatal("expected non-nil owner") + } + assert.Equal(t, tc.wantOwnType, ref.Owner.Type) + if tc.wantOwnGUID != "" { + assert.Equal(t, tc.wantOwnGUID, ref.Owner.TeamGUID) + } + }) + } +} + // noopStorage satisfies the storageCache interface with no-ops for all methods except // removeMetric, which records calls for assertion in cleanup tests. type noopStorage struct { diff --git a/pkg/transaction/metric/util.go b/pkg/transaction/metric/util.go index 8aa49ac5b..b64582667 100644 --- a/pkg/transaction/metric/util.go +++ b/pkg/transaction/metric/util.go @@ -157,7 +157,7 @@ func buildAPIRef(api models.APIDetails) *models.APIResourceReference { Name: api.Name, } cacheManager := agent.GetCacheManager() - svc := cacheManager.GetAPIServiceWithAPIID(strings.TrimPrefix(api.ID, transutil.SummaryEventProxyIDPrefix)) + svc := cacheManager.GetAPIServiceWithAPIID(transutil.StripSummaryEventPrefix(api.ID)) if svc != nil { ref.APIServiceID = svc.Metadata.ID } diff --git a/pkg/transaction/metric/util_test.go b/pkg/transaction/metric/util_test.go index cfd557964..480320bc9 100644 --- a/pkg/transaction/metric/util_test.go +++ b/pkg/transaction/metric/util_test.go @@ -12,6 +12,7 @@ import ( "github.com/Axway/agent-sdk/pkg/cmd" "github.com/Axway/agent-sdk/pkg/config" "github.com/Axway/agent-sdk/pkg/transaction/models" + transutil "github.com/Axway/agent-sdk/pkg/transaction/util" ) const ( @@ -30,6 +31,13 @@ func makeAPIServiceRI(apiID string, owner *v1.Owner) *v1.ResourceInstance { return ri } +// withMetaID sets the resource's metadata ID, letting a test case declare the +// cached resource's identity inline instead of mutating it in the test runner. +func withMetaID(ri *v1.ResourceInstance, id string) *v1.ResourceInstance { + ri.Metadata.ID = id + return ri +} + func TestCentralMetricFromAPIMetric(t *testing.T) { testCfg := &config.CentralConfiguration{AgentName: "agent"} agent.InitializeForTest(nil, agent.TestWithCentralConfig(testCfg)) @@ -546,3 +554,57 @@ func TestResolveAppOwnerFromCache(t *testing.T) { }) } } + +func TestBuildAPIRef(t *testing.T) { + cases := map[string]struct { + apiServiceRI *v1.ResourceInstance + apiID string + apiName string + wantOwnType string + wantOwnGUID string + wantSvcID string + }{ + "real ID prefix resolves owner and apiServiceId from cache": { + apiServiceRI: withMetaID(makeAPIServiceRI("api-1", &v1.Owner{Type: v1.TeamOwner, ID: testTeamGUID1}), "svc-meta-api-1"), + apiID: transutil.SummaryEventProxyIDPrefix + "api-1", + apiName: "api-one", + wantOwnType: "team", + wantOwnGUID: testTeamGUID1, + wantSvcID: "svc-meta-api-1", + }, + "name-fallback prefix resolves owner and apiServiceId from cache": { + apiServiceRI: withMetaID(makeAPIServiceRI("api-2", &v1.Owner{Type: v1.TeamOwner, ID: testTeamGUID1}), "svc-meta-api-2"), + apiID: transutil.SummaryEventAPINamePrefix + "api-2", + apiName: "api-two", + wantOwnType: "team", + wantOwnGUID: testTeamGUID1, + wantSvcID: "svc-meta-api-2", + }, + "api not found in cache returns unknown owner and empty apiServiceId": { + apiServiceRI: nil, + apiID: transutil.SummaryEventProxyIDPrefix + "missing-api", + apiName: "missing", + wantOwnType: "unknown", + wantSvcID: "", + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + agent.InitializeForTest(nil, agent.TestWithCentralConfig(newUtilCacheConfig())) + if tc.apiServiceRI != nil { + agent.GetCacheManager().AddAPIService(tc.apiServiceRI) + } + + ref := buildAPIRef(models.APIDetails{ID: tc.apiID, Name: tc.apiName}) + require.NotNil(t, ref) + assert.Equal(t, tc.apiID, ref.ID) + assert.Equal(t, tc.apiName, ref.Name) + assert.Equal(t, tc.wantSvcID, ref.APIServiceID) + require.NotNil(t, ref.Owner) + assert.Equal(t, tc.wantOwnType, ref.Owner.Type) + if tc.wantOwnGUID != "" { + assert.Equal(t, tc.wantOwnGUID, ref.Owner.TeamGUID) + } + }) + } +} diff --git a/pkg/transaction/util/ownerresolver.go b/pkg/transaction/util/ownerresolver.go index c0b1a402f..1f6da0f10 100644 --- a/pkg/transaction/util/ownerresolver.go +++ b/pkg/transaction/util/ownerresolver.go @@ -1,8 +1,6 @@ package util import ( - "strings" - "github.com/Axway/agent-sdk/pkg/agent/cache" v1 "github.com/Axway/agent-sdk/pkg/apic/apiserver/models/api/v1" management "github.com/Axway/agent-sdk/pkg/apic/apiserver/models/management/v1" @@ -19,7 +17,7 @@ func ResolveAPIOwner(apiExternalID string, cacheManager cache.Manager) *models.O return &models.Owner{Type: "unknown"} } - apiID := strings.TrimPrefix(apiExternalID, SummaryEventProxyIDPrefix) + apiID := StripSummaryEventPrefix(apiExternalID) ri := cacheManager.GetAPIServiceWithAPIID(apiID) if ri == nil { logger.WithField("apiID", apiID).Trace("api service not found in cache, owner is unknown") diff --git a/pkg/transaction/util/ownerresolver_test.go b/pkg/transaction/util/ownerresolver_test.go index 6ec5b3613..d99d56e9c 100644 --- a/pkg/transaction/util/ownerresolver_test.go +++ b/pkg/transaction/util/ownerresolver_test.go @@ -84,6 +84,11 @@ func TestResolveAPIOwner(t *testing.T) { cache: newCacheWithAPIService("api-5", &v1.Owner{Type: v1.TeamOwner, ID: "team-5"}), expected: &models.Owner{Type: "team", TeamGUID: "team-5"}, }, + "name-fallback prefix stripped before lookup": { + apiID: SummaryEventAPINamePrefix + "api-7", + cache: newCacheWithAPIService("api-7", &v1.Owner{Type: v1.TeamOwner, ID: "team-7"}), + expected: &models.Owner{Type: "team", TeamGUID: "team-7"}, + }, "api service x-private owner": { apiID: "api-6", cache: newCacheWithAPIService("api-6", &v1.Owner{Type: v1.TeamOwner, ID: "team-guid-6", User: &v1.OwnerUser{ID: testOwnerUserGUID}}), diff --git a/pkg/transaction/util/util.go b/pkg/transaction/util/util.go index 1f3cff73d..ba6835bfc 100644 --- a/pkg/transaction/util/util.go +++ b/pkg/transaction/util/util.go @@ -26,6 +26,14 @@ const ( SummaryEventApplicationIDPrefix = "remoteAppId_" ) +// StripSummaryEventPrefix removes whichever proxy-ID prefix ResolveIDWithPrefix produced. +// Meaning the real-ID prefix or the name-fallback prefix, leaving the bare value used as a cache lookup key. +func StripSummaryEventPrefix(apiID string) string { + apiID = strings.TrimPrefix(apiID, SummaryEventProxyIDPrefix) + apiID = strings.TrimPrefix(apiID, SummaryEventAPINamePrefix) + return apiID +} + // GetAccessRequest - func GetAccessRequest(cacheManager cache.Manager, managedApp *v1.ResourceInstance, apiID, stage, version string) *management.AccessRequest { if managedApp == nil { @@ -33,7 +41,7 @@ func GetAccessRequest(cacheManager cache.Manager, managedApp *v1.ResourceInstanc } // Lookup Access Request - apiID = strings.TrimPrefix(apiID, SummaryEventProxyIDPrefix) + apiID = StripSummaryEventPrefix(apiID) accessReq := &management.AccessRequest{} ri := cacheManager.GetAccessRequestByAppAndAPIStageVersion(managedApp.Name, apiID, stage, version) if ri == nil { diff --git a/pkg/transaction/util/util_test.go b/pkg/transaction/util/util_test.go index 4f5ac35f1..83bc144b3 100644 --- a/pkg/transaction/util/util_test.go +++ b/pkg/transaction/util/util_test.go @@ -128,6 +128,36 @@ func TestResolveIDWithPrefix(t *testing.T) { } } +func TestStripSummaryEventPrefix(t *testing.T) { + tests := map[string]struct { + apiID string + expected string + }{ + "real ID prefix stripped": { + apiID: SummaryEventProxyIDPrefix + "dwight", + expected: "dwight", + }, + "name-fallback prefix stripped": { + apiID: SummaryEventAPINamePrefix + "schrute", + expected: "schrute", + }, + "no prefix, returned as-is": { + apiID: "dwight", + expected: "dwight", + }, + "empty string": { + apiID: "", + expected: "", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.expected, StripSummaryEventPrefix(tc.apiID)) + }) + } +} + func TestGetMarketplaceDetails(t *testing.T) { // "marketplace" as a string instead of an object triggers a real parse failure. malformedInstance := &v1.ResourceInstance{} diff --git a/pkg/transaction/v2event.go b/pkg/transaction/v2event.go index c8faaa831..04e415846 100644 --- a/pkg/transaction/v2event.go +++ b/pkg/transaction/v2event.go @@ -402,7 +402,7 @@ func buildSummaryAPIDetail(logger log.FieldLogger, apiID, apiName, apiServiceID detail := &insightsAPIDetail{ID: apiID, Name: apiName, Owner: apiOwner, APIServiceID: apiServiceID} if apiServiceID == "" && cacheManager != nil { - stripped := strings.TrimPrefix(apiID, transutil.SummaryEventProxyIDPrefix) + stripped := transutil.StripSummaryEventPrefix(apiID) if svc := cacheManager.GetAPIServiceWithAPIID(stripped); svc != nil { detail.APIServiceID = svc.Metadata.ID } diff --git a/pkg/transaction/v2event_test.go b/pkg/transaction/v2event_test.go index af87d57d7..f249bda60 100644 --- a/pkg/transaction/v2event_test.go +++ b/pkg/transaction/v2event_test.go @@ -8,12 +8,28 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/Axway/agent-sdk/pkg/agent/cache" + v1 "github.com/Axway/agent-sdk/pkg/apic/apiserver/models/api/v1" + management "github.com/Axway/agent-sdk/pkg/apic/apiserver/models/management/v1" + "github.com/Axway/agent-sdk/pkg/config" "github.com/Axway/agent-sdk/pkg/traceability/redaction" "github.com/Axway/agent-sdk/pkg/transaction/metric" "github.com/Axway/agent-sdk/pkg/transaction/models" + transutil "github.com/Axway/agent-sdk/pkg/transaction/util" "github.com/Axway/agent-sdk/pkg/util/log" ) +func newAPIServiceRIForTest(apiID, metaID string, owner *v1.Owner) *v1.ResourceInstance { + svc := management.NewAPIService("svc-"+apiID, "env1") + svc.SubResources = map[string]interface{}{ + "x-agent-details": map[string]interface{}{"externalAPIID": apiID}, + } + svc.Owner = owner + ri, _ := svc.AsInstance() + ri.Metadata.ID = metaID + return ri +} + // compile-time assertions. Both data types must satisfy the V4Data interface. var _ metric.V4Data = (*TransactionLegData)(nil) var _ metric.V4Data = (*TransactionSummaryData)(nil) @@ -1187,3 +1203,56 @@ func TestV4DataInterfaceMethods(t *testing.T) { assert.Equal(t, "Success", fields["status"]) }) } + +func TestBuildSummaryAPIDetailAPIServiceIDFallback(t *testing.T) { + const ( + testAPIServiceGUID = "team-guid-svc" + testFallbackAPIName = "api-name" + ) + + cases := map[string]struct { + apiServiceRI *v1.ResourceInstance + apiID string + apiServiceIDArg string + wantAPIServiceID string + }{ + "apiServiceId already populated skips cache lookup": { + // meta ID deliberately differs from apiServiceIDArg to prove the cache is never consulted. + apiServiceRI: newAPIServiceRIForTest("api-pre", "unused-cached-id", &v1.Owner{Type: v1.TeamOwner, ID: testAPIServiceGUID}), + apiID: transutil.SummaryEventProxyIDPrefix + "api-pre", + apiServiceIDArg: "already-known-id", + wantAPIServiceID: "already-known-id", + }, + "real ID prefix resolves apiServiceId via cache fallback": { + apiServiceRI: newAPIServiceRIForTest("api-real", "svc-meta-api-real", &v1.Owner{Type: v1.TeamOwner, ID: testAPIServiceGUID}), + apiID: transutil.SummaryEventProxyIDPrefix + "api-real", + apiServiceIDArg: "", + wantAPIServiceID: "svc-meta-api-real", + }, + "name-fallback prefix resolves apiServiceId via cache fallback": { + apiServiceRI: newAPIServiceRIForTest(testFallbackAPIName, "svc-meta-api-name", &v1.Owner{Type: v1.TeamOwner, ID: testAPIServiceGUID}), + apiID: transutil.SummaryEventAPINamePrefix + testFallbackAPIName, + apiServiceIDArg: "", + wantAPIServiceID: "svc-meta-api-name", + }, + "api not found in cache leaves apiServiceId empty": { + apiServiceRI: nil, + apiID: transutil.SummaryEventProxyIDPrefix + "missing-api", + apiServiceIDArg: "", + wantAPIServiceID: "", + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + cacheManager := cache.NewAgentCacheManager(&config.CentralConfiguration{}, false) + if tc.apiServiceRI != nil { + cacheManager.AddAPIService(tc.apiServiceRI) + } + + detail := buildSummaryAPIDetail(log.NewFieldLogger(), tc.apiID, testFallbackAPIName, tc.apiServiceIDArg, nil, cacheManager) + require.NotNil(t, detail) + assert.Equal(t, tc.wantAPIServiceID, detail.APIServiceID) + }) + } +}