-
Notifications
You must be signed in to change notification settings - Fork 8
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: do not try to notify if required data are missing
Adds a check which controls if notification will happen or not. The check validates presence of the most required parts and creates a basis for adding more. Now it checks for: - having at least one sample to send - HostInfo present and containing HostID and ExternalOrganization Signed-off-by: Petr Vobornik <[email protected]>
- Loading branch information
Showing
4 changed files
with
190 additions
and
9 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package notify | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/prometheus/prometheus/prompb" | ||
|
|
||
| "github.com/RedHatInsights/host-metering/hostinfo" | ||
| ) | ||
|
|
||
| type NotifyPolicy interface { | ||
| ShouldNotify([]prompb.Sample, *hostinfo.HostInfo) error | ||
| } | ||
|
|
||
| type GeneralNotifyPolicy struct{} | ||
|
|
||
| func (p *GeneralNotifyPolicy) ShouldNotify(samples []prompb.Sample, hostinfo *hostinfo.HostInfo) error { | ||
|
|
||
| count := len(samples) | ||
| if count == 0 { | ||
| return fmt.Errorf("no samples to send") | ||
| } | ||
|
|
||
| if hostinfo == nil { | ||
| return fmt.Errorf("missing internal HostInfo") | ||
| } | ||
|
|
||
| if hostinfo.HostId == "" { | ||
| return fmt.Errorf("missing HostId") | ||
| } | ||
|
|
||
| if hostinfo.ExternalOrganization == "" { | ||
| return fmt.Errorf("missing ExternalOrganization") | ||
| } | ||
|
|
||
| return nil | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| package notify | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/RedHatInsights/host-metering/hostinfo" | ||
| "github.com/prometheus/prometheus/prompb" | ||
| ) | ||
|
|
||
| type ShouldNotifyTestCase struct { | ||
| name string | ||
| samples []prompb.Sample | ||
| hostInfo *hostinfo.HostInfo | ||
| expected string | ||
| } | ||
|
|
||
| func TestGeneralNotifyPolicy(t *testing.T) { | ||
|
|
||
| p := &GeneralNotifyPolicy{} | ||
| correctSamples := []prompb.Sample{{}} | ||
| correctHostInfo := fullyDefinedMockHostInfo() | ||
|
|
||
| emptyHostIdHI := fullyDefinedMockHostInfo() | ||
| emptyHostIdHI.HostId = "" | ||
|
|
||
| emptyExternalOrganizationHI := fullyDefinedMockHostInfo() | ||
| emptyExternalOrganizationHI.ExternalOrganization = "" | ||
|
|
||
| testCases := []ShouldNotifyTestCase{ | ||
| { | ||
| name: "No samples", | ||
| samples: []prompb.Sample{}, | ||
| hostInfo: correctHostInfo, | ||
| expected: "no samples to send", | ||
| }, | ||
| { | ||
| name: "Nil hostInfo", | ||
| samples: correctSamples, | ||
| hostInfo: nil, | ||
| expected: "missing internal HostInfo", | ||
| }, | ||
| { | ||
| name: "Empty HostId", | ||
| samples: correctSamples, | ||
| hostInfo: emptyHostIdHI, | ||
| expected: "missing HostId", | ||
| }, | ||
| { | ||
| name: "Empty ExternalOrganization", | ||
| samples: correctSamples, | ||
| hostInfo: emptyExternalOrganizationHI, | ||
| expected: "missing ExternalOrganization", | ||
| }, | ||
| } | ||
|
|
||
| t.Run("Test ShouldNotify", func(t *testing.T) { | ||
|
|
||
| for _, tc := range testCases { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| // when | ||
| err := p.ShouldNotify(tc.samples, tc.hostInfo) | ||
|
|
||
| // then | ||
| expectErrorContains(t, err, tc.expected) | ||
| }) | ||
| } | ||
|
|
||
| t.Run("Possitive", func(t *testing.T) { | ||
| // when | ||
| err := p.ShouldNotify(correctSamples, correctHostInfo) | ||
|
|
||
| // then | ||
| if err != nil { | ||
| t.Fatalf("expected no error, got '%s'", err.Error()) | ||
| } | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
| func fullyDefinedMockHostInfo() *hostinfo.HostInfo { | ||
| return &hostinfo.HostInfo{ | ||
| HostId: "hostid", | ||
| ExternalOrganization: "externalorganization", | ||
| SocketCount: "socketcount", | ||
| Product: "product", | ||
| Support: "support", | ||
| Usage: "usage", | ||
| Billing: fullyDefinedMockBillingInfo(), | ||
| } | ||
| } | ||
|
|
||
| func fullyDefinedMockBillingInfo() hostinfo.BillingInfo { | ||
| return hostinfo.BillingInfo{ | ||
| Model: "model", | ||
| Marketplace: "marketplace", | ||
| MarketplaceAccount: "marketplaceaccount", | ||
| MarketplaceInstanceId: "marketplaceinstanceid", | ||
| } | ||
| } | ||
|
|
||
| func expectErrorContains(t *testing.T, err error, expected string) { | ||
| t.Helper() | ||
| if err == nil { | ||
| t.Fatalf("expected error, got nil") | ||
| } | ||
| if !strings.Contains(err.Error(), expected) { | ||
| t.Fatalf("expected error to contain '%s', got '%s'", expected, err.Error()) | ||
| } | ||
| } |