Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion backend/pkg/httpserver/create_notification_channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,12 @@ func validateNotificationChannel(input *backend.CreateNotificationChannelRequest
if err := httputils.ValidateSlackWebhookURL(cfg.Url); err != nil {
fieldErrors.addFieldError("config.url", err)
}
} else if cfg, err := input.Config.AsRSSConfig(); err == nil && cfg.Type == backend.RSSConfigTypeRss {
// RSS channels currently have no configuration fields to validate.
_ = cfg
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's time we leverage the switch case (and get coverage from the exhaustive) linter. Feel free to do this in this PR or a follow up:

func validateUpdateNotificationChannel(request *backend.UpdateNotificationChannelRequest) fieldErrors {
	var fieldErrors &fieldValidationErrors{fieldErrorMap: nil} 

	if len(input.Name) < notificationChannelNameMinLength || len(input.Name) > notificationChannelNameMaxLength {
		fieldErrors.addFieldError("name", errNotificationChannelInvalidNameLength)
	}

	// 1. Get the explicit type discriminator from the JSON
	discriminator, err := request.Config.Discriminator()
	if err != nil {
		fieldErrors.addFieldError("config", errors.New("invalid configuration structure"))
		return fieldErrors
	}

	// 2. Switch on the type (cast to enum for exhaustiveness and type safety)
	switch backend.NotificationChannelType(discriminator) {

	case backend.NotificationChannelTypeRss:
		// Valid! RSS channels currently have no configuration fields to validate.
		// No variable is created, so no unused variable issue.

	case backend.NotificationChannelTypeWebhook:
		cfg, err := request.Config.AsWebhookConfig()
		if err != nil {
			fieldErrors.addFieldError("config", errors.New("invalid webhook config"))
			break
		}
		if err := httputils.ValidateSlackWebhookURL(cfg.Url); err != nil {
			fieldErrors.addFieldError("config.url", err)
		}

	case backend.NotificationChannelTypeEmail:
		// Explicitly fall through to default since email channels
		// cannot be managed manually via this endpoint.
		fallthrough

	default:
		fieldErrors.addFieldError("config", errors.New("invalid config: only webhook or rss updates are supported"))
	}

	if fieldErrors.hasErrors() {
		return fieldErrors
	}

	return nil
}

} else {
fieldErrors.addFieldError("config", errors.New("invalid config: only webhook channels can be created manually"))
fieldErrors.addFieldError("config",
errors.New("invalid config: only webhook or rss channels can be created manually"))
}

if fieldErrors.hasErrors() {
Expand Down
47 changes: 46 additions & 1 deletion backend/pkg/httpserver/create_notification_channel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,51 @@ func TestCreateNotificationChannel(t *testing.T) {
"updated_at": "2000-01-01T00:00:00Z"
}`),
},
{
name: "success rss",
requestBody: `
{
"name": "My RSS Feed",
"config": {
"type": "rss"
}
}`,
storerCfg: &MockCreateNotificationChannelConfig{
expectedUserID: testUser.ID,
expectedRequest: backend.CreateNotificationChannelRequest{
Name: "My RSS Feed",
Config: newTestCreateNotificationChannelConfig(t, backend.RSSConfig{
Type: backend.RSSConfigTypeRss,
}),
},
output: &backend.NotificationChannelResponse{
Id: "channel456",
Name: "My RSS Feed",
Type: backend.NotificationChannelResponseTypeRss,
Config: newTestNotificationChannelConfig(t, backend.RSSConfig{
Type: backend.RSSConfigTypeRss,
}),
Status: backend.NotificationChannelStatusEnabled,
CreatedAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
UpdatedAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
},
err: nil,
},
expectedCallCount: 1,
expectedResponse: testJSONResponse(201, `
{
"id": "channel456",
"name": "My RSS Feed",
"type": "rss",
"config": {
"type": "rss"
},
"status": "enabled",
"created_at": "2000-01-01T00:00:00Z",
"updated_at": "2000-01-01T00:00:00Z"
}`),
},

{
name: "reject email config",
// Attempt to create an email channel manually should be rejected.
Expand All @@ -106,7 +151,7 @@ func TestCreateNotificationChannel(t *testing.T) {
"code": 400,
"message": "input validation errors",
"errors": {
"config": "invalid config: only webhook channels can be created manually"
"config": "invalid config: only webhook or rss channels can be created manually"
}
}`),
},
Expand Down
10 changes: 9 additions & 1 deletion backend/pkg/httpserver/update_notification_channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,17 @@ func validateUpdateNotificationChannel(request *backend.UpdateNotificationChanne
if err := httputils.ValidateSlackWebhookURL(cfg.Url); err != nil {
fieldErrors.addFieldError("config.url", err)
}
} else if cfg, err := request.Config.AsRSSConfig(); err == nil &&
cfg.Type == backend.RSSConfigTypeRss {
// RSS channels currently have no configuration fields to validate.
_ = cfg
} else {
Comment on lines +52 to 56
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See other note

fieldErrors.addFieldError("config", errors.New("invalid config: only webhook updates are supported"))
fieldErrors.addFieldError(
"config",
errors.New("invalid config: only webhook or rss updates are supported"),
)
}

}
}

