Skip to content

Commit

Permalink
Improve error checking code and fix sql script (#4041)
Browse files Browse the repository at this point in the history
  • Loading branch information
javfg authored Jul 11, 2023
1 parent 6997a51 commit b38215d
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 20 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/notifications-changes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Change: Clean up notifications error checking code, fix sql creation script

https://github.com/cs3org/reva/pull/4041
15 changes: 7 additions & 8 deletions internal/serverless/services/notifications/notifications.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,13 @@ func (s *svc) handleMsgTemplate(msg []byte) {

name, err := s.templates.Put(msg, s.handlers)
if err != nil {
s.log.Error().Err(err).Msgf("template registration failed %v", err)
s.log.Error().Err(err).Msg("template registration failed")

// If a template file was not found, delete that template from the registry altogether,
// this way we ensure templates that are deleted from the config are deleted from the
// store too.
wrappedErr := errors.Unwrap(errors.Unwrap(err))
_, isFileNotFoundError := wrappedErr.(*template.FileNotFoundError)
if isFileNotFoundError && name != "" {
var e *template.FileNotFoundError
if errors.As(err, &e) && name != "" {
err := s.kv.Purge(name)
if err != nil {
s.log.Error().Err(err).Msgf("deletion of template %s from store failed", name)
Expand Down Expand Up @@ -274,8 +273,8 @@ func (s *svc) handleMsgUnregisterNotification(msg *nats.Msg) {

err := s.nm.DeleteNotification(ref)
if err != nil {
_, isNotFoundError := err.(*notification.NotFoundError)
if isNotFoundError {
var e *notification.NotFoundError
if errors.As(err, &e) {
s.log.Debug().Msgf("a notification with ref %s does not exist", ref)
} else {
s.log.Error().Err(err).Msgf("notification unregister failed")
Expand Down Expand Up @@ -322,8 +321,8 @@ func (s *svc) handleMsgTrigger(msg *nats.Msg) {
if notif == nil {
notif, err = s.nm.GetNotification(tr.Ref)
if err != nil {
_, isNotFoundError := err.(*notification.NotFoundError)
if isNotFoundError {
var e *notification.NotFoundError
if errors.As(err, &e) {
s.log.Debug().Msgf("trigger %s does not have a notification attached", tr.Ref)
return
}
Expand Down
19 changes: 10 additions & 9 deletions pkg/notification/db_changes.sql
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,35 @@
-- This file can be used to make the required changes to the MySQL DB. This is
-- not a proper migration but it should work on most situations.

CREATE TABLE `cbox_notifications` (
CREATE TABLE `notifications` (
`id` INT PRIMARY KEY AUTO_INCREMENT,
`ref` VARCHAR(3072) UNIQUE NOT NULL,
`template_name` VARCHAR(320) NOT NULL
);

COMMIT;

CREATE TABLE `cbox_notification_recipients` (
CREATE TABLE `notification_recipients` (
`id` INT PRIMARY KEY AUTO_INCREMENT,
`notification_id` INT NOT NULL,
`recipient` VARCHAR(320) NOT NULL,
FOREIGN KEY (notification_id)
REFERENCES cbox_notifications (id)
REFERENCES notifications (id)
ON DELETE CASCADE
);

COMMIT;

CREATE INDEX `cbox_notifications_ix0` ON `cbox_notifications` (`ref`);
CREATE INDEX `notifications_ix0` ON `notifications` (`ref`);

CREATE INDEX `cbox_notification_recipients_ix0` ON `cbox_notification_recipients` (`notification_id`);
CREATE INDEX `cbox_notification_recipients_ix1` ON `cbox_notification_recipients` (`user_name`);
CREATE INDEX `notification_recipients_ix0` ON `notification_recipients` (`notification_id`);
CREATE INDEX `notification_recipients_ix1` ON `notification_recipients` (`recipient`);

-- changes for added notifications on oc shares

ALTER TABLE cernboxngcopy.oc_share ADD notify_uploads BOOL DEFAULT false;
ALTER TABLE oc_share ADD notify_uploads BOOL DEFAULT false;
ALTER TABLE oc_share ADD notify_uploads_extra_recipients VARCHAR(2048);

UPDATE cernboxngcopy.oc_share SET notify_uploads = false;
UPDATE oc_share SET notify_uploads = false;

ALTER TABLE cernboxngcopy.oc_share MODIFY notify_uploads BOOL DEFAULT false NOT NULL;
ALTER TABLE oc_share MODIFY notify_uploads BOOL DEFAULT false NOT NULL;
4 changes: 2 additions & 2 deletions pkg/notification/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ type InvalidNotificationError struct {
}

// Error returns the string error msg for NotFoundError.
func (n *NotFoundError) Error() string {
func (n NotFoundError) Error() string {
return fmt.Sprintf("notification %s not found", n.Ref)
}

// Error returns the string error msg for InvalidNotificationError.
func (i *InvalidNotificationError) Error() string {
func (i InvalidNotificationError) Error() string {
return i.Msg
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/notification/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type FileNotFoundError struct {
}

// Error returns the string error msg for FileNotFoundError.
func (t *FileNotFoundError) Error() string {
func (t FileNotFoundError) Error() string {
return fmt.Sprintf("template file %s not found", t.TemplateFileName)
}

Expand Down

0 comments on commit b38215d

Please sign in to comment.