Skip to content

URNs refactor #738

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

Merged
merged 2 commits into from
May 7, 2024
Merged
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
56 changes: 28 additions & 28 deletions backends/rapidpro/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/nyaruka/courier/test"
"github.com/nyaruka/gocommon/dbutil/assertdb"
"github.com/nyaruka/gocommon/httpx"
"github.com/nyaruka/gocommon/i18n"
"github.com/nyaruka/gocommon/jsonx"
"github.com/nyaruka/gocommon/urns"
"github.com/nyaruka/gocommon/uuids"
Expand Down Expand Up @@ -195,7 +196,7 @@ func (ts *BackendTestSuite) TestDeleteMsgByExternalID() {
func (ts *BackendTestSuite) TestContact() {
knChannel := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c95d")
clog := courier.NewChannelLog(courier.ChannelLogTypeUnknown, knChannel, nil)
urn, _ := urns.NewTelURNForCountry("12065551518", "US")
Copy link
Member Author

Choose a reason for hiding this comment

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

tests like this don't need to call URN parsing functions.. especially if they're not checking for errors

urn := urns.URN("tel:+12065551518")

ctx := context.Background()
now := time.Now()
Expand All @@ -220,15 +221,15 @@ func (ts *BackendTestSuite) TestContact() {
ts.True(contact2.CreatedOn_.Before(now2))

// load a contact by URN instead (this one is in our testdata)
cURN, _ := urns.NewTelURNForCountry("+12067799192", "US")
cURN := urns.URN("tel:+12067799192")
contact, err = contactForURN(ctx, ts.b, knChannel.OrgID(), knChannel, cURN, nil, "", clog)
ts.NoError(err)
ts.NotNil(contact)

ts.Equal(null.String(""), contact.Name_)
ts.Equal(courier.ContactUUID("a984069d-0008-4d8c-a772-b14a8a6acccc"), contact.UUID_)

urn, _ = urns.NewTelURNForCountry("12065551519", "US")
urn = urns.URN("tel:+12065551519")

// long name are truncated

Expand All @@ -243,7 +244,7 @@ func (ts *BackendTestSuite) TestContact() {
func (ts *BackendTestSuite) TestContactRace() {
knChannel := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c95d")
clog := courier.NewChannelLog(courier.ChannelLogTypeUnknown, knChannel, nil)
urn, _ := urns.NewTelURNForCountry("12065551518", "US")
urn := urns.URN("tel:+12065551518")

urnSleep = true
defer func() { urnSleep = false }()
Expand Down Expand Up @@ -273,8 +274,7 @@ func (ts *BackendTestSuite) TestAddAndRemoveContactURN() {
clog := courier.NewChannelLog(courier.ChannelLogTypeUnknown, knChannel, nil)
ctx := context.Background()

cURN, err := urns.NewTelURNForCountry("+12067799192", "US")
ts.NoError(err)
cURN := urns.URN("tel:+12067799192")

contact, err := contactForURN(ctx, ts.b, knChannel.OrgID_, knChannel, cURN, nil, "", clog)
ts.NoError(err)
Expand All @@ -287,7 +287,7 @@ func (ts *BackendTestSuite) TestAddAndRemoveContactURN() {
ts.NoError(err)
ts.Equal(len(contactURNs), 1)

urn, _ := urns.NewTelURNForCountry("12065551518", "US")
urn := urns.URN("tel:+12065551518")
addedURN, err := ts.b.AddURNtoContact(ctx, knChannel, contact, urn, nil)
ts.NoError(err)
ts.NotNil(addedURN)
Expand All @@ -314,7 +314,7 @@ func (ts *BackendTestSuite) TestContactURN() {
knChannel := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c95d")
twChannel := ts.getChannel("TW", "dbc126ed-66bc-4e28-b67b-81dc3327c96a")
clog := courier.NewChannelLog(courier.ChannelLogTypeUnknown, knChannel, nil)
urn, _ := urns.NewTelURNForCountry("12065551515", "US")
urn := urns.URN("tel:+12065551515")

ctx := context.Background()

Expand Down Expand Up @@ -371,12 +371,12 @@ func (ts *BackendTestSuite) TestContactURN() {

// test that we don't use display when looking up URNs
tgChannel := ts.getChannel("TG", "dbc126ed-66bc-4e28-b67b-81dc3327c98a")
tgURN, _ := urns.NewTelegramURN(12345, "")
tgURN := urns.URN("telegram:12345")

tgContact, err := contactForURN(ctx, ts.b, tgChannel.OrgID_, tgChannel, tgURN, nil, "", clog)
ts.NoError(err)

tgURNDisplay, _ := urns.NewTelegramURN(12345, "Jane")
tgURNDisplay := urns.URN("telegram:12345#Jane")
displayContact, err := contactForURN(ctx, ts.b, tgChannel.OrgID_, tgChannel, tgURNDisplay, nil, "", clog)

ts.NoError(err)
Expand All @@ -393,7 +393,7 @@ func (ts *BackendTestSuite) TestContactURN() {
ts.Equal(null.String("Jane"), tgContactURN.Display)

// try to create two contacts at the same time in goroutines, this tests our transaction rollbacks
urn2, _ := urns.NewTelURNForCountry("12065551616", "US")
urn2 := urns.URN("tel:+12065551616")
var wait sync.WaitGroup
var contact2, contact3 *Contact
wait.Add(2)
Expand All @@ -419,8 +419,8 @@ func (ts *BackendTestSuite) TestContactURN() {
func (ts *BackendTestSuite) TestContactURNPriority() {
knChannel := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c95d")
twChannel := ts.getChannel("TW", "dbc126ed-66bc-4e28-b67b-81dc3327c96a")
knURN, _ := urns.NewTelURNForCountry("12065551111", "US")
twURN, _ := urns.NewTelURNForCountry("12065552222", "US")
knURN := urns.URN("tel:+12065551111")
twURN := urns.URN("tel:+12065552222")
clog := courier.NewChannelLog(courier.ChannelLogTypeUnknown, knChannel, nil)

ctx := context.Background()
Expand Down Expand Up @@ -627,12 +627,12 @@ func (ts *BackendTestSuite) TestMsgStatus() {

// update URN when the new doesn't exist
tx, _ := ts.b.db.BeginTxx(ctx, nil)
oldURN, _ := urns.NewWhatsAppURN("55988776655")
oldURN := urns.URN("whatsapp:55988776655")
_ = insertContactURN(tx, newContactURN(channel.OrgID_, channel.ID_, NilContactID, oldURN, nil))

ts.NoError(tx.Commit())

newURN, _ := urns.NewWhatsAppURN("5588776655")
newURN := urns.URN("whatsapp:5588776655")
status = ts.b.NewStatusUpdate(channel, courier.MsgID(10000), courier.MsgStatusSent, clog6)
status.SetURNUpdate(oldURN, newURN)

Expand All @@ -646,8 +646,8 @@ func (ts *BackendTestSuite) TestMsgStatus() {
ts.NoError(tx.Commit())

// new URN already exits but don't have an associated contact
oldURN, _ = urns.NewWhatsAppURN("55999887766")
newURN, _ = urns.NewWhatsAppURN("5599887766")
oldURN = urns.URN("whatsapp:55999887766")
newURN = urns.URN("whatsapp:5599887766")
tx, _ = ts.b.db.BeginTxx(ctx, nil)
contact, _ := contactForURN(ctx, ts.b, channel.OrgID_, channel, oldURN, nil, "", clog6)
_ = insertContactURN(tx, newContactURN(channel.OrgID_, channel.ID_, NilContactID, newURN, nil))
Expand All @@ -668,8 +668,8 @@ func (ts *BackendTestSuite) TestMsgStatus() {
ts.NoError(tx.Commit())

// new URN already exits and have an associated contact
oldURN, _ = urns.NewWhatsAppURN("55988776655")
newURN, _ = urns.NewWhatsAppURN("5588776655")
oldURN = urns.URN("whatsapp:55988776655")
newURN = urns.URN("whatsapp:5588776655")
tx, _ = ts.b.db.BeginTxx(ctx, nil)
_, _ = contactForURN(ctx, ts.b, channel.OrgID_, channel, oldURN, nil, "", clog6)
otherContact, _ := contactForURN(ctx, ts.b, channel.OrgID_, channel, newURN, nil, "", clog6)
Expand Down Expand Up @@ -747,8 +747,8 @@ func (ts *BackendTestSuite) TestCheckForDuplicate() {
ctx := context.Background()
knChannel := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c95d")
twChannel := ts.getChannel("TW", "dbc126ed-66bc-4e28-b67b-81dc3327c96a")
urn, _ := urns.NewTelURNForCountry("12065551215", knChannel.Country())
urn2, _ := urns.NewTelURNForCountry("12065551277", knChannel.Country())
urn := urns.URN("tel:+12065551215")
urn2 := urns.URN("tel:+12065551277")

createAndWriteMsg := func(ch courier.Channel, u urns.URN, text, extID string) *Msg {
clog := courier.NewChannelLog(courier.ChannelLogTypeUnknown, knChannel, nil)
Expand Down Expand Up @@ -900,14 +900,14 @@ func (ts *BackendTestSuite) TestOutgoingQueue() {

func (ts *BackendTestSuite) TestChannel() {
noAddress := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c99a")
ts.Equal("US", noAddress.Country())
ts.Equal(i18n.Country("US"), noAddress.Country())
ts.Equal(courier.NilChannelAddress, noAddress.ChannelAddress())

knChannel := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c95d")

ts.Equal("2500", knChannel.Address())
ts.Equal(courier.ChannelAddress("2500"), knChannel.ChannelAddress())
ts.Equal("RW", knChannel.Country())
ts.Equal(i18n.Country("RW"), knChannel.Country())
ts.Equal([]courier.ChannelRole{courier.ChannelRoleSend, courier.ChannelRoleReceive}, knChannel.Roles())
ts.True(knChannel.HasRole(courier.ChannelRoleSend))
ts.True(knChannel.HasRole(courier.ChannelRoleReceive))
Expand Down Expand Up @@ -1098,7 +1098,7 @@ func (ts *BackendTestSuite) TestWriteMsg() {
now := time.Now().Round(time.Microsecond).In(time.UTC)

// create a new courier msg
urn, _ := urns.NewTelURNForCountry("12065551212", knChannel.Country())
urn := urns.URN("tel:+12065551212")
msg := ts.b.NewIncomingMsg(knChannel, urn, "test123", "ext123", clog).WithReceivedOn(now).WithContactName("test contact").(*Msg)

// try to write it to our db
Expand Down Expand Up @@ -1197,7 +1197,7 @@ func (ts *BackendTestSuite) TestWriteMsgWithAttachments() {

knChannel := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c95d")
clog := courier.NewChannelLog(courier.ChannelLogTypeUnknown, knChannel, nil)
urn, _ := urns.NewTelURNForCountry("12065551218", knChannel.Country())
urn := urns.URN("tel:+12065551218")

msg := ts.b.NewIncomingMsg(knChannel, urn, "two regular attachments", "", clog).(*Msg)
msg.WithAttachment("http://example.com/test.jpg")
Expand Down Expand Up @@ -1242,7 +1242,7 @@ func (ts *BackendTestSuite) TestPreferredChannelCheckRole() {
// have to round to microseconds because postgres can't store nanos
now := time.Now().Round(time.Microsecond).In(time.UTC)

urn, _ := urns.NewTelURNForCountry("12065552020", exChannel.Country())
urn := urns.URN("tel:+12065552020")
msg := ts.b.NewIncomingMsg(exChannel, urn, "test123", "ext123", clog).WithReceivedOn(now).WithContactName("test contact").(*Msg)

// try to write it to our db
Expand Down Expand Up @@ -1270,7 +1270,7 @@ func (ts *BackendTestSuite) TestChannelEvent() {
ctx := context.Background()
channel := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c95d")
clog := courier.NewChannelLog(courier.ChannelLogTypeUnknown, channel, nil)
urn, _ := urns.NewTelURNForCountry("12065551616", channel.Country())
urn := urns.URN("tel:+12065551616")

event := ts.b.NewChannelEvent(channel, courier.EventTypeReferral, urn, clog).WithExtra(map[string]string{"ref_id": "12345"}).WithContactName("kermit frog")
err := ts.b.WriteChannelEvent(ctx, event, clog)
Expand Down Expand Up @@ -1319,7 +1319,7 @@ func (ts *BackendTestSuite) TestMailroomEvents() {

channel := ts.getChannel("KN", "dbc126ed-66bc-4e28-b67b-81dc3327c95d")
clog := courier.NewChannelLog(courier.ChannelLogTypeUnknown, channel, nil)
urn, _ := urns.NewTelURNForCountry("12065551616", channel.Country())
urn := urns.URN("tel:+12065551616")

event := ts.b.NewChannelEvent(channel, courier.EventTypeReferral, urn, clog).
WithExtra(map[string]string{"ref_id": "12345"}).
Expand Down
8 changes: 5 additions & 3 deletions backends/rapidpro/channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

"github.com/lib/pq"
"github.com/nyaruka/courier"
"github.com/nyaruka/gocommon/i18n"
"github.com/nyaruka/gocommon/urns"
"github.com/nyaruka/null/v3"
)

Expand Down Expand Up @@ -56,11 +58,11 @@
}

// Country returns the country code for this channel if any
func (c *Channel) Country() string { return c.Country_.String }
func (c *Channel) Country() i18n.Country { return i18n.Country(c.Country_.String) }

// IsScheme returns whether this channel serves only the passed in scheme
func (c *Channel) IsScheme(scheme string) bool {
return len(c.Schemes_) == 1 && c.Schemes_[0] == scheme
func (c *Channel) IsScheme(scheme *urns.Scheme) bool {
return len(c.Schemes_) == 1 && c.Schemes_[0] == scheme.Prefix

Check warning on line 65 in backends/rapidpro/channel.go

View check run for this annotation

Codecov / codecov/patch

backends/rapidpro/channel.go#L64-L65

Added lines #L64 - L65 were not covered by tests
}

// Roles returns the roles of this channel
Expand Down
2 changes: 1 addition & 1 deletion backends/rapidpro/urn.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func setDefaultURN(db *sqlx.Tx, channel *Channel, contact *Contact, urn urns.URN
existing.Priority = currPriority

// if this is a phone number and we just received a message on a tel scheme, set that as our new preferred channel
if existing.Scheme == urns.TelScheme && scheme == urns.TelScheme && channel.HasRole(courier.ChannelRoleSend) {
if existing.Scheme == urns.Phone.Prefix && scheme == urns.Phone.Prefix && channel.HasRole(courier.ChannelRoleSend) {
existing.ChannelID = channel.ID()
}
currPriority--
Expand Down
6 changes: 4 additions & 2 deletions channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"database/sql/driver"
"errors"

"github.com/nyaruka/gocommon/i18n"
"github.com/nyaruka/gocommon/urns"
"github.com/nyaruka/gocommon/uuids"
"github.com/nyaruka/null/v3"
)
Expand Down Expand Up @@ -123,14 +125,14 @@ type Channel interface {
Name() string
ChannelType() ChannelType
Schemes() []string
Country() string
Country() i18n.Country
Address() string
ChannelAddress() ChannelAddress

Roles() []ChannelRole

// is this channel for the passed in scheme (and only that scheme)
IsScheme(string) bool
IsScheme(*urns.Scheme) bool

// CallbackDomain returns the domain that should be used for any callbacks the channel registers
CallbackDomain(fallbackDomain string) string
Expand Down
20 changes: 10 additions & 10 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.22

require (
github.com/antchfx/xmlquery v1.4.0
github.com/aws/aws-sdk-go v1.51.25
github.com/aws/aws-sdk-go v1.52.3
github.com/buger/jsonparser v1.1.1
github.com/dghubble/oauth1 v0.7.3
github.com/getsentry/sentry-go v0.27.0
Expand All @@ -13,18 +13,18 @@ require (
github.com/gomodule/redigo v1.9.2
github.com/gorilla/schema v1.3.0
github.com/h2non/filetype v1.1.3
github.com/jmoiron/sqlx v1.3.5
github.com/jmoiron/sqlx v1.4.0
github.com/lib/pq v1.10.9
github.com/nyaruka/ezconf v0.3.0
github.com/nyaruka/gocommon v1.53.2
github.com/nyaruka/gocommon v1.54.2
github.com/nyaruka/null/v3 v3.0.0
github.com/nyaruka/redisx v0.8.0
github.com/patrickmn/go-cache v2.1.0+incompatible
github.com/pkg/errors v0.9.1
github.com/samber/slog-multi v1.0.2
github.com/samber/slog-sentry v1.2.2
github.com/stretchr/testify v1.9.0
golang.org/x/exp v0.0.0-20240416160154-fe59bbe5cc7f
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842
golang.org/x/mod v0.17.0
gopkg.in/go-playground/validator.v9 v9.31.0
)
Expand All @@ -36,7 +36,7 @@ require (
github.com/gabriel-vasile/mimetype v1.4.3 // indirect
github.com/go-playground/locales v0.14.1 // indirect
github.com/go-playground/universal-translator v0.18.1 // indirect
github.com/go-playground/validator/v10 v10.19.0 // indirect
github.com/go-playground/validator/v10 v10.20.0 // indirect
github.com/gofrs/uuid v4.4.0+incompatible // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/gorilla/websocket v1.5.1 // indirect
Expand All @@ -52,12 +52,12 @@ require (
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/samber/lo v1.39.0 // indirect
github.com/shopspring/decimal v1.4.0 // indirect
golang.org/x/crypto v0.22.0 // indirect
golang.org/x/net v0.24.0 // indirect
golang.org/x/crypto v0.23.0 // indirect
golang.org/x/net v0.25.0 // indirect
golang.org/x/sync v0.7.0 // indirect
golang.org/x/sys v0.19.0 // indirect
golang.org/x/text v0.14.0 // indirect
google.golang.org/protobuf v1.33.0 // indirect
golang.org/x/sys v0.20.0 // indirect
golang.org/x/text v0.15.0 // indirect
google.golang.org/protobuf v1.34.1 // indirect
gopkg.in/go-playground/assert.v1 v1.2.1 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
Loading
Loading