From a22457ce5f0be156f69fb37e4b6fae23c5ab68d6 Mon Sep 17 00:00:00 2001 From: m1saka Date: Wed, 22 Jul 2026 11:38:23 +0800 Subject: [PATCH] fix: preserve partial batch property results --- device.go | 20 +++++++++++++++++--- device_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++------ types.go | 1 + 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/device.go b/device.go index 9ccc69e..fb576f0 100644 --- a/device.go +++ b/device.go @@ -747,6 +747,7 @@ func (device *Device) GetMany(ctx context.Context, names []string) ([]DeviceProp return nil, err } byIdentity := make(map[propertyIdentity]PropertyResult, len(chunkResults)) + duplicateIdentities := make(map[propertyIdentity]struct{}) requested := make(map[propertyIdentity]struct{}, end-start) for _, request := range requests[start:end] { requested[propertyIdentity{did: request.DID, siid: request.SIID, piid: request.PIID}] = struct{}{} @@ -757,17 +758,30 @@ func (device *Device) GetMany(ctx context.Context, names []string) ([]DeviceProp return nil, fmt.Errorf("get properties protocol error: unexpected identity (%s,%d,%d)", result.DID, result.SIID, result.PIID) } if _, duplicate := byIdentity[identity]; duplicate { - return nil, fmt.Errorf("get properties protocol error: duplicate identity (%s,%d,%d)", result.DID, result.SIID, result.PIID) + duplicateIdentities[identity] = struct{}{} + continue } byIdentity[identity] = result } for index, request := range requests[start:end] { identity := propertyIdentity{did: request.DID, siid: request.SIID, piid: request.PIID} + name := names[start+index] + if _, duplicate := duplicateIdentities[identity]; duplicate { + results[start+index] = DevicePropertyResult{ + Name: name, + Err: fmt.Errorf("get properties protocol error: duplicate identity (%s,%d,%d)", request.DID, request.SIID, request.PIID), + } + continue + } result, ok := byIdentity[identity] if !ok { - return nil, fmt.Errorf("get properties protocol error: missing identity (%s,%d,%d)", request.DID, request.SIID, request.PIID) + results[start+index] = DevicePropertyResult{ + Name: name, + Err: fmt.Errorf("get properties protocol error: missing identity (%s,%d,%d)", request.DID, request.SIID, request.PIID), + } + continue } - results[start+index] = DevicePropertyResult{Name: names[start+index], Value: result.Value, Code: result.Code} + results[start+index] = DevicePropertyResult{Name: name, Value: result.Value, Code: result.Code} } } if err := device.wait(ctx); err != nil { diff --git a/device_test.go b/device_test.go index 7b62f35..3c30df5 100644 --- a/device_test.go +++ b/device_test.go @@ -625,26 +625,51 @@ func TestDeviceGetManyMatchesIdentityAndPreservesBusinessErrors(t *testing.T) { } } -func TestDeviceGetManyRejectsInvalidResponseIdentities(t *testing.T) { +func TestDeviceGetManyPreservesPartialResultsForInvalidResponseIdentities(t *testing.T) { tests := []struct { name string response string + wantErrs []string }{ - {name: "missing", response: `[{"did":"a","siid":2,"piid":1,"value":true,"code":0}]`}, - {name: "duplicate", response: `[{"did":"a","siid":2,"piid":1,"value":true,"code":0},{"did":"a","siid":2,"piid":1,"value":false,"code":0}]`}, - {name: "extra", response: `[{"did":"a","siid":2,"piid":1,"value":true,"code":0},{"did":"a","siid":2,"piid":2,"value":5,"code":0},{"did":"other","siid":9,"piid":9,"value":1,"code":0}]`}, + {name: "missing", response: `[{"did":"a","siid":2,"piid":1,"value":true,"code":0}]`, wantErrs: []string{"", "missing identity"}}, + {name: "duplicate and missing", response: `[{"did":"a","siid":2,"piid":1,"value":true,"code":0},{"did":"a","siid":2,"piid":1,"value":false,"code":0}]`, wantErrs: []string{"duplicate identity", "missing identity"}}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { device := fixtureDeviceWithResults(t, []string{test.response}, 0) results, err := device.GetMany(context.Background(), []string{"power", "brightness"}) - if err == nil || results != nil || !strings.Contains(err.Error(), "protocol") { - t.Fatalf("GetMany() = %#v, %v, want nil protocol error", results, err) + if err != nil || len(results) != 2 { + t.Fatalf("GetMany() = %#v, %v", results, err) + } + for index, wantErr := range test.wantErrs { + if results[index].Name != []string{"power", "brightness"}[index] { + t.Fatalf("result %d name = %q", index, results[index].Name) + } + if wantErr == "" { + if results[index].Err != nil || results[index].Value != true { + t.Fatalf("result %d = %#v, want successful power result", index, results[index]) + } + } else if results[index].Err == nil || !strings.Contains(results[index].Err.Error(), wantErr) { + t.Fatalf("result %d error = %v, want %q", index, results[index].Err, wantErr) + } } }) } } +func TestDeviceGetManyRejectsExtraResponseIdentity(t *testing.T) { + device := fixtureDeviceWithResults(t, []string{`[ + {"did":"a","siid":2,"piid":1,"value":true,"code":0}, + {"did":"a","siid":2,"piid":2,"value":5,"code":0}, + {"did":"other","siid":9,"piid":9,"value":1,"code":0} + ]`}, 0) + + results, err := device.GetMany(context.Background(), []string{"power", "brightness"}) + if err == nil || results != nil || !strings.Contains(err.Error(), "protocol") { + t.Fatalf("GetMany() = %#v, %v, want nil protocol error", results, err) + } +} + func TestDeviceGetManyValidatesBeforeNetwork(t *testing.T) { device, testServer := fixtureDeviceWithServer(t, nil) tests := [][]string{{"power", "power"}, {"power", "missing"}, {"power", "write-only"}} @@ -692,6 +717,20 @@ func TestDeviceGetManyWaitsOnceAfterAllChunks(t *testing.T) { } } +func TestDeviceGetManyWaitsOnceWithPartialProtocolErrors(t *testing.T) { + device := fixtureDeviceWithResults(t, []string{`[{"did":"a","siid":2,"piid":1,"value":true,"code":0}]`}, 40*time.Millisecond) + + started := time.Now() + results, err := device.GetMany(context.Background(), []string{"power", "brightness"}) + elapsed := time.Since(started) + if err != nil || len(results) != 2 || results[1].Err == nil { + t.Fatalf("GetMany() = %#v, %v", results, err) + } + if elapsed < 30*time.Millisecond || elapsed >= 75*time.Millisecond { + t.Fatalf("GetMany() delay = %v, want one approximately 40ms wait", elapsed) + } +} + func batchPropertyFixture(count int) (map[string]PropertySpec, []string) { properties := make(map[string]PropertySpec, count) names := make([]string, count) diff --git a/types.go b/types.go index c18f25b..0c0f8ee 100644 --- a/types.go +++ b/types.go @@ -163,6 +163,7 @@ type DevicePropertyResult struct { Name string Value any Code int + Err error `json:"-"` } type ActionRequest struct {