Merge pull request #1084 from xmidt-org/denopink/cherry-pick/patch/fi…

…xed-duplicated–device-removal

patch: fixed duplicated device removing itself (#439 cherry picked)

.gitignore

@@ -9,4 +9,9 @@ report.json
 .idea
 server/cpuprofile
 server/memprofile
+
+# VSCode
 *.code-workspace
+.vscode/*
+.dev/*
+

device/manager.go

@@ -293,8 +293,11 @@ func (m *manager) dispatch(e *Event) {
 // dispatches message failed events for any messages that were waiting to be delivered
 // at the time of pump closure.
 func (m *manager) pumpClose(d *device, c io.Closer, reason CloseReason) {
-	// remove will invoke requestClose()
-	m.devices.remove(d.id, reason)
+
+	if !m.isDeviceDuplicated(d) {
+		// remove will invoke requestClose()
+		m.devices.remove(d.id, reason)
+	}
 
 	closeError := c.Close()
 
@@ -635,3 +638,11 @@ func (m *manager) Route(request *Request) (*Response, error) {
 func (m *manager) MaxDevices() int {
 	return m.devices.limit
 }
+
+func (m *manager) isDeviceDuplicated(d *device) bool {
+	existing, ok := m.devices.get(d.id)
+	if !ok {
+		return false
+	}
+	return existing.state != d.state
+}

device/manager_test.go

@@ -533,3 +533,50 @@ func newTestCounter() *testCounter {
 		labelPairs: make(map[string]string),
 	}
 }
+
+func TestManagerIsDeviceDuplicated(t *testing.T) {
+	var (
+		assert = assert.New(t)
+		tests  = []struct {
+			expected bool
+			existing *device
+			new      *device
+			m        *manager
+		}{
+			{
+				expected: false,
+				existing: nil,
+				new:      &device{id: "test"},
+				m: NewManager(&Options{
+					MaxDevices: 0,
+				}).(*manager),
+			},
+			{
+				expected: false,
+				existing: &device{id: "test", state: stateOpen},
+				new:      &device{id: "test", state: stateOpen},
+				m: NewManager(&Options{
+					MaxDevices: 0,
+				}).(*manager),
+			},
+			{
+				expected: true,
+				existing: &device{id: "test", state: stateOpen},
+				new:      &device{id: "test", state: stateClosed},
+				m: NewManager(&Options{
+					MaxDevices: 0,
+				}).(*manager),
+			},
+		}
+	)
+
+	for _, test := range tests {
+		if test.existing != nil {
+			err := test.m.devices.add(test.existing)
+			if err != nil {
+				assert.Error(err)
+			}
+		}
+		assert.Equal(test.expected, test.m.isDeviceDuplicated(test.new))
+	}
+}