Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Matcher refactor #543

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Matcher refactor #543

merged 2 commits into from
Sep 11, 2024

Conversation

maurafortino
Copy link
Contributor

@maurafortino maurafortino commented Sep 9, 2024

What's Included:
-Refactored the Matcher functionality so that it was only checking the matcher field for v2 and the matcher and events fields for v1
-Moved the urls functionality out of Matcher into Sink

Future TODO: renaming the Sink structs so as not to get confused with the webhook-schema structs

@@ -30,26 +26,22 @@ func (c *ClientMock) Do(req *http.Request) (*http.Response, error) {
// move to subpackage and change to Interface
type Matcher interface {
IsMatch(*wrp.Message) bool

Copy link
Contributor Author

@maurafortino maurafortino Sep 9, 2024

Choose a reason for hiding this comment

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

removed getUrls from Mathcer interface since the urls field has been moved to sink.go

}

type MatcherV1 struct {
events []*regexp.Regexp
matcher []*regexp.Regexp
urls *ring.Ring
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved urls to sink.go (webhookV1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any functionality related to urls that was in matcher.go has also been moved to sink.go

type CommonWebhook struct {
mutex sync.RWMutex

type CommonMatcher struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a common matcher struct - not sure if it's all that necessary though

@@ -27,84 +30,141 @@ import (
)

type Sink interface {
Update(ancla.Register) error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

still have the update function - but the signature has been updated and is no longer required for the Sink interface

//TODO: need to determine best way to add client and client middleware to WebhooV1
// client http.Client
// clientMiddleware func(http.Client) http.Client
}

type Webhooks []*WebhookV1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

going to use this webhookV1 struct (will be renaming this so as not to be confused with the webhookV1 schema) as the struct for the newest webhook struct in webhook schema for now - might have to add a third struct to satisfy all registrations

type Kafkas []*Kafka
type Kafka struct {
id string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

id and logger are both in comonwebhook struct

}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all of this has been move to the kafka.Update function

return nil
}

func (v1 *WebhookV1) updateUrls(urlCount int, url string, urls []string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved over this functionality from mathcer.go

@@ -420,7 +418,6 @@ Loop:
}
s.queueDepthGauge.Add(-1.0)
s.mutex.RLock()
urls = s.matcher.getUrls()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

urls is now added to the sink structs and we will be referencing them directly from the struck rather than sending them to the send function

@maurafortino maurafortino self-assigned this Sep 9, 2024
@denopink denopink merged commit 5a717ad into denopink/feat/rewrite Sep 11, 2024
11 of 14 checks passed
@denopink denopink deleted the matcher-refactor branch September 11, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants