Skip to content

replace Map with sync.Map to handle concurrent reads/writes (changes generated by Cursor); #2646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
+93 −59
Open

replace Map with sync.Map to handle concurrent reads/writes (changes generated by Cursor); #2646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b894b5
replace Map with sync.Map to handle concurrent reads/writes (changes …
jaredholloway94 Apr 14, 2025
c0d8ded
Merge branch 'pyrevitlabs:develop' into develop
jaredholloway94 Apr 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 77 additions & 56 deletions dev/pyRevitTelemetryServer/persistence/models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package persistence

import (
"fmt"
"sync"

"pyrevittelemetryserver/cli"

Expand All @@ -21,26 +22,37 @@ type TraceInfoV1 struct {
}

type ScriptTelemetryRecordV1 struct {
Date string `json:"date" bson:"date" valid:"-"`
Time string `json:"time" bson:"time" valid:"-"`
UserName string `json:"username" bson:"username" valid:"-"`
RevitVersion string `json:"revit" bson:"revit" valid:"numeric~Invalid revit version"`
RevitBuild string `json:"revitbuild" bson:"revitbuild" valid:"matches(\\d{8}_\\d{4}\\(x\\d{2}\\))~Invalid revit build number"`
SessionId string `json:"sessionid" bson:"sessionid" valid:"uuidv4~Invalid session id"`
PyRevitVersion string `json:"pyrevit" bson:"pyrevit" valid:"-"`
IsDebugMode bool `json:"debug" bson:"debug"`
IsConfigMode bool `json:"config" bson:"config"`
CommandName string `json:"commandname" bson:"commandname" valid:"-"`
CommandUniqueName string `json:"commanduniquename" bson:"commanduniquename" valid:"-"`
BundleName string `json:"commandbundle" bson:"commandbundle" valid:"-"`
ExtensionName string `json:"commandextension" bson:"commandextension" valid:"-"`
ResultCode int `json:"resultcode" bson:"resultcode" valid:"numeric~Invalid result code"`
CommandResults map[string]string `json:"commandresults" bson:"commandresults" valid:"-"`
ScriptPath string `json:"scriptpath" bson:"scriptpath" valid:"-"`
TraceInfo TraceInfoV1 `json:"trace" bson:"trace"`
Date string `json:"date" bson:"date" valid:"-"`
Time string `json:"time" bson:"time" valid:"-"`
UserName string `json:"username" bson:"username" valid:"-"`
RevitVersion string `json:"revit" bson:"revit" valid:"numeric~Invalid revit version"`
RevitBuild string `json:"revitbuild" bson:"revitbuild" valid:"matches(\\d{8}_\\d{4}\\(x\\d{2}\\))~Invalid revit build number"`
SessionId string `json:"sessionid" bson:"sessionid" valid:"uuidv4~Invalid session id"`
PyRevitVersion string `json:"pyrevit" bson:"pyrevit" valid:"-"`
IsDebugMode bool `json:"debug" bson:"debug"`
IsConfigMode bool `json:"config" bson:"config"`
CommandName string `json:"commandname" bson:"commandname" valid:"-"`
CommandUniqueName string `json:"commanduniquename" bson:"commanduniquename" valid:"-"`
BundleName string `json:"commandbundle" bson:"commandbundle" valid:"-"`
ExtensionName string `json:"commandextension" bson:"commandextension" valid:"-"`
ResultCode int `json:"resultcode" bson:"resultcode" valid:"numeric~Invalid result code"`
CommandResults sync.Map `json:"commandresults" bson:"commandresults" valid:"-"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using sync.Map with JSON marshaling/unmarshaling may cause issues with serialization. The default Go JSON encoder doesn't have built-in support for sync.Map. You'll need to implement custom MarshalJSON and UnmarshalJSON methods for each struct using sync.Map to properly serialize and deserialize the data. Without this, data might be lost or improperly stored during JSON operations.

ScriptPath string `json:"scriptpath" bson:"scriptpath" valid:"-"`
TraceInfo TraceInfoV1 `json:"trace" bson:"trace"`
}

func (logrec ScriptTelemetryRecordV1) PrintRecordInfo(logger *cli.Logger, message string) {
// Convert sync.Map to regular map for printing
results := make(map[string]string)
logrec.CommandResults.Range(func(key, value interface{}) bool {
if k, ok := key.(string); ok {
if v, ok := value.(string); ok {
results[k] = v
}
}
return true
})

logger.Print(fmt.Sprintf(
"%s %s-%s %q @ %s:%s [%s.%s] code=%d info=%v",
message,
Expand All @@ -52,7 +64,7 @@ func (logrec ScriptTelemetryRecordV1) PrintRecordInfo(logger *cli.Logger, messag
logrec.ExtensionName,
logrec.CommandName,
logrec.ResultCode,
logrec.CommandResults,
results,
))
}

Expand All @@ -66,10 +78,10 @@ func (logrec ScriptTelemetryRecordV1) Validate() error {

// v2.0
type EngineInfoV2 struct {
Type string `json:"type" bson:"type" valid:"engine~Invalid executor engine type"`
Version string `json:"version" bson:"version" valid:"-"`
SysPaths []string `json:"syspath" bson:"syspath" valid:"-"`
Configs map[string]interface{} `json:"configs" bson:"configs" valid:"-"`
Type string `json:"type" bson:"type" valid:"engine~Invalid executor engine type"`
Version string `json:"version" bson:"version" valid:"-"`
SysPaths []string `json:"syspath" bson:"syspath" valid:"-"`
Configs sync.Map `json:"configs" bson:"configs" valid:"-"`
}

type TraceInfoV2 struct {
Expand All @@ -82,33 +94,42 @@ type RecordMetaV2 struct {
}

type ScriptTelemetryRecordV2 struct {
RecordMeta RecordMetaV2 `json:"meta" bson:"meta"`
TimeStamp string `json:"timestamp" bson:"timestamp" valid:"rfc3339~Invalid timestamp"`
UserName string `json:"username" bson:"username" valid:"-"`
HostUserName string `json:"host_user" bson:"host_user" valid:"-"`
RevitVersion string `json:"revit" bson:"revit" valid:"numeric~Invalid revit version"`
RevitBuild string `json:"revitbuild" bson:"revitbuild" valid:"matches(\\d{8}_\\d{4}\\(x\\d{2}\\))~Invalid revit build number"`
SessionId string `json:"sessionid" bson:"sessionid" valid:"uuidv4~Invalid session id"`
PyRevitVersion string `json:"pyrevit" bson:"pyrevit" valid:"-"`
Clone string `json:"clone" bson:"clone" valid:"-"`
IsDebugMode bool `json:"debug" bson:"debug"`
IsConfigMode bool `json:"config" bson:"config"`
IsExecFromGUI bool `json:"from_gui" bson:"from_gui"`
ExecId string `json:"exec_id" bson:"exec_id" valid:"-"`
ExecTimeStamp string `json:"exec_timestamp" bson:"exec_timestamp" valid:"-"`
CommandName string `json:"commandname" bson:"commandname" valid:"-"`
CommandUniqueName string `json:"commanduniquename" bson:"commanduniquename" valid:"-"`
BundleName string `json:"commandbundle" bson:"commandbundle" valid:"-"`
ExtensionName string `json:"commandextension" bson:"commandextension" valid:"-"`
DocumentName string `json:"docname" bson:"docname" valid:"-"`
DocumentPath string `json:"docpath" bson:"docpath" valid:"-"`
ResultCode int `json:"resultcode" bson:"resultcode" valid:"numeric~Invalid result code"`
CommandResults map[string]interface{} `json:"commandresults" bson:"commandresults" valid:"-"`
ScriptPath string `json:"scriptpath" bson:"scriptpath" valid:"-"`
TraceInfo TraceInfoV2 `json:"trace" bson:"trace"`
RecordMeta RecordMetaV2 `json:"meta" bson:"meta"`
TimeStamp string `json:"timestamp" bson:"timestamp" valid:"rfc3339~Invalid timestamp"`
UserName string `json:"username" bson:"username" valid:"-"`
HostUserName string `json:"host_user" bson:"host_user" valid:"-"`
RevitVersion string `json:"revit" bson:"revit" valid:"numeric~Invalid revit version"`
RevitBuild string `json:"revitbuild" bson:"revitbuild" valid:"matches(\\d{8}_\\d{4}\\(x\\d{2}\\))~Invalid revit build number"`
SessionId string `json:"sessionid" bson:"sessionid" valid:"uuidv4~Invalid session id"`
PyRevitVersion string `json:"pyrevit" bson:"pyrevit" valid:"-"`
Clone string `json:"clone" bson:"clone" valid:"-"`
IsDebugMode bool `json:"debug" bson:"debug"`
IsConfigMode bool `json:"config" bson:"config"`
IsExecFromGUI bool `json:"from_gui" bson:"from_gui"`
ExecId string `json:"exec_id" bson:"exec_id" valid:"-"`
ExecTimeStamp string `json:"exec_timestamp" bson:"exec_timestamp" valid:"-"`
CommandName string `json:"commandname" bson:"commandname" valid:"-"`
CommandUniqueName string `json:"commanduniquename" bson:"commanduniquename" valid:"-"`
BundleName string `json:"commandbundle" bson:"commandbundle" valid:"-"`
ExtensionName string `json:"commandextension" bson:"commandextension" valid:"-"`
DocumentName string `json:"docname" bson:"docname" valid:"-"`
DocumentPath string `json:"docpath" bson:"docpath" valid:"-"`
ResultCode int `json:"resultcode" bson:"resultcode" valid:"numeric~Invalid result code"`
CommandResults sync.Map `json:"commandresults" bson:"commandresults" valid:"-"`
ScriptPath string `json:"scriptpath" bson:"scriptpath" valid:"-"`
TraceInfo TraceInfoV2 `json:"trace" bson:"trace"`
}

func (logrec ScriptTelemetryRecordV2) PrintRecordInfo(logger *cli.Logger, message string) {
// Convert sync.Map to regular map for printing
results := make(map[string]interface{})
logrec.CommandResults.Range(func(key, value interface{}) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a potential type safety issue here. The original code has explicit type assertions when converting to a regular map, but the current implementation might assign different types to the results map. The value type should be explicitly checked like it is in the ScriptTelemetryRecordV1.PrintRecordInfo method (lines 46-54). Consider adding proper type assertion for the value parameter similar to:

logrec.CommandResults.Range(func(key, value interface{}) bool {
    if k, ok := key.(string); ok {
        if v, ok := value.(interface{}); ok {
            results[k] = v
        }
    }
    return true
})

if k, ok := key.(string); ok {
results[k] = value
}
return true
})

logger.Print(fmt.Sprintf(
"%s %s %q %s:%s (%s) [%s.%s] code=%d info=%v",
message,
Expand All @@ -120,7 +141,7 @@ func (logrec ScriptTelemetryRecordV2) PrintRecordInfo(logger *cli.Logger, messag
logrec.ExtensionName,
logrec.CommandName,
logrec.ResultCode,
logrec.CommandResults,
results,
))
}

Expand Down Expand Up @@ -158,15 +179,15 @@ func (logrec ScriptTelemetryRecordV2) Validate() error {

// introduced with api v2
type EventTelemetryRecordV2 struct {
RecordMeta RecordMetaV2 `json:"meta" bson:"meta"`
TimeStamp string `json:"timestamp" bson:"timestamp" valid:"rfc3339~Invalid timestamp"`
HandlerId string `json:"handler_id" bson:"handler_id" valid:"-"`
EventType string `json:"type" bson:"type" valid:"-"`
EventArgs map[string]interface{} `json:"args" bson:"args" valid:"-"`
UserName string `json:"username" bson:"username" valid:"-"`
HostUserName string `json:"host_user" bson:"host_user" valid:"-"`
RevitVersion string `json:"revit" bson:"revit" valid:"numeric~Invalid revit version"`
RevitBuild string `json:"revitbuild" bson:"revitbuild" valid:"matches(\\d{8}_\\d{4}\\(x\\d{2}\\))~Invalid revit build number"`
RecordMeta RecordMetaV2 `json:"meta" bson:"meta"`
TimeStamp string `json:"timestamp" bson:"timestamp" valid:"rfc3339~Invalid timestamp"`
HandlerId string `json:"handler_id" bson:"handler_id" valid:"-"`
EventType string `json:"type" bson:"type" valid:"-"`
EventArgs sync.Map `json:"args" bson:"args" valid:"-"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventTelemetryRecordV2 PrintRecordInfo method (line 204) doesn't handle the EventArgs sync.Map conversion before printing. Add code similar to what's done in ScriptTelemetryRecordV2.PrintRecordInfo to convert the sync.Map to a regular map before printing, otherwise the logging output may be incorrect or misleading.

UserName string `json:"username" bson:"username" valid:"-"`
HostUserName string `json:"host_user" bson:"host_user" valid:"-"`
RevitVersion string `json:"revit" bson:"revit" valid:"numeric~Invalid revit version"`
RevitBuild string `json:"revitbuild" bson:"revitbuild" valid:"matches(\\d{8}_\\d{4}\\(x\\d{2}\\))~Invalid revit build number"`

// general
Cancellable bool `json:"cancellable" bson:"cancellable"`
Expand Down
5 changes: 4 additions & 1 deletion dev/pyRevitTelemetryServer/server/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"sync"

"pyrevittelemetryserver/cli"
"pyrevittelemetryserver/persistence"
Expand Down Expand Up @@ -37,7 +38,9 @@ func RouteEvents(router *mux.Router, opts *cli.Options, dbConn persistence.Conne
// https://stackoverflow.com/a/26212073
router.HandleFunc("/api/v2/events/", func(w http.ResponseWriter, r *http.Request) {
// parse given json data into a new record
logrec := persistence.EventTelemetryRecordV2{}
logrec := persistence.EventTelemetryRecordV2{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sync.Map initialization is correct, but there should be proper error handling for JSON deserialization of sync.Map fields. If the incoming JSON contains data intended for EventArgs, the current code might silently ignore it since the unmarshalled data may not properly map to sync.Map. Consider adding custom JSON unmarshalling for this struct or validate that EventArgs is correctly populated after decoding.

EventArgs: sync.Map{},
}
decodeErr := json.NewDecoder(r.Body).Decode(&logrec)
if decodeErr != nil {
logger.Debug(decodeErr)
Expand Down
14 changes: 12 additions & 2 deletions dev/pyRevitTelemetryServer/server/scripts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"sync"

"pyrevittelemetryserver/cli"
"pyrevittelemetryserver/persistence"
Expand Down Expand Up @@ -47,7 +48,9 @@ func RouteScripts(router *mux.Router, opts *cli.Options, dbConn persistence.Conn
// https://stackoverflow.com/a/26212073
router.HandleFunc("/api/v1/scripts/", func(w http.ResponseWriter, r *http.Request) {
// parse given json data into a new record
logrec := persistence.ScriptTelemetryRecordV1{}
logrec := persistence.ScriptTelemetryRecordV1{
CommandResults: sync.Map{},
}
decodeErr := json.NewDecoder(r.Body).Decode(&logrec)
if decodeErr != nil {
logger.Debug(decodeErr)
Expand Down Expand Up @@ -78,7 +81,14 @@ func RouteScripts(router *mux.Router, opts *cli.Options, dbConn persistence.Conn

router.HandleFunc("/api/v2/scripts/", func(w http.ResponseWriter, r *http.Request) {
// parse given json data into a new record
logrec := persistence.ScriptTelemetryRecordV2{}
logrec := persistence.ScriptTelemetryRecordV2{
CommandResults: sync.Map{},
TraceInfo: persistence.TraceInfoV2{
EngineInfo: persistence.EngineInfoV2{
Configs: sync.Map{},
},
},
}
decodeErr := json.NewDecoder(r.Body).Decode(&logrec)
if decodeErr != nil {
logger.Debug(decodeErr)
Expand Down