refactoring mistake

- fix
- tests

Author: Mikhail Podtserkovskiy

handlers/createSession.go

@@ -79,13 +79,7 @@ func (h *CreateSession) tryCreateSession(r *http.Request, capabilities *capabili
 	if err != nil {
 		return nil, errors.New("reserve node error: " + err.Error())
 	}
-	//todo: посылать в мониторинг событие, если вернулся не 0
-	seleniumClient := h.ClientFactory.Create(node.Address)
-	seleniumNode := jsonwire.NewNode(seleniumClient)
-	_, err = seleniumNode.RemoveAllSessions()
-	if err != nil {
-		return nil, errors.New("Can't remove all sessions from node: " + err.Error() + ", go to next available node: " + node.String())
-	}
+
 	reverseProxy := httputil.NewSingleHostReverseProxy(&url.URL{
 		Scheme: "http",
 		Host:   node.Address,

pool/strategy/persistent/factory.go

@@ -13,5 +13,10 @@ func (f *StrategyFactory) Create(
 	capsComparator capabilities.ComparatorInterface,
 	clientFactory jsonwire.ClientFactoryInterface,
 ) (pool.StrategyInterface, error) {
-	return &Strategy{storage, capsComparator, clientFactory}, nil
+	return &Strategy{
+		storage,
+		capsComparator,
+		clientFactory,
+		new(nodeHelperFactory),
+	}, nil
 }

jsonwire/node.go renamed to pool/strategy/persistent/nodeHelper.go

@@ -1,18 +1,21 @@
-package jsonwire
+package persistent
 
 import (
 	"fmt"
+	"github.com/qa-dev/jsonwire-grid/jsonwire"
 )
 
-type Node struct {
-	client ClientInterface
+type nodeHelperFactory struct{}
+
+func (f *nodeHelperFactory) create(abstractClient jsonwire.ClientInterface) sessionsRemover {
+	return &nodeHelper{client: abstractClient}
 }
 
-func NewNode(abstractClient ClientInterface) *Node {
-	return &Node{client: abstractClient}
+type nodeHelper struct {
+	client jsonwire.ClientInterface
 }
 
-func (n *Node) RemoveAllSessions() (int, error) {
+func (n *nodeHelper) removeAllSessions() (int, error) {
 	message, err := n.client.Sessions()
 	if err != nil {
 		return 0, fmt.Errorf("Can't get sessions, %s", err.Error())

pool/strategy/persistent/nodeHelper_test.go

@@ -0,0 +1,96 @@
+package persistent
+
+import (
+	"testing"
+	"github.com/stretchr/testify/assert"
+	"github.com/qa-dev/jsonwire-grid/jsonwire"
+	"encoding/json"
+	"github.com/stretchr/testify/mock"
+	"errors"
+)
+
+func TestNodeHelperFactory_create(t *testing.T) {
+	nhf := new(nodeHelperFactory)
+	nodeHelper := nhf.create(nil)
+	assert.NotNil(t, nodeHelper)
+}
+
+func TestNodeHelper_removeAllSessions_Positive_NothingToRemove(t *testing.T) {
+	cm := new(jsonwire.ClientMock)
+	nodeHelper := &nodeHelper{cm}
+	message := new(jsonwire.Sessions)
+	cm.On("Sessions").Return(message, nil)
+	_, err := nodeHelper.removeAllSessions()
+	assert.Nil(t, err)
+}
+
+func TestNodeHelper_removeAllSessions_Positive_SuccessRemove(t *testing.T) {
+	cm := new(jsonwire.ClientMock)
+	nodeHelper := &nodeHelper{cm}
+	sessions := new(jsonwire.Sessions)
+	sessions.Value = []struct {
+		ID           string          `json:"id"`
+		Capabilities json.RawMessage `json:"capabilities"`
+	}{
+		{ID: "lrololo", Capabilities: nil},
+	}
+	cm.On("Sessions").Return(sessions, nil)
+	message := new(jsonwire.Message)
+	cm.On("CloseSession", mock.AnythingOfType("string")).Return(message, nil)
+	_, err := nodeHelper.removeAllSessions()
+	assert.Nil(t, err)
+}
+
+func TestNodeHelper_removeAllSessions_Negative_Sessions_Error(t *testing.T) {
+	cm := new(jsonwire.ClientMock)
+	nodeHelper := &nodeHelper{cm}
+	cm.On("Sessions").Return(new(jsonwire.Sessions), errors.New("Err"))
+	_, err := nodeHelper.removeAllSessions()
+	assert.NotNil(t, err)
+}
+
+func TestNodeHelper_removeAllSessions_Negative_Sessions_MessageStatusNotOk(t *testing.T) {
+	cm := new(jsonwire.ClientMock)
+	nodeHelper := &nodeHelper{cm}
+	sessions := new(jsonwire.Sessions)
+	sessions.Status = 99999
+	cm.On("Sessions").Return(new(jsonwire.Sessions), errors.New("Err"))
+	_, err := nodeHelper.removeAllSessions()
+	assert.NotNil(t, err)
+}
+
+func TestNodeHelper_removeAllSessions_Negative_CloseSession_Error(t *testing.T) {
+	cm := new(jsonwire.ClientMock)
+	nodeHelper := &nodeHelper{cm}
+	sessions := new(jsonwire.Sessions)
+	sessions.Value = []struct {
+		ID           string          `json:"id"`
+		Capabilities json.RawMessage `json:"capabilities"`
+	}{
+		{ID: "lrololo", Capabilities: nil},
+	}
+	cm.On("Sessions").Return(sessions, nil)
+	message := new(jsonwire.Message)
+	cm.On("CloseSession", mock.AnythingOfType("string")).Return(message, errors.New("Err"))
+	_, err := nodeHelper.removeAllSessions()
+	assert.NotNil(t, err)
+}
+
+func TestNodeHelper_removeAllSessions_Negative_CloseSession_MessageStatusNotOk(t *testing.T) {
+	cm := new(jsonwire.ClientMock)
+	nodeHelper := &nodeHelper{cm}
+	sessions := new(jsonwire.Sessions)
+	sessions.Value = []struct {
+		ID           string          `json:"id"`
+		Capabilities json.RawMessage `json:"capabilities"`
+	}{
+		{ID: "lrololo", Capabilities: nil},
+	}
+	cm.On("Sessions").Return(sessions, nil)
+	message := new(jsonwire.Message)
+	message.Status = 999999
+	cm.On("CloseSession", mock.AnythingOfType("string")).Return(message, errors.New("Err"))
+	_, err := nodeHelper.removeAllSessions()
+	assert.NotNil(t, err)
+}
+

pool/strategy/persistent/strategy.go

@@ -9,10 +9,19 @@ import (
 	"github.com/qa-dev/jsonwire-grid/pool/strategy"
 )
 
+type sessionsRemoverFactory interface {
+	create(abstractClient jsonwire.ClientInterface) sessionsRemover
+}
+
+type sessionsRemover interface {
+	removeAllSessions() (int, error)
+}
+
 type Strategy struct {
-	storage        pool.StorageInterface
-	capsComparator capabilities.ComparatorInterface
-	clientFactory  jsonwire.ClientFactoryInterface
+	storage                pool.StorageInterface
+	capsComparator         capabilities.ComparatorInterface
+	clientFactory          jsonwire.ClientFactoryInterface
+	sessionsRemoverFactory sessionsRemoverFactory
 }
 
 func (s *Strategy) Reserve(desiredCaps capabilities.Capabilities) (pool.Node, error) {
@@ -43,10 +52,20 @@ func (s *Strategy) Reserve(desiredCaps capabilities.Capabilities) (pool.Node, er
 			}
 			continue
 		}
+
 		//todo: заменить магические числа на константы статусов
-		if message.Status == 0 { // status == ok
-			return node, nil
+		if message.Status != 0 { // status != ok
+			continue
 		}
+
+		seleniumNode := s.sessionsRemoverFactory.create(client)
+		//todo: посылать в мониторинг событие, если вернулся не 0
+		_, err = seleniumNode.removeAllSessions()
+		if err != nil {
+			return pool.Node{}, errors.New("remove all existing sessions from node, " + err.Error())
+		}
+
+		return node, nil
 	}
 
 	return pool.Node{}, strategy.ErrNotFound

pool/strategy/persistent/strategy_test.go

@@ -11,6 +11,23 @@ import (
 	"testing"
 )
 
+type sessionsRemoverMockFactory struct {
+	sessionsRemover sessionsRemover
+}
+
+func (f *sessionsRemoverMockFactory) create(abstractClient jsonwire.ClientInterface) sessionsRemover {
+	return f.sessionsRemover
+}
+
+type sessionsRemoverMock struct {
+	mock.Mock
+}
+
+func (r *sessionsRemoverMock) removeAllSessions() (int, error) {
+	args := r.Called()
+	return args.Int(0), args.Error(1)
+}
+
 func TestStrategy_Reserve_Positive(t *testing.T) {
 	sm := new(pool.StorageMock)
 	expectedNode := pool.Node{CapabilitiesList: []capabilities.Capabilities{{"cap1": "cal1"}}}
@@ -24,7 +41,10 @@ func TestStrategy_Reserve_Positive(t *testing.T) {
 	cfm.On("Create", mock.AnythingOfType("string")).Return(clm)
 	message := new(jsonwire.Message)
 	clm.On("Status").Return(message, nil)
-	s := Strategy{storage: sm, capsComparator: cm, clientFactory: cfm}
+	srm := new(sessionsRemoverMock)
+	srm.On("removeAllSessions").Return(0, nil)
+	srfm := &sessionsRemoverMockFactory{srm}
+	s := Strategy{storage: sm, capsComparator: cm, clientFactory: cfm, sessionsRemoverFactory: srfm}
 	node, err := s.Reserve(capabilities.Capabilities{})
 	assert.Nil(t, err)
 	assert.Equal(t, expectedNode, node)
@@ -95,6 +115,27 @@ func TestStrategy_Reserve_Negative_Client_Status_NotOk(t *testing.T) {
 	assert.NotNil(t, err)
 }
 
+func TestStrategy_Reserve_Negative_removeAllSessions_Error(t *testing.T) {
+	sm := new(pool.StorageMock)
+	sm.On("GetAll").Return([]pool.Node{{}}, nil)
+	cm := new(capabilities.ComparatorMock)
+	cm.On("Compare", mock.AnythingOfType("capabilities.Capabilities"), mock.AnythingOfType("capabilities.Capabilities")).Return(true)
+	expectedNode := pool.Node{CapabilitiesList: []capabilities.Capabilities{{"cap1": "cal1"}}}
+	sm.On("ReserveAvailable", mock.AnythingOfType("[]pool.Node")).Return([]pool.Node{expectedNode}, nil)
+	cfm := new(jsonwire.ClientFactoryMock)
+	clm := new(jsonwire.ClientMock)
+	cfm.On("Create", mock.AnythingOfType("string")).Return(clm)
+	message := new(jsonwire.Message)
+	clm.On("Status").Return(message, nil)
+	eError := errors.New("Error")
+	srm := new(sessionsRemoverMock)
+	srm.On("removeAllSessions").Return(0, eError)
+	srfm := &sessionsRemoverMockFactory{srm}
+	s := Strategy{storage: sm, capsComparator: cm, clientFactory: cfm, sessionsRemoverFactory: srfm}
+	_, err := s.Reserve(capabilities.Capabilities{})
+	assert.NotNil(t, err)
+}
+
 func TestStrategy_CleanUp_Positive(t *testing.T) {
 	sm := new(pool.StorageMock)
 	sm.On("SetAvailable", mock.AnythingOfType("pool.Node")).Return(nil)