Expand Down
61 changes: 60 additions & 1 deletion backend/pkg/httpserver/update_notification_channel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,65 @@ func TestUpdateNotificationChannel_Restrictions(t *testing.T) {
},
expectedUpdateCount: 1,
},

{
name: "success rss update",
requestBody: `
{
"name": "Updated RSS",
"update_mask": ["name"]
}`,
expectedStatus: 200,
expectedResponseBody: `
{
"id": "channel123",
"name": "Updated RSS",
"type": "rss",
"config": {
"type": "rss"
},
"status": "enabled",
"created_at": "2000-01-01T00:00:00Z",
"updated_at": "2000-01-01T00:00:00Z"
}`,
expectFetch: true,
expectedGetOutput: &backend.NotificationChannelResponse{
Id: "channel123",
Name: "Old RSS",
Type: backend.NotificationChannelResponseTypeRss,
Config: newTestNotificationChannelConfig(t, backend.RSSConfig{
Type: backend.RSSConfigTypeRss,
}),
Status: backend.NotificationChannelStatusEnabled,
CreatedAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
UpdatedAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
},
updateStorerCfg: &MockUpdateNotificationChannelConfig{
expectedUserID: testUser.ID,
expectedChannelID: "channel123",
expectedRequest: backend.UpdateNotificationChannelRequest{
Config: nil,
Name: new("Updated RSS"),
UpdateMask: []backend.UpdateNotificationChannelRequestUpdateMask{
backend.UpdateNotificationChannelRequestMaskName,
},
},
output: &backend.NotificationChannelResponse{
Id: "channel123",
Name: "Updated RSS",
Type: backend.NotificationChannelResponseTypeRss,
Config: newTestNotificationChannelConfig(t, backend.RSSConfig{
Type: backend.RSSConfigTypeRss,
}),
Status: backend.NotificationChannelStatusEnabled,
CreatedAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
UpdatedAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
},
err: nil,
},
expectedUpdateCount: 1,
},

