Use copy on write strategy for metadata #489
Conversation
Codecov Report
@@ Coverage Diff @@
## master #489 +/- ##
==========================================
- Coverage 86.76% 86.71% -0.05%
==========================================
Files 186 186
Lines 8280 8272 -8
==========================================
- Hits 7184 7173 -11
- Misses 896 898 +2
- Partials 200 201 +1
Continue to review full report at Codecov.
|
device/metadata.go
Outdated
| m := make(Metadata) | ||
| m.SetJWTClaims(deepCopyMap(claims)) | ||
| m.initSessionID() | ||
| func NewDeviceMetadataWithClaims(claims map[string]interface{}) *Metadata { |
There was a problem hiding this comment.
As a general design principle, you want to allow someone to do var m Metadata or m := new(Metadata) and have a Metadata object ready to go. That's preferable to a constructor. Sometimes you can't avoid having a constructor, but in this case you can if:
- The internal
Metadatamethods should handle the case when theatomic.Valueisn't initialized. This is easier than you might think, since anilmap is readable but not writable:
func (m *Metadata) loadData() map[string]interface{} {
claims, _ := m.v.Load().(map[string]interface{})
return claims // this can be nil for reads
}
- Every exported method that sets keys or sets the claims wholesale should do the deep copy. Again, that's relatively easy and not very expensive if you're optimizing for reads.
There was a problem hiding this comment.
I can give that a try. I just found the constructors to be much simpler initializing defaults that should be available even on reads.
Yeah, we can change to a deepCopy of the entire map. It thought we might not need them since we're always overwriting only top keys.
No description provided.