Move more of the error handling logic up into the call that makes the

rest request...trying to simplify the core logic of the sources connector
RedHatInsights · Mar 4, 2024 · 0a57c11 · 0a57c11
1 parent 02b7b55
commit 0a57c11
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 30 deletions.
diff --git a/internal/api/connectors/sources/impl.go b/internal/api/connectors/sources/impl.go
@@ -69,7 +69,7 @@ func NewSourcesClient(cfg *viper.Viper) SourcesConnector {
 	return NewSourcesClientWithHttpRequestDoer(cfg, &doer)
 }
 
-func (this *sourcesClientImpl) getRHCConnectionStatus(ctx context.Context, sourceId string) (*RhcConnectionCollection, error) {
+func (this *sourcesClientImpl) getRHCConnectionStatus(ctx context.Context, sourceId string) (*string, *string, error) {
 
 	utils.GetLogFromContext(ctx).Debugw("Sending Sources RHC Connection Request")
 
@@ -80,28 +80,32 @@ func (this *sourcesClientImpl) getRHCConnectionStatus(ctx context.Context, sourc
 	res, err := this.client.GetSourcesRhcConnectionWithResponse(ctx, ID, &params)
 
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	if res.HTTPResponse.StatusCode == 404 {
-		return nil, fmt.Errorf("RHCStatus Not Found")
+		return nil, nil, fmt.Errorf("RHCStatus Not Found")
 	}
 
 	if res.HTTPResponse.StatusCode == 400 {
-		return nil, fmt.Errorf("RHCStatus Bad Request")
+		return nil, nil, fmt.Errorf("RHCStatus Bad Request")
 	}
 
 	if res.JSON200 == nil {
-		return nil, fmt.Errorf(`GetRhcConnectionStatus unexpected status code "%d" or content type "%s"`, res.HTTPResponse.StatusCode, res.HTTPResponse.Header.Get("content-type"))
+		return nil, nil, fmt.Errorf(`GetRhcConnectionStatus unexpected status code "%d" or content type "%s"`, res.HTTPResponse.StatusCode, res.HTTPResponse.Header.Get("content-type"))
 	}
 
-	return res.JSON200, err
+	if res.JSON200.Data == nil || len(*res.JSON200.Data) == 0 {
+		return nil, nil, fmt.Errorf("GetRHCConnectionStatus returned an empty response")
+	}
+
+	return (*res.JSON200.Data)[0].RhcId, (*res.JSON200.Data)[0].AvailabilityStatus, err
 }
 
-func (this *sourcesClientImpl) getSources(ctx context.Context, sourceId string) (sources *[]Source, err error) {
+func (this *sourcesClientImpl) getSourceIdBySatelliteId(ctx context.Context, satelliteId string) (sourceId string, sourceName string, err error) {
 	utils.GetLogFromContext(ctx).Debugw("Sending Sources Request")
 
-	ID := ID(sourceId)
+	ID := ID(satelliteId)
 	queryFilter := filterPath + QueryFilter(ID)
 
 	params := &ListSourcesParams{
@@ -111,51 +115,49 @@ func (this *sourcesClientImpl) getSources(ctx context.Context, sourceId string)
 	res, err := this.client.ListSourcesWithResponse(ctx, params)
 
 	if err != nil {
-		return nil, err
+		return "", "", err
 	}
 
 	if res.JSON400 != nil {
-		return nil, fmt.Errorf("Source Bad Request")
+		return "", "", fmt.Errorf("Source Bad Request")
 	}
 
 	if res.JSON200 == nil {
-		return nil, fmt.Errorf(`GetSources unexpected status code "%d" or content type "%s"`, res.HTTPResponse.StatusCode, res.HTTPResponse.Header.Get("content-type"))
+		return "", "", fmt.Errorf(`GetSources unexpected status code "%d" or content type "%s"`, res.HTTPResponse.StatusCode, res.HTTPResponse.Header.Get("content-type"))
+	}
+
+	if res.JSON200.Data == nil || len(*res.JSON200.Data) == 0 {
+		return "", "", fmt.Errorf("GetSources returned an empty response")
+	}
+
+	source := (*res.JSON200.Data)[0]
+
+	if source.Id == nil {
+		return "", "", fmt.Errorf("GetSources did not return a valid sources id")
 	}
 
-	return res.JSON200.Data, err
+	return string(*source.Id), *source.Name, nil
 }
 
 func (this *sourcesClientImpl) GetSourceConnectionDetails(ctx context.Context, sourceID string) (details SourceConnectionStatus, err error) {
 	utils.GetLogFromContext(ctx).Debugw("Gathering Source Connection Details")
 
-	sourcesResponse, err := this.getSources(ctx, sourceID)
+	sourceId, sourceName, err := this.getSourceIdBySatelliteId(ctx, sourceID)
 
 	if err != nil {
 		return SourceConnectionStatus{}, err
 	}
 
-	if sourcesResponse == nil {
-		return SourceConnectionStatus{}, fmt.Errorf("GetSources returned an empty response")
-	}
-
-	source := (*sourcesResponse)[0]
-
-	rhcConnectionResponse, err := this.getRHCConnectionStatus(ctx, string(*source.Id))
+	rhcId, availabilityStatus, err := this.getRHCConnectionStatus(ctx, sourceId)
 
 	if err != nil {
 		return SourceConnectionStatus{}, err
 	}
 
-	if rhcConnectionResponse == nil || rhcConnectionResponse.Data == nil {
-		return SourceConnectionStatus{}, fmt.Errorf("GetRHCConnectionStatus returned an empty response")
-	}
-
-	rhcInfo := (*rhcConnectionResponse).Data
-
 	return SourceConnectionStatus{
-		ID:                 string(*source.Id),
-		SourceName:         source.Name,
-		RhcID:              (*rhcInfo)[0].RhcId,
-		AvailabilityStatus: (*rhcInfo)[0].AvailabilityStatus,
+		ID:                 sourceId,
+		SourceName:         &sourceName,
+		RhcID:              rhcId,
+		AvailabilityStatus: availabilityStatus,
 	}, err
 }
diff --git a/internal/api/connectors/sources/sources_test.go b/internal/api/connectors/sources/sources_test.go
@@ -86,6 +86,36 @@ var _ = Describe("Sources", func() {
 			Expect(err.Error()).To(ContainSubstring("GetSources returned an empty response"))
 		})
 
+		It("interperates response correctly if getSources returns a response where an entry does not have an id", func() {
+			responses := []test.MockHttpResponse{
+				{StatusCode: 200, Body: `{"data": [{"name": "test", "availability_status": "connected"}]}`},
+				{StatusCode: 200, Body: `{}`},
+			}
+
+			doer := test.MockMultiResponseHttpClient(responses...)
+			client := NewSourcesClientWithHttpRequestDoer(config.Get(), doer)
+			ctx := test.TestContext()
+
+			_, err := client.GetSourceConnectionDetails(ctx, "4f37c752-ba1c-48b1-bcf7-4ef8f585d9ee")
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("GetSources did not return a valid sources id"))
+		})
+
+		It("interperates response correctly if getSources returns a response where the data array is empty", func() {
+			responses := []test.MockHttpResponse{
+				{StatusCode: 200, Body: `{"data": []}`},
+				{StatusCode: 200, Body: `{}`},
+			}
+
+			doer := test.MockMultiResponseHttpClient(responses...)
+			client := NewSourcesClientWithHttpRequestDoer(config.Get(), doer)
+			ctx := test.TestContext()
+
+			_, err := client.GetSourceConnectionDetails(ctx, "4f37c752-ba1c-48b1-bcf7-4ef8f585d9ee")
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("GetSources returned an empty response"))
+		})
+
 		It("interperates response correctly if getRhcConnectionStatus returns a 404", func() {
 			responses := []test.MockHttpResponse{
 				{StatusCode: 200, Body: `{"data": [{"id": "1", "name": "test", "availability_status": "connected"}]}`},