{
name: "reject update to existing email channel (rename)",
requestBody: `
Expand Down Expand Up @@ -159,7 +218,7 @@ func TestUpdateNotificationChannel_Restrictions(t *testing.T) {
"code": 400,
"message": "input validation errors",
"errors": {
"config": "invalid config: only webhook updates are supported"
"config": "invalid config: only webhook or rss updates are supported"
}
}`,
expectFetch: false,
Expand Down
10 changes: 9 additions & 1 deletion lib/gcpspanner/notification_channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ type NotificationChannelType string
const (
NotificationChannelTypeEmail NotificationChannelType = "email"
NotificationChannelTypeWebhook NotificationChannelType = "webhook"
NotificationChannelTypeRSS NotificationChannelType = "rss"
)

func getAllNotificationTypes() []NotificationChannelType {
Expand All @@ -76,6 +77,7 @@ func getAllNotificationTypes() []NotificationChannelType {
types := map[NotificationChannelType]any{
NotificationChannelTypeEmail: nil,
NotificationChannelTypeWebhook: nil,
NotificationChannelTypeRSS: nil,
}

ret := make([]NotificationChannelType, 0, len(types))
Expand All @@ -84,7 +86,6 @@ func getAllNotificationTypes() []NotificationChannelType {
}

return ret

}

// CreateNotificationChannelRequest is the request to create a channel.
Expand Down Expand Up @@ -175,6 +176,8 @@ func (c *NotificationChannel) toSpanner() *spannerNotificationChannel {
configData = c.EmailConfig
case NotificationChannelTypeWebhook:
configData = c.WebhookConfig
case NotificationChannelTypeRSS:
configData = nil
}

var config spanner.NullJSON
Expand All @@ -201,6 +204,8 @@ func (sc *spannerNotificationChannel) toPublic() (*NotificationChannel, error) {
channelType = NotificationChannelTypeEmail
case string(NotificationChannelTypeWebhook):
channelType = NotificationChannelTypeWebhook
case string(NotificationChannelTypeRSS):
channelType = NotificationChannelTypeRSS
default:
return nil, fmt.Errorf("unknown notification channel type '%s'", sc.Type)
}
Expand Down Expand Up @@ -258,6 +263,9 @@ func loadSubscriptionConfigs(
}
ret.WebhookConfig = &webhookConfig
ret.EmailConfig = nil
case NotificationChannelTypeRSS:
ret.WebhookConfig = nil
ret.EmailConfig = nil
}

return ret, nil
Expand Down
31 changes: 23 additions & 8 deletions lib/gcpspanner/spanneradapters/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"log/slog"
"math/big"
"slices"
Expand Down Expand Up @@ -188,8 +189,13 @@ func (s *Backend) ListSavedSearchNotificationEvents(ctx context.Context,
for _, e := range notifEvents {
var summaryBytes []byte
if e.Summary.Valid {
summaryBytes, _ = json.Marshal(e.Summary.Value)
var err error
summaryBytes, err = json.Marshal(e.Summary.Value)
if err != nil {
return nil, fmt.Errorf("failed to marshal summary for event %s: %w", e.ID, err)
}
}

events = append(events, backendtypes.SavedSearchNotificationEvent{
ID: e.ID,
SavedSearchID: e.SavedSearchID,
Expand Down Expand Up @@ -598,6 +604,8 @@ func (s *Backend) CreateNotificationChannel(ctx context.Context,
if cfg, err := req.Config.AsWebhookConfig(); err == nil && cfg.Type == backend.Webhook {
channelType = gcpspanner.NotificationChannelTypeWebhook
spannerWebhookConfig = s.toSpannerWebhookConfig(&cfg)
} else if cfg, err := req.Config.AsRSSConfig(); err == nil && cfg.Type == backend.RSSConfigTypeRss {
channelType = gcpspanner.NotificationChannelTypeRSS
} else {
return nil, errors.New("invalid notification channel request: missing or invalid config")
}
Expand Down Expand Up @@ -677,9 +685,14 @@ func (s *Backend) UpdateNotificationChannel(
updateReq.WebhookConfig.Value = &gcpspanner.WebhookConfig{
URL: cfg.Url,
}
} else if cfg, err := req.Config.AsRSSConfig(); err == nil &&
cfg.Type == backend.RSSConfigTypeRss {
updateReq.Type.IsSet = true
updateReq.Type.Value = gcpspanner.NotificationChannelTypeRSS
} else {
return nil, errors.New("invalid notification channel update: unsupported config type")
}

}
}

Expand Down Expand Up @@ -708,6 +721,8 @@ func getChannelSortKey(channel gcpspanner.NotificationChannel) string {
if channel.WebhookConfig != nil {
return channel.WebhookConfig.URL
}
case gcpspanner.NotificationChannelTypeRSS:
// RSS channels don't have a specific configuration field to use as a sort key.
}

return "" // Default sort key if type is unknown or has no specific key.
Expand All @@ -717,7 +732,6 @@ func getChannelSortKey(channel gcpspanner.NotificationChannel) string {
// notification channel to backend notification channel.
func toBackendNotificationChannel(channel *gcpspanner.NotificationChannel) *backend.NotificationChannelResponse {
if channel == nil {

return nil
}

Expand All @@ -726,22 +740,23 @@ func toBackendNotificationChannel(channel *gcpspanner.NotificationChannel) *back
switch channel.Type {
case gcpspanner.NotificationChannelTypeEmail:
if channel.EmailConfig != nil {
bytes, _ := json.Marshal(backend.EmailConfig{
_ = config.FromEmailConfig(backend.EmailConfig{
Type: backend.EmailConfigTypeEmail,
Address: openapi_types.Email(channel.EmailConfig.Address),
})
// UnmarshalJSON() is confusingly named - it just makes a copy of 'bytes' to store in config.
_ = config.UnmarshalJSON(bytes)
}
case gcpspanner.NotificationChannelTypeWebhook:
if channel.WebhookConfig != nil {
bytes, _ := json.Marshal(backend.WebhookConfig{
_ = config.FromWebhookConfig(backend.WebhookConfig{
Type: backend.Webhook,
Url: channel.WebhookConfig.URL,
})
// UnmarshalJSON() is confusingly named - it just makes a copy of 'bytes' to store in config.
_ = config.UnmarshalJSON(bytes)
}
case gcpspanner.NotificationChannelTypeRSS:
_ = config.FromRSSConfig(backend.RSSConfig{
Type: backend.RSSConfigTypeRss,
})

}

return &backend.NotificationChannelResponse{
Expand Down
Loading