Use accessors (new, get, set) instead of direct field access - #172
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the NRF cache implementation and its tests to use generated model accessor/constructor methods (e.g., Get*, Set*, New*) instead of direct struct field access, and adds a compatibility accessor for TAI on ApiSearchNFInstancesRequest.
Changes:
- Replaced direct
SearchResult/NFProfileDiscoveryfield usage innrfcachewith generated getters/setters. - Updated
nrfcacheunit tests to construct and mutate models viaNew*+Set*APIs. - Added
GetTai()accessor toApiSearchNFInstancesRequestcompat helpers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| nrfcache/nrfcache.go | Switches cache hit/miss handling and cache keying to use Get*/Set* accessors. |
| nrfcache/nrfcache_test.go | Updates test fixtures and callbacks to use generated constructors/setters. |
| nrfcache/match_filters.go | Converts matching logic to nil-safe getters (notably affects nil-profile behavior). |
| Nnrf_NFDiscovery/api_search_nf_instances_request_compat.go | Adds GetTai() accessor for request compatibility. |
Suppressed comments (4)
nrfcache/match_filters.go:310
- MatchUdmProfile can now return true for a nil profile (both with and without SUPI filters) because accessor methods are nil-safe and the function defaults to "match any". This risks selecting an invalid profile.
func MatchUdmProfile(profile *models.NFProfileDiscovery, opts Nnrf_NFDiscovery.ApiSearchNFInstancesRequest) (bool, error) {
supi := opts.GetSupi()
nrfcache/match_filters.go:336
- MatchUdrProfile can now return true for a nil profile (e.g., when SUPI filter is not set, it immediately returns true and logs GetNfInstanceId()). Add an explicit nil guard to prevent false matches and to align with MatchAmfProfile's behavior.
func MatchUdrProfile(profile *models.NFProfileDiscovery, opts Nnrf_NFDiscovery.ApiSearchNFInstancesRequest) (bool, error) {
supi := opts.GetSupi()
nrfcache/match_filters.go:295
- MatchPcfProfile now treats a nil profile as an unrestricted match because GetPcfInfoOk() is nil-safe and the logic treats missing PcfInfo/SupiRanges as "match any". A nil profile should be rejected explicitly.
pcfInfo, ok := profile.GetPcfInfoOk()
if !ok || len(pcfInfo.GetSupiRanges()) == 0 {
logger.NrfcacheLog.Debugf("pcf match found = true (unrestricted: no SUPI ranges)")
nrfcache/match_filters.go:195
- MatchAusfProfile can now treat a nil profile as a successful match because accessor methods are nil-safe and the logic treats missing AusfInfo/SupiRanges as "unrestricted". Add an explicit nil guard (similar to MatchAmfProfile) so invalid cache data can’t be selected.
ausfInfo, ok := profile.GetAusfInfoOk()
if !ok || len(ausfInfo.GetSupiRanges()) == 0 {
logger.NrfcacheLog.Debugf("ausf match successful (unrestricted: no SUPI ranges) for %s", profile.GetNfInstanceId())
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f8a07a0 to
c0905d1
Compare
Signed-off-by: Arrobo, Gabriel <gabriel.arrobo@intel.com>
Signed-off-by: Arrobo, Gabriel <gabriel.arrobo@intel.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
nrfcache/match_filters.go:72
- In SNSSAI matching, switching the SmfInfo branch condition from a nil-check to
len(...) > 0changes semantics: an explicitly provided but emptysNssaiSmfInfoListwill now fall back toAllowedNssaisand may match, whereas the previous logic treated a non-nil (even empty) list as authoritative and would not fall back. If an empty list is meant to mean “no supported SNSSAIs”, this would incorrectly allow matches.
if hasSmfInfo && len(smfInfo.GetSNssaiSmfInfoList()) > 0 {
No description provided.