Skip to content

Commit

Permalink
refactored Record.data and Record.expand to be concurrent safe
Browse files Browse the repository at this point in the history
  • Loading branch information
ganigeorgiev committed Jan 25, 2023
1 parent 39df263 commit ae371e8
Show file tree
Hide file tree
Showing 38 changed files with 312 additions and 87 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@

- Improved API and validations error reporting by providing more detailed messages.

- Refactored `models.Record` expand and data change operations to be concurrent safe.

- Added `models.Record.CleanCopy()` helper that creates a new record copy with only the latest data state of the existing one and all other options reset to their defaults.

- Added several `store.Store` helpers:
```go
store.Reset(newData map[string]T)
store.Length() int
store.GetAll() map[string]T
```

## v0.11.3

Expand Down
11 changes: 4 additions & 7 deletions apis/realtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,9 @@ func (api *realtimeApi) broadcastRecord(action string, record *models.Record) er
return nil // no subscribers
}

// remove the expand from the broadcasted record because we don't
// know if the clients have access to view the expanded records
cleanRecord := *record
cleanRecord.SetExpand(nil)
cleanRecord.WithUnkownData(false)
cleanRecord.IgnoreEmailVisibility(false)
// create a clean record copy without expand and unknown fields
// because we don't know if the clients have permissions to view them
cleanRecord := record.CleanCopy()

