-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: Domains API operations #208
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package clerk | ||
|
||
// DeletedResource describes an API resource that is no longer | ||
// available. | ||
// It's usually encountered as a result of delete API operations. | ||
type DeletedResource struct { | ||
APIResource | ||
ID string `json:"id,omitempty"` | ||
Slug string `json:"slug,omitempty"` | ||
Object string `json:"object"` | ||
Deleted bool `json:"deleted"` | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package clerk | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ Couldn't we use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could, but I'm afraid that at some point we might need this type from another package. Imagine an Instance which has a nested Domain. I believe it's safer to have these types for "models" in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, I thought that we might want to reuse structs. Do you think that if we introduced a import (
domainmodels "github.com/clerk/clerk-sdk-go/v2/models/domains"
) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion! Your solution would certainly avoid circular dependency, but isn't it a bit too much packaging? Is there any reason why using the clerk package might be problematic? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not problematic, it's only that packages provide a native namespace and the import aliases provide some more naming flexibility for someone that uses them. Names also become more uniform. package domain
// Debatable name :)
type Object struct{}
type List struct {
Data []Object `json:"data"`
} import (
domainmodels "github.com/clerk/clerk-sdk-go/v2/models/domains"
)
func HandleDomains() {
var domainList domainmodels.List
//...
} Just an idea here, not insisting on this, it might be overly verbose/complicated for some reason. Noting now, since it would be a breaking change afterwards! |
||
|
||
type Domain struct { | ||
APIResource | ||
ID string `json:"id"` | ||
Object string `json:"object"` | ||
Name string `json:"name"` | ||
IsSatellite bool `json:"is_satellite"` | ||
FrontendAPIURL string `json:"frontend_api_url"` | ||
AccountPortalURL *string `json:"accounts_portal_url,omitempty"` | ||
ProxyURL *string `json:"proxy_url,omitempty"` | ||
CNAMETargets []CNAMETarget `json:"cname_targets,omitempty"` | ||
DevelopmentOrigin string `json:"development_origin"` | ||
} | ||
|
||
type CNAMETarget struct { | ||
Host string `json:"host"` | ||
Value string `json:"value"` | ||
} | ||
|
||
type DomainList struct { | ||
APIResource | ||
Domains []*Domain `json:"data"` | ||
TotalCount int64 `json:"total_count"` | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
// Package domain provides the Domains API. | ||
package domain | ||
|
||
import ( | ||
"context" | ||
"net/http" | ||
"net/url" | ||
|
||
"github.com/clerk/clerk-sdk-go/v2" | ||
) | ||
|
||
const path = "/domains" | ||
|
||
type CreateParams struct { | ||
clerk.APIParams | ||
Name *string `json:"name,omitempty"` | ||
ProxyURL *string `json:"proxy_url,omitempty"` | ||
IsSatellite *bool `json:"is_satellite,omitempty"` | ||
} | ||
|
||
// Create creates a new domain. | ||
func Create(ctx context.Context, params *CreateParams) (*clerk.Domain, error) { | ||
req := clerk.NewAPIRequest(http.MethodPost, path) | ||
req.SetParams(params) | ||
|
||
domain := &clerk.Domain{} | ||
err := clerk.GetBackend().Call(ctx, req, domain) | ||
return domain, err | ||
} | ||
|
||
type UpdateParams struct { | ||
clerk.APIParams | ||
Name *string `json:"name,omitempty"` | ||
ProxyURL *string `json:"proxy_url,omitempty"` | ||
} | ||
|
||
// Update updates a domain's properties. | ||
func Update(ctx context.Context, id string, params *UpdateParams) (*clerk.Domain, error) { | ||
path, err := url.JoinPath(path, id) | ||
if err != nil { | ||
return nil, err | ||
} | ||
req := clerk.NewAPIRequest(http.MethodPatch, path) | ||
req.SetParams(params) | ||
|
||
domain := &clerk.Domain{} | ||
err = clerk.GetBackend().Call(ctx, req, domain) | ||
return domain, err | ||
} | ||
|
||
// Delete removes a domain. | ||
func Delete(ctx context.Context, id string) (*clerk.DeletedResource, error) { | ||
path, err := url.JoinPath(path, id) | ||
if err != nil { | ||
return nil, err | ||
} | ||
req := clerk.NewAPIRequest(http.MethodDelete, path) | ||
domain := &clerk.DeletedResource{} | ||
err = clerk.GetBackend().Call(ctx, req, domain) | ||
return domain, err | ||
} | ||
|
||
type ListParams struct { | ||
clerk.APIParams | ||
} | ||
|
||
// List returns a list of domains | ||
func List(ctx context.Context, params *ListParams) (*clerk.DomainList, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 If there's not a strong reason to use pointers, I think we should avoid them where possible, as they make garbage collection more difficult and the risk of accidental mutations increases. |
||
req := clerk.NewAPIRequest(http.MethodGet, path) | ||
list := &clerk.DomainList{} | ||
err := clerk.GetBackend().Call(ctx, req, list) | ||
return list, err | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,194 @@ | ||
package domain | ||
|
||
import ( | ||
"bytes" | ||
"context" | ||
"encoding/json" | ||
"fmt" | ||
"io" | ||
"net/http" | ||
"testing" | ||
|
||
"github.com/clerk/clerk-sdk-go/v2" | ||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestDomainCreate(t *testing.T) { | ||
name := "clerk.com" | ||
id := "dmn_123" | ||
clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ | ||
HTTPClient: &http.Client{ | ||
Transport: &mockRoundTripper{ | ||
T: t, | ||
in: json.RawMessage(fmt.Sprintf(`{"name":"%s"}`, name)), | ||
out: json.RawMessage(fmt.Sprintf(`{"id":"%s","name":"%s"}`, id, name)), | ||
path: "/v1/domains", | ||
method: http.MethodPost, | ||
}, | ||
}, | ||
})) | ||
|
||
dmn, err := Create(context.Background(), &CreateParams{ | ||
Name: clerk.String(name), | ||
}) | ||
require.NoError(t, err) | ||
assert.Equal(t, id, dmn.ID) | ||
assert.Equal(t, name, dmn.Name) | ||
} | ||
|
||
func TestDomainCreate_Error(t *testing.T) { | ||
clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ | ||
HTTPClient: &http.Client{ | ||
Transport: &mockRoundTripper{ | ||
T: t, | ||
status: http.StatusBadRequest, | ||
out: json.RawMessage(`{ | ||
"errors":[{ | ||
"code":"create-error-code" | ||
}], | ||
"clerk_trace_id":"create-trace-id" | ||
}`), | ||
}, | ||
}, | ||
})) | ||
|
||
_, err := Create(context.Background(), &CreateParams{}) | ||
require.Error(t, err) | ||
apiErr, ok := err.(*clerk.APIErrorResponse) | ||
require.True(t, ok) | ||
assert.Equal(t, "create-trace-id", apiErr.TraceID) | ||
require.Equal(t, 1, len(apiErr.Errors)) | ||
assert.Equal(t, "create-error-code", apiErr.Errors[0].Code) | ||
} | ||
|
||
func TestDomainUpdate(t *testing.T) { | ||
id := "dmn_456" | ||
name := "clerk.dev" | ||
clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ | ||
HTTPClient: &http.Client{ | ||
Transport: &mockRoundTripper{ | ||
T: t, | ||
in: json.RawMessage(fmt.Sprintf(`{"name":"%s"}`, name)), | ||
out: json.RawMessage(fmt.Sprintf(`{"id":"%s","name":"%s"}`, id, name)), | ||
path: fmt.Sprintf("/v1/domains/%s", id), | ||
method: http.MethodPatch, | ||
}, | ||
}, | ||
})) | ||
|
||
dmn, err := Update(context.Background(), id, &UpdateParams{ | ||
Name: clerk.String(name), | ||
}) | ||
require.NoError(t, err) | ||
assert.Equal(t, id, dmn.ID) | ||
assert.Equal(t, name, dmn.Name) | ||
} | ||
|
||
func TestDomainUpdate_Error(t *testing.T) { | ||
clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ | ||
HTTPClient: &http.Client{ | ||
Transport: &mockRoundTripper{ | ||
T: t, | ||
status: http.StatusBadRequest, | ||
out: json.RawMessage(`{ | ||
"errors":[{ | ||
"code":"update-error-code" | ||
}], | ||
"clerk_trace_id":"update-trace-id" | ||
}`), | ||
}, | ||
}, | ||
})) | ||
|
||
_, err := Update(context.Background(), "dmn_123", &UpdateParams{}) | ||
require.Error(t, err) | ||
apiErr, ok := err.(*clerk.APIErrorResponse) | ||
require.True(t, ok) | ||
assert.Equal(t, "update-trace-id", apiErr.TraceID) | ||
require.Equal(t, 1, len(apiErr.Errors)) | ||
assert.Equal(t, "update-error-code", apiErr.Errors[0].Code) | ||
} | ||
|
||
func TestDomainDelete(t *testing.T) { | ||
id := "dmn_789" | ||
clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ | ||
HTTPClient: &http.Client{ | ||
Transport: &mockRoundTripper{ | ||
T: t, | ||
out: json.RawMessage(fmt.Sprintf(`{"id":"%s","deleted":true}`, id)), | ||
path: fmt.Sprintf("/v1/domains/%s", id), | ||
method: http.MethodDelete, | ||
}, | ||
}, | ||
})) | ||
|
||
dmn, err := Delete(context.Background(), id) | ||
require.NoError(t, err) | ||
assert.Equal(t, id, dmn.ID) | ||
assert.True(t, dmn.Deleted) | ||
} | ||
|
||
func TestDomainList(t *testing.T) { | ||
clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ | ||
HTTPClient: &http.Client{ | ||
Transport: &mockRoundTripper{ | ||
T: t, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Don't you want to assert that the |
||
out: json.RawMessage(`{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 You can also introduce a helper here: out: MustJSONMarshal(clerk.DomainList{}), Since JSON serialization/deserialization is generally symmetric, I think this doesn't break the expectation validity here. |
||
"data": [{"id":"dmn_123","name":"clerk.com"}], | ||
"total_count": 1 | ||
}`), | ||
path: "/v1/domains", | ||
method: http.MethodGet, | ||
}, | ||
}, | ||
})) | ||
|
||
list, err := List(context.Background(), &ListParams{}) | ||
require.NoError(t, err) | ||
assert.Equal(t, int64(1), list.TotalCount) | ||
assert.Equal(t, 1, len(list.Domains)) | ||
assert.Equal(t, "dmn_123", list.Domains[0].ID) | ||
assert.Equal(t, "clerk.com", list.Domains[0].Name) | ||
} | ||
|
||
type mockRoundTripper struct { | ||
T *testing.T | ||
// Response status. | ||
status int | ||
// Response body. | ||
out json.RawMessage | ||
// If set, we'll assert that the request body | ||
// matches. | ||
in json.RawMessage | ||
// If set, we'll assert the request path matches. | ||
path string | ||
// If set, we'll assert that the request method matches. | ||
method string | ||
} | ||
|
||
func (rt *mockRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { | ||
georgepsarakis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if rt.status == 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Mostly a suggestion, it might be easier to use a constructor for initializations: |
||
rt.status = http.StatusOK | ||
} | ||
|
||
if rt.method != "" { | ||
georgepsarakis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
require.Equal(rt.T, rt.method, r.Method) | ||
} | ||
if rt.path != "" { | ||
georgepsarakis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
require.Equal(rt.T, rt.path, r.URL.Path) | ||
} | ||
if rt.in != nil { | ||
georgepsarakis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
body, err := io.ReadAll(r.Body) | ||
if err != nil { | ||
return nil, err | ||
} | ||
defer r.Body.Close() | ||
require.JSONEq(rt.T, string(rt.in), string(body)) | ||
georgepsarakis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
return &http.Response{ | ||
StatusCode: rt.status, | ||
Body: io.NopCloser(bytes.NewReader(rt.out)), | ||
}, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ I just noticed this and I'm not sure I understand it 100%. This also allows for some room for someone to erroneously tamper with the
q
object, if they implement their own custom struct, isn't that right? I would probably expect it to be the other way round, but let me know what you think (and this goes up to theAPIRequest
struct I believe):The
Queryable
interface could be then:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great point! I'll update the interface and the logic. I'd rather do it on a separate PR. Do you think that would be a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a problem at all, by all means proceed with merging 🚀