From bba8931d64e412a296a20f153b9a62f7b7d4f4dc Mon Sep 17 00:00:00 2001 From: chrislu Date: Thu, 30 Oct 2025 20:56:13 -0700 Subject: [PATCH] Improve encapsulation: Add shallowClone() method to vidMap Added a shallowClone() method to vidMap to improve encapsulation and prevent MasterClient from directly accessing vidMap's internal fields. Changes: 1. Added vidMap.shallowClone() in vid_map.go - Encapsulates the shallow copy logic within vidMap - Makes vidMap responsible for its own state representation - Documented that caller is responsible for thread safety 2. Simplified resetVidMap() in masterclient.go - Uses tail := mc.vidMap.shallowClone() instead of manual field access - Cleaner, more maintainable code - Better adherence to encapsulation principles Benefits: - Improved code organization and maintainability - vidMap internals are now properly encapsulated - Easier to modify vidMap structure in the future - More self-documenting code Verified with: go test -race ./weed/wdclient/... (passes) --- weed/wdclient/masterclient.go | 10 +++------- weed/wdclient/vid_map.go | 11 +++++++++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/weed/wdclient/masterclient.go b/weed/wdclient/masterclient.go index a91cb3614..c883576f1 100644 --- a/weed/wdclient/masterclient.go +++ b/weed/wdclient/masterclient.go @@ -592,14 +592,10 @@ func (mc *MasterClient) resetVidMap() { mc.vidMapLock.Lock() defer mc.vidMapLock.Unlock() - tail := &vidMap{ - vid2Locations: mc.vidMap.vid2Locations, - ecVid2Locations: mc.vidMap.ecVid2Locations, - DataCenter: mc.vidMap.DataCenter, - cache: mc.vidMap.cache, - } + // Create a shallow clone to preserve in the cache chain + tail := mc.vidMap.shallowClone() - nvm := newVidMap(mc.vidMap.DataCenter) + nvm := newVidMap(tail.DataCenter) nvm.cache = tail mc.vidMap = nvm diff --git a/weed/wdclient/vid_map.go b/weed/wdclient/vid_map.go index 9d5e5d378..6d6ccd6e6 100644 --- a/weed/wdclient/vid_map.go +++ b/weed/wdclient/vid_map.go @@ -53,6 +53,17 @@ func newVidMap(dataCenter string) *vidMap { } } +// shallowClone creates a shallow copy of the vidMap for use in cache chaining. +// The caller is responsible for ensuring thread safety. +func (vc *vidMap) shallowClone() *vidMap { + return &vidMap{ + vid2Locations: vc.vid2Locations, + ecVid2Locations: vc.ecVid2Locations, + DataCenter: vc.DataCenter, + cache: vc.cache, + } +} + func (vc *vidMap) getLocationIndex(length int) (int, error) { if length <= 0 { return 0, fmt.Errorf("invalid length: %d", length)