Skip to content

Commit

Permalink
Check for nil pointer in the sources client (#340)
Browse files Browse the repository at this point in the history 
* Check for nil pointer in the sources client
* 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
  • Loading branch information
@dehort
dehort authored Mar 5, 2024
1 parent 3822d9f commit a74edfa
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 22 deletions.
54 changes: 32 additions & 22 deletions internal/api/connectors/sources/impl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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{
Expand All @@ -111,43 +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")
}

return res.JSON200.Data, err
source := (*res.JSON200.Data)[0]

if source.Id == nil {
return "", "", fmt.Errorf("GetSources did not return a valid sources id")
}

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
}

source := (*sourcesResponse)[0]

rhcConnectionResponse, err := this.getRHCConnectionStatus(ctx, string(*source.Id))
rhcId, availabilityStatus, err := this.getRHCConnectionStatus(ctx, sourceId)

if err != nil {
return SourceConnectionStatus{}, err
}

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
}
61 changes: 61 additions & 0 deletions internal/api/connectors/sources/sources_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,51 @@ var _ = Describe("Sources", func() {
Expect(err.Error()).To(ContainSubstring("Source Bad Request"))
})

It("interperates response correctly if getSources returns an empty response", func() {
responses := []test.MockHttpResponse{
{StatusCode: 200, Body: `{}`},
{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 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"}]}`},
Expand Down Expand Up @@ -100,5 +145,21 @@ var _ = Describe("Sources", func() {
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("RHCStatus Bad Request"))
})

It("interperates response correctly if getRhcConnectionStatus returns an empty response", func() {
responses := []test.MockHttpResponse{
{StatusCode: 200, Body: `{"data": [{"id": "1", "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("GetRHCConnectionStatus returned an empty response"))
})

})
})

0 comments on commit a74edfa

Please sign in to comment.