subscriptionRuleMap := map[string]*string{
(collection.Name + "/" + cleanRecord.Id): collection.ViewRule,
Expand All @@ -367,7 +364,7 @@ func (api *realtimeApi) broadcastRecord(action string, record *models.Record) er

data := &recordData{
Action: action,
Record: &cleanRecord,
Record: cleanRecord,
}

dataBytes, err := json.Marshal(data)
Expand Down
2 changes: 2 additions & 0 deletions daos/record_expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ func (dao *Dao) expandRecords(records []*models.Record, expandPath string, fetch
recordIds[i] = record.Id
}

// @todo after the index optimizations consider allowing
// indirect expand for multi-relation fields
indirectRecords, err := dao.FindRecordsByExpr(
indirectRel.Id,
dbx.In(inflector.Columnify(matches[2]), recordIds...),
Expand Down
119 changes: 82 additions & 37 deletions models/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/pocketbase/pocketbase/models/schema"
"github.com/pocketbase/pocketbase/tools/list"
"github.com/pocketbase/pocketbase/tools/security"
"github.com/pocketbase/pocketbase/tools/store"
"github.com/pocketbase/pocketbase/tools/types"
"github.com/spf13/cast"
"golang.org/x/crypto/bcrypt"
Expand All @@ -28,19 +29,19 @@ type Record struct {

collection *Collection

exportUnknown bool // whether to export unknown fields
ignoreEmailVisibility bool // whether to ignore the emailVisibility flag for auth collections
data map[string]any // any custom data in addition to the base model fields
expand map[string]any // expanded relations
exportUnknown bool // whether to export unknown fields
ignoreEmailVisibility bool // whether to ignore the emailVisibility flag for auth collections
loaded bool
originalData map[string]any // the original (aka. first loaded) model data
originalData map[string]any // the original (aka. first loaded) model data
expand *store.Store[any] // expanded relations
data *store.Store[any] // any custom data in addition to the base model fields
}

// NewRecord initializes a new empty Record model.
func NewRecord(collection *Collection) *Record {
return &Record{
collection: collection,
data: map[string]any{},
data: store.New[any](nil),
}
}

Expand Down Expand Up @@ -109,7 +110,8 @@ func (m *Record) Collection() *Collection {
}

// OriginalCopy returns a copy of the current record model populated
// with its original (aka. the initially loaded) data state.
// with its ORIGINAL data state (aka. the initially loaded) and
// everything else reset to the defaults.
func (m *Record) OriginalCopy() *Record {
newRecord := NewRecord(m.collection)
newRecord.Load(m.originalData)
Expand All @@ -123,15 +125,40 @@ func (m *Record) OriginalCopy() *Record {
return newRecord
}

// Expand returns a shallow copy of the record.expand data
// attached to the current Record model.
// CleanCopy returns a copy of the current record model populated only
// with its LATEST data state and everything else reset to the defaults.
func (m *Record) CleanCopy() *Record {
newRecord := NewRecord(m.collection)
newRecord.Load(m.data.GetAll())
newRecord.Id = m.Id
newRecord.Created = m.Created
newRecord.Updated = m.Updated

if m.IsNew() {
newRecord.MarkAsNew()
} else {
newRecord.MarkAsNotNew()
}

return newRecord
}

// Expand returns a shallow copy of the current Record model expand data.
func (m *Record) Expand() map[string]any {
return shallowCopy(m.expand)
if m.expand == nil {
m.expand = store.New[any](nil)
}

return m.expand.GetAll()
}

// SetExpand assigns the provided data to record.expand.
// SetExpand shallow copies the provided data to the current Record model's expand.
func (m *Record) SetExpand(expand map[string]any) {
m.expand = shallowCopy(expand)
if m.expand == nil {
m.expand = store.New[any](nil)
}

m.expand.Reset(expand)
}

// MergeExpand merges recursively the provided expand data into
Expand All @@ -141,14 +168,23 @@ func (m *Record) SetExpand(expand map[string]any) {
// then both old and new records will be merged into a new slice (aka. a :merge: [b,c] => [a,b,c]).
// Otherwise the "old" expanded record will be replace with the "new" one (aka. a :merge: aNew => aNew).
func (m *Record) MergeExpand(expand map[string]any) {
if m.expand == nil && len(expand) > 0 {
m.expand = make(map[string]any, len(expand))
// nothing to merge
if len(expand) == 0 {
return
}

// no old expand
if m.expand == nil {
m.expand = store.New(expand)
return
}

oldExpand := m.expand.GetAll()

for key, new := range expand {
old, ok := m.expand[key]
old, ok := oldExpand[key]
if !ok {
m.expand[key] = new
oldExpand[key] = new
continue
}

Expand All @@ -163,7 +199,7 @@ func (m *Record) MergeExpand(expand map[string]any) {
default:
// invalid old expand data -> assign directly the new
// (no matter whether new is valid or not)
m.expand[key] = new
oldExpand[key] = new
continue
}

Expand Down Expand Up @@ -197,19 +233,23 @@ func (m *Record) MergeExpand(expand map[string]any) {
}

if wasOldSlice || wasNewSlice || len(oldSlice) == 0 {
m.expand[key] = oldSlice
oldExpand[key] = oldSlice
} else {
m.expand[key] = oldSlice[0]
oldExpand[key] = oldSlice[0]
}
}

m.expand.Reset(oldExpand)
}

// SchemaData returns a shallow copy ONLY of the defined record schema fields data.
func (m *Record) SchemaData() map[string]any {
result := make(map[string]any, len(m.collection.Schema.Fields()))

data := m.data.GetAll()

for _, field := range m.collection.Schema.Fields() {
if v, ok := m.data[field.Name]; ok {
if v, ok := data[field.Name]; ok {
result[field.Name] = v
}
}
Expand All @@ -221,7 +261,11 @@ func (m *Record) SchemaData() map[string]any {
// aka. fields that are neither one of the base and special system ones,
// nor defined by the collection schema.
func (m *Record) UnknownData() map[string]any {
return m.extractUnknownData(m.data)
if m.data == nil {
return nil
}

return m.extractUnknownData(m.data.GetAll())
}

// IgnoreEmailVisibility toggles the flag to ignore the auth record email visibility check.
Expand Down Expand Up @@ -267,10 +311,10 @@ func (m *Record) Set(key string, value any) {
}

if m.data == nil {
m.data = map[string]any{}
m.data = store.New[any](nil)
}

m.data[key] = v
m.data.Set(key, v)
}
}

Expand All @@ -284,11 +328,11 @@ func (m *Record) Get(key string) any {
case schema.FieldNameUpdated:
return m.Updated
default:
if v, ok := m.data[key]; ok {
return v
if m.data == nil {
return nil
}

return nil
return m.data.Get(key)
}
}

Expand Down Expand Up @@ -331,10 +375,11 @@ func (m *Record) GetStringSlice(key string) []string {
// Retrieves the "key" json field value and unmarshals it into "result".
//
// Example
// result := struct {
// FirstName string `json:"first_name"`
// }{}
// err := m.UnmarshalJSONField("my_field_name", &result)
//
// result := struct {
// FirstName string `json:"first_name"`
// }{}
// err := m.UnmarshalJSONField("my_field_name", &result)
func (m *Record) UnmarshalJSONField(key string, result any) error {
return json.Unmarshal([]byte(m.GetString(key)), &result)
}
Expand Down Expand Up @@ -432,8 +477,8 @@ func (m *Record) PublicExport() map[string]any {
result[schema.FieldNameCollectionName] = m.collection.Name

// add expand (if set)
if m.expand != nil {
result[schema.FieldNameExpand] = m.expand
if m.expand != nil && m.expand.Length() > 0 {
result[schema.FieldNameExpand] = m.expand.GetAll()
}

return result
Expand Down Expand Up @@ -469,10 +514,10 @@ func (m *Record) UnmarshalJSON(data []byte) error {
//
// Example usage:
//
// newData := record.ReplaceModifers(data)
// // record: {"field": 10}
// // data: {"field+": 5}
// // newData: {"field": 15}
// newData := record.ReplaceModifers(data)
// // record: {"field": 10}
// // data: {"field+": 5}
// // newData: {"field": 15}
func (m *Record) ReplaceModifers(data map[string]any) map[string]any {
var clone = shallowCopy(data)
if len(clone) == 0 {
Expand Down Expand Up @@ -624,7 +669,7 @@ func (m *Record) extractUnknownData(data map[string]any) map[string]any {

result := map[string]any{}

for k, v := range m.data {
for k, v := range data {
if _, ok := knownFields[k]; !ok {
result[k] = v
}
Expand Down
35 changes: 34 additions & 1 deletion models/record_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package models_test

import (
"bytes"
"database/sql"
"encoding/json"
"testing"
Expand Down Expand Up @@ -346,7 +347,7 @@ func TestRecordOriginalCopy(t *testing.T) {
t.Fatalf("Expected the initial/original f to be %q, got %q", "123", v)
}

// Loading new data shouldn't affect the original state
// loading new data shouldn't affect the original state
m.Load(map[string]any{"f": "789"})

if v := m.GetString("f"); v != "789" {
Expand All @@ -358,6 +359,38 @@ func TestRecordOriginalCopy(t *testing.T) {
}
}

func TestRecordCleanCopy(t *testing.T) {
m := models.NewRecord(&models.Collection{
Name: "cname",
Type: models.CollectionTypeAuth,
})
m.Load(map[string]any{
"id": "id1",
"created": "2023-01-01 00:00:00.000Z",
"updated": "2023-01-02 00:00:00.000Z",
"username": "test",
"verified": true,
"email": "[email protected]",
"unknown": "456",
})

// make a change to ensure that the latest data is targeted
m.Set("id", "id2")

// allow the special flags and options to check whether they will be ignored
m.SetExpand(map[string]any{"test": 123})
m.IgnoreEmailVisibility(true)
m.WithUnkownData(true)

copy := m.CleanCopy()
copyExport, _ := copy.MarshalJSON()

expectedExport := []byte(`{"collectionId":"","collectionName":"cname","created":"2023-01-01 00:00:00.000Z","emailVisibility":false,"id":"id2","updated":"2023-01-02 00:00:00.000Z","username":"test","verified":true}`)
if !bytes.Equal(copyExport, expectedExport) {
t.Fatalf("Expected clean export \n%s, \ngot \n%s", expectedExport, copyExport)
}
}

func TestRecordSetAndGetExpand(t *testing.T) {
collection := &models.Collection{}
m := models.NewRecord(collection)
Expand Down
Loading

0 comments on commit ae371e8

Please sign in to comment.