Merge pull request #14 from Comcast/hotfix/tuneResponseBehavior

The status codes returned by tr1d1um should align much more to those …

src/tr1d1um/communication.go

@@ -94,21 +94,28 @@ func (tr1 *Tr1SendAndHandle) HandleResponse(ch *ConversionHandler, err error, re
 	}
 
 	if respFromServer.StatusCode != http.StatusOK {
-		writeResponse("Non-200 Response From Server", respFromServer.StatusCode, origin)
+		var bodyResp []byte
+		if responseBody, err := ioutil.ReadAll(respFromServer.Body); err == nil {
+			bodyResp = responseBody
+		}
+
+		origin.WriteHeader(respFromServer.StatusCode)
+		origin.Write(bodyResp)
 		errorLogger.Log(logging.MessageKey(), "non-200 response from server", logging.ErrorKey(), respFromServer.Status)
 		return
 	}
 
-	// Do not attempt extracting payload, forward whole body
-	if wholeBody {
+	if wholeBody { // Do not attempt extracting payload, forward whole body
 		err = forwardInput(origin, respFromServer.Body)
 		ReportError(err, origin)
 		return
 	}
 
-	if responsePayload, err := ch.encodingHelper.ExtractPayload(respFromServer.Body, wrp.JSON); err == nil {
-		origin.WriteHeader(http.StatusOK)
-		origin.Write(responsePayload)
+	if RDKResponse, err := ch.encodingHelper.ExtractPayload(respFromServer.Body, wrp.JSON); err == nil {
+		if RDKRespCode, err := GetStatusCodeFromRDKResponse(RDKResponse); err == nil && RDKRespCode != http.StatusInternalServerError {
+			origin.WriteHeader(RDKRespCode)
+		}
+		origin.Write(RDKResponse)
 	} else {
 		ReportError(err, origin)
 		errorLogger.Log(logging.ErrorKey(), err)

src/tr1d1um/communication_test.go

@@ -139,9 +139,12 @@ func TestHandleResponse(t *testing.T) {
 
 	t.Run("StatusNotOK", func(t *testing.T) {
 		recorder := httptest.NewRecorder()
-		fakeResponse := &http.Response{StatusCode: http.StatusBadRequest}
+		var mockBody bytes.Buffer
+		mockBody.WriteString("expectMe")
+		fakeResponse := &http.Response{StatusCode: http.StatusBadRequest, Body: ioutil.NopCloser(&mockBody)}
 		tr1.HandleResponse(nil, nil, fakeResponse, recorder, false)
 		assert.EqualValues(http.StatusBadRequest, recorder.Code)
+		assert.EqualValues("expectMe", recorder.Body.String())
 	})
 
 	t.Run("ExtractPayloadFail", func(t *testing.T) {
@@ -166,19 +169,6 @@ func TestHandleResponse(t *testing.T) {
 		mockEncoding.AssertExpectations(t)
 	})
 
-	t.Run("IdealCase", func(t *testing.T) {
-		fakeResponse := &http.Response{StatusCode: http.StatusOK}
-		extractedData := []byte("extract")
-
-		mockEncoding.On("ExtractPayload", fakeResponse.Body, wrp.JSON).Return(extractedData, nil).Once()
-		recorder := httptest.NewRecorder()
-		tr1.HandleResponse(ch, nil, fakeResponse, recorder, false)
-
-		assert.EqualValues(http.StatusOK, recorder.Code)
-		assert.EqualValues(extractedData, recorder.Body.Bytes())
-		mockEncoding.AssertExpectations(t)
-	})
-
 	t.Run("IdealReadEntireBody", func(t *testing.T) {
 		var fakeBody bytes.Buffer
 		bodyString := "bodyString"
@@ -193,6 +183,33 @@ func TestHandleResponse(t *testing.T) {
 		assert.EqualValues(bodyString, recorder.Body.String())
 		mockEncoding.AssertNotCalled(t, "ExtractPayload", fakeResponse.Body, wrp.JSON)
 	})
+
+	t.Run("GoodRDKResponse", func(t *testing.T) {
+		fakeResponse := &http.Response{StatusCode: http.StatusOK}
+		extractedData := []byte(`{"statusCode": 202}`)
+
+		mockEncoding.On("ExtractPayload", fakeResponse.Body, wrp.JSON).Return(extractedData, nil).Once()
+		recorder := httptest.NewRecorder()
+		tr1.HandleResponse(ch, nil, fakeResponse, recorder, false)
+
+		assert.EqualValues(202, recorder.Code)
+		assert.EqualValues(extractedData, recorder.Body.Bytes())
+		mockEncoding.AssertExpectations(t)
+	})
+
+	t.Run("BadRDKResponse", func(t *testing.T) {
+		fakeResponse := &http.Response{StatusCode: http.StatusOK}
+		extractedData := []byte(`{"statusCode": 500}`)
+
+		mockEncoding.On("ExtractPayload", fakeResponse.Body, wrp.JSON).Return(extractedData, nil).Once()
+		recorder := httptest.NewRecorder()
+		tr1.HandleResponse(ch, nil, fakeResponse, recorder, false)
+
+		assert.EqualValues(http.StatusOK, recorder.Code) // reflect transaction instead of device status
+		assert.EqualValues(extractedData, recorder.Body.Bytes())
+		mockEncoding.AssertExpectations(t)
+	})
+
 }
 
 func TestPerformRequest(t *testing.T) {
@@ -276,6 +293,11 @@ func TestForwardInput(t *testing.T) {
 	})
 }
 
+func TestGetRespTimeout(t *testing.T) {
+	assert := assert.New(t)
+	tr1 := &Tr1SendAndHandle{respTimeout: time.Second * 3}
+	assert.EqualValues(time.Second*3, tr1.GetRespTimeout())
+}
 func NewHTTPRequestFail(_, _ string, _ io.Reader) (*http.Request, error) {
 	return nil, errors.New(errMsg)
 }

src/tr1d1um/tr1d1um_response.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"net/http"
 
+	"encoding/json"
+
 	"github.com/Comcast/webpa-common/wrp"
 )
 
@@ -13,11 +15,10 @@ const (
 	Tr1StatusTimeout = 503
 )
 
-//TR1Response will serve as the struct to read in
-//results coming from a server we ping
-type TR1Response struct {
-	Message    string `json:"message,omitempty"`
-	StatusCode int    `json:"code"`
+//RDKResponse will serve as the struct to read in
+//results coming from some RDK device
+type RDKResponse struct {
+	StatusCode int `json:"StatusCode"`
 }
 
 //writeResponse is a tiny helper function that passes responses (In Json format only for now)
@@ -28,7 +29,7 @@ func writeResponse(message string, statusCode int, w http.ResponseWriter) {
 	w.Write([]byte(fmt.Sprintf(`{"message":"%s"}`, message)))
 }
 
-//ReportError writes back to the caller responses based on the given error. Error must be non-nil
+//ReportError writes back to the caller responses based on the given error. Error must be non-nil to be reported.
 func ReportError(err error, w http.ResponseWriter) {
 	if err == nil {
 		return
@@ -39,3 +40,14 @@ func ReportError(err error, w http.ResponseWriter) {
 	}
 	writeResponse(message, statusCode, w)
 }
+
+//GetStatusCodeFromRDKResponse returns the status code given a well-formatted
+//RDK response. Otherwise, it defaults to 500 as code and returns an error
+func GetStatusCodeFromRDKResponse(RDKPayload []byte) (statusCode int, err error) {
+	RDKResp := new(RDKResponse)
+	statusCode = http.StatusInternalServerError
+	if err = json.Unmarshal(RDKPayload, RDKResp); err == nil {
+		statusCode, err = RDKResp.StatusCode, nil
+	}
+	return
+}

src/tr1d1um/tr1d1um_response_test.go

@@ -41,3 +41,22 @@ func TestReportError(t *testing.T) {
 		assert.EqualValues(`{"message":"Error Timeout"}`, origin.Body.String())
 	})
 }
+
+func TestGetStatusCodeFromRDKResponse(t *testing.T) {
+	t.Run("IdealRDKResponse", func(t *testing.T) {
+		assert := assert.New(t)
+
+		RDKResponse := []byte(`{"statusCode": 200}`)
+		statusCode, err := GetStatusCodeFromRDKResponse(RDKResponse)
+		assert.EqualValues(200, statusCode)
+		assert.Nil(err)
+	})
+
+	t.Run("InvalidRDKResponse", func(t *testing.T) {
+		assert := assert.New(t)
+
+		statusCode, err := GetStatusCodeFromRDKResponse(nil)
+		assert.EqualValues(500, statusCode)
+		assert.NotNil(err)
+	})
+}