Skip to content
This repository has been archived by the owner on Jun 21, 2024. It is now read-only.

Hydration with Href, ID, and Get/Reload abstraction #280

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Hydration with Href, ID, and Get/Reload abstraction #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
830e8c2
demonstrate how Devices could use Hydrate
displague Feb 9, 2021
21f77d3
implement Hrefer in OrganizationOps
displague May 10, 2021
df1fd48
wip
displague Aug 4, 2021
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
2 changes: 1 addition & 1 deletion api_call_options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func nextPage(meta meta, opts *GetOptions) (path string) {
if meta.Next != nil && (opts.GetPage() == 0) {
optsCopy := opts.CopyOrNew()
optsCopy.Page = meta.CurrentPageNum + 1
return optsCopy.WithQuery(stripQuery(meta.Next.Href))
return optsCopy.WithQuery(stripQuery(*meta.Next.Href))
}
if opts != nil {
opts.Meta = meta
Expand Down
54 changes: 34 additions & 20 deletions devices.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package packngo
import (
"encoding/json"
"fmt"
"net/http"
"net/url"
"path"
"strconv"
Expand Down Expand Up @@ -43,8 +44,9 @@ type devicesRoot struct {

// Device represents an Equinix Metal device from API
type Device struct {
*Href `json:",inline"`

ID string `json:"id"`
Href string `json:"href,omitempty"`
Hostname string `json:"hostname,omitempty"`
Description *string `json:"description,omitempty"`
State string `json:"state,omitempty"`
Expand Down Expand Up @@ -458,7 +460,7 @@ func (d DeviceActionRequest) String() string {

// DeviceServiceOp implements DeviceService
type DeviceServiceOp struct {
client *Client
*serviceOp
}

// List returns devices on a project
Expand All @@ -475,14 +477,14 @@ func (s *DeviceServiceOp) List(projectID string, opts *ListOptions) (devices []D
if validateErr := ValidateUUID(projectID); validateErr != nil {
return nil, nil, validateErr
}
opts = opts.Including("facility")
opts = opts.Including(s.DefaultIncludes()...)
endpointPath := path.Join(projectBasePath, projectID, deviceBasePath)
apiPathQuery := opts.WithQuery(endpointPath)

for {
subset := new(devicesRoot)

resp, err = s.client.DoRequest("GET", apiPathQuery, nil, subset)
resp, err = s.client.DoRequest(http.MethodGet, apiPathQuery, nil, subset)
if err != nil {
return nil, resp, err
}
Expand All @@ -497,18 +499,30 @@ func (s *DeviceServiceOp) List(projectID string, opts *ListOptions) (devices []D
}
}

func (s *DeviceServiceOp) DefaultIncludes() []string {
return []string{"facility"}
}

func (d *Device) SetHref(href string) {
d.Href = &Href{Href: &href}
}

func (d *Device) SetID(id string) {
d.ID = id
d.SetHref(path.Join(deviceBasePath, id))
}

// Get returns a device by id
func (s *DeviceServiceOp) Get(deviceID string, opts *GetOptions) (*Device, *Response, error) {
if validateErr := ValidateUUID(deviceID); validateErr != nil {
return nil, nil, validateErr
}
opts = opts.Including("facility")
endpointPath := path.Join(deviceBasePath, deviceID)
apiPathQuery := opts.WithQuery(endpointPath)
device := new(Device)
resp, err := s.client.DoRequest("GET", apiPathQuery, nil, device)
device := &Device{}
device.SetID(deviceID)
resp, err := s.Hydrate(device, opts)
if err != nil {
return nil, resp, err
device = nil
}
return device, resp, err
}
Expand All @@ -518,7 +532,7 @@ func (s *DeviceServiceOp) Create(createRequest *DeviceCreateRequest) (*Device, *
apiPath := path.Join(projectBasePath, createRequest.ProjectID, deviceBasePath)
device := new(Device)

resp, err := s.client.DoRequest("POST", apiPath, createRequest, device)
resp, err := s.client.DoRequest(http.MethodPost, apiPath, createRequest, device)
if err != nil {
return nil, resp, err
}
Expand All @@ -531,12 +545,12 @@ func (s *DeviceServiceOp) Update(deviceID string, updateRequest *DeviceUpdateReq
return nil, nil, validateErr
}
opts := &GetOptions{}
opts = opts.Including("facility")
opts = opts.Including(s.DefaultIncludes()...)
endpointPath := path.Join(deviceBasePath, deviceID)
apiPathQuery := opts.WithQuery(endpointPath)
device := new(Device)

resp, err := s.client.DoRequest("PUT", apiPathQuery, updateRequest, device)
resp, err := s.client.DoRequest(http.MethodPut, apiPathQuery, updateRequest, device)
if err != nil {
return nil, resp, err
}
Expand All @@ -552,7 +566,7 @@ func (s *DeviceServiceOp) Delete(deviceID string, force bool) (*Response, error)
apiPath := path.Join(deviceBasePath, deviceID)
req := &DeviceDeleteRequest{Force: force}

return s.client.DoRequest("DELETE", apiPath, req, nil)
return s.client.DoRequest(http.MethodDelete, apiPath, req, nil)
}

// Reboot reboots on a device
Expand All @@ -563,7 +577,7 @@ func (s *DeviceServiceOp) Reboot(deviceID string) (*Response, error) {
apiPath := path.Join(deviceBasePath, deviceID, "actions")
action := &DeviceActionRequest{Type: "reboot"}

return s.client.DoRequest("POST", apiPath, action, nil)
return s.client.DoRequest(http.MethodPost, apiPath, action, nil)
}

// Reinstall reinstalls a device
Expand All @@ -585,7 +599,7 @@ func (s *DeviceServiceOp) PowerOff(deviceID string) (*Response, error) {
apiPath := path.Join(deviceBasePath, deviceID, "actions")
action := &DeviceActionRequest{Type: "power_off"}

return s.client.DoRequest("POST", apiPath, action, nil)
return s.client.DoRequest(http.MethodPost, apiPath, action, nil)
}

// PowerOn powers on a device
Expand All @@ -596,7 +610,7 @@ func (s *DeviceServiceOp) PowerOn(deviceID string) (*Response, error) {
apiPath := path.Join(deviceBasePath, deviceID, "actions")
action := &DeviceActionRequest{Type: "power_on"}

return s.client.DoRequest("POST", apiPath, action, nil)
return s.client.DoRequest(http.MethodPost, apiPath, action, nil)
}

type lockType struct {
Expand All @@ -611,7 +625,7 @@ func (s *DeviceServiceOp) Lock(deviceID string) (*Response, error) {
apiPath := path.Join(deviceBasePath, deviceID)
action := lockType{Locked: true}

return s.client.DoRequest("PATCH", apiPath, action, nil)
return s.client.DoRequest(http.MethodPatch, apiPath, action, nil)
}

// Unlock sets a device to "unlocked"
Expand All @@ -622,7 +636,7 @@ func (s *DeviceServiceOp) Unlock(deviceID string) (*Response, error) {
apiPath := path.Join(deviceBasePath, deviceID)
action := lockType{Locked: false}

return s.client.DoRequest("PATCH", apiPath, action, nil)
return s.client.DoRequest(http.MethodPatch, apiPath, action, nil)
}

func (s *DeviceServiceOp) ListBGPNeighbors(deviceID string, opts *ListOptions) ([]BGPNeighbor, *Response, error) {
Expand All @@ -633,7 +647,7 @@ func (s *DeviceServiceOp) ListBGPNeighbors(deviceID string, opts *ListOptions) (
endpointPath := path.Join(deviceBasePath, deviceID, bgpNeighborsBasePath)
apiPathQuery := opts.WithQuery(endpointPath)

resp, err := s.client.DoRequest("GET", apiPathQuery, nil, root)
resp, err := s.client.DoRequest(http.MethodGet, apiPathQuery, nil, root)
if err != nil {
return nil, resp, err
}
Expand All @@ -653,7 +667,7 @@ func (s *DeviceServiceOp) ListBGPSessions(deviceID string, opts *ListOptions) (b
for {
subset := new(bgpSessionsRoot)

resp, err = s.client.DoRequest("GET", apiPathQuery, nil, subset)
resp, err = s.client.DoRequest(http.MethodGet, apiPathQuery, nil, subset)
if err != nil {
return nil, resp, err
}
Expand Down
6 changes: 3 additions & 3 deletions devices_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ func TestAccDeviceAssignGlobalIP(t *testing.T) {
}
t.Fatalf("assignment %s should be listed in device %s", assignment, d)

if assignment.AssignedTo.Href != d.Href {
if *assignment.AssignedTo.Href != d.GetHref() {
t.Fatalf("device %s should be listed in assignment %s",
d, assignment)
}
Expand Down Expand Up @@ -587,7 +587,7 @@ func TestAccDeviceAssignIP(t *testing.T) {
}
t.Fatalf("assignment %s should be listed in device %s", assignment, d)

if assignment.AssignedTo.Href != d.Href {
if *assignment.AssignedTo.Href != d.GetHref() {
t.Fatalf("device %s should be listed in assignment %s",
d, assignment)
}
Expand Down Expand Up @@ -741,7 +741,7 @@ func TestAccDeviceAttachVolume(t *testing.T) {
t.Fatalf("wrong volume href in the attachment: %s, should be %s", a.Volume.Href, v.ID)
}

if path.Base(a.Device.Href) != d.ID {
if path.Base(a.Device.GetHref()) != d.ID {
t.Fatalf("wrong device href in the attachment: %s, should be %s", a.Device.Href, d.ID)
}

Expand Down
3 changes: 2 additions & 1 deletion hardware_reservations_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ func TestHardwareReservationServiceOp_List(t *testing.T) {
v.Meta.CurrentPageNum = page
if page < v.Meta.Total {
nextPage := page + 1
v.Meta.Next = &Href{Href: fmt.Sprintf("%s?page=%d", u.Path, nextPage)}
nextHref := fmt.Sprintf("%s?page=%d", u.Path, nextPage)
v.Meta.Next = &Href{Href: &nextHref}
}
return &Response{}, nil
}
Expand Down
13 changes: 13 additions & 0 deletions href/href.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package href

import "strings"

type Hrefer interface {
GetHref() string
}

func ParseID(obj Hrefer) string {
href := obj.GetHref()
parts := strings.Split(href, "/")
return parts[len(parts)-1]
}
6 changes: 3 additions & 3 deletions ip_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestAccPublicIPReservation(t *testing.T) {
quantityToMask[quantity], res.CIDR)
}

if path.Base(res.Project.Href) != projectID {
if path.Base(res.Project.GetHref()) != projectID {
t.Fatalf("Wrong project linked in reserved block: %s", res.Project.Href)
}

Expand Down Expand Up @@ -153,7 +153,7 @@ func TestAccGlobalIPReservation(t *testing.T) {
quantityToMask[quantity], res.CIDR)
}

if path.Base(res.Project.Href) != projectID {
if path.Base(res.Project.GetHref()) != projectID {
t.Fatalf("Wrong project linked in reserved block: %s", res.Project.Href)
}

Expand Down Expand Up @@ -280,7 +280,7 @@ func TestAccPublicMetroIPReservation(t *testing.T) {
quantityToMask[quantity], res.CIDR)
}

if path.Base(res.Project.Href) != projectID {
if path.Base(res.Project.GetHref()) != projectID {
t.Fatalf("Wrong project linked in reserved block: %s", res.Project.Href)
}

Expand Down
25 changes: 19 additions & 6 deletions organizations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type organizationsRoot struct {

// Organization represents an Equinix Metal organization
type Organization struct {
*Href `json:",inline"`
ID string `json:"id"`
Name string `json:"name,omitempty"`
Description string `json:"description,omitempty"`
Expand All @@ -45,10 +46,19 @@ type Organization struct {
Owners []User `json:"owners,omitempty"`
}

func (o Organization) String() string {
func (o *Organization) String() string {
return Stringify(o)
}

func (o *Organization) SetHref(href string) {
o.Href = &Href{Href: &href}
}

func (o *Organization) SetID(id string) {
o.ID = id
o.SetHref(path.Join(projectBasePath, id))
}

// OrganizationCreateRequest type used to create an Equinix Metal organization
type OrganizationCreateRequest struct {
Name string `json:"name"`
Expand Down Expand Up @@ -77,7 +87,7 @@ func (o OrganizationUpdateRequest) String() string {

// OrganizationServiceOp implements OrganizationService
type OrganizationServiceOp struct {
client *Client
*serviceOp
}

// List returns the user's organizations
Expand All @@ -101,16 +111,19 @@ func (s *OrganizationServiceOp) List(opts *ListOptions) (orgs []Organization, re
}
}

func (s *OrganizationServiceOp) DefaultIncludes() []string {
return []string{}
}

// Get returns a organization by id
func (s *OrganizationServiceOp) Get(organizationID string, opts *GetOptions) (*Organization, *Response, error) {
if validateErr := ValidateUUID(organizationID); validateErr != nil {
return nil, nil, validateErr
}
endpointPath := path.Join(organizationBasePath, organizationID)
apiPathQuery := opts.WithQuery(endpointPath)
organization := new(Organization)

resp, err := s.client.DoRequest("GET", apiPathQuery, nil, organization)
href := path.Join(organizationBasePath, organizationID)
Copy link
Contributor Author

@displague displague May 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If we implement a GetBasePath() string for *ServiceOp, and NewResource() interface{} // return new(Organization), we could implement Get in a ServiceOp type embedded in {Every}ServiceOp.

We would get Get and Hydrate for free (using the ServiceOp.Get implementation) in each endpoint (that uses Href and ID).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we could get Delete and others for free?

Copy link
Contributor

@t0mk t0mk Jul 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It would be cool to have Get method for free. Just rarely there are still some resources with more than one ID param to lookup. not fitting the Get(string, *GetOptions) signature. Simple command grep -P "^\tGet\(" *go | grep string shows me only vlan assigments are the case at the moment (because I have the #302 PR checked out). But I think API keys are special too.

Then, I'm not sure how/where we'd ensure that the Get method returns the desired resource type. Right now the type is declared in the Get signature Get(DeviceID string, opts *GetOptions) (>>*Device<<, *Response, error).

So I think, for the parent interface would have to do the Hydrate method as you wrote it, and for every concrete ServiceOp we would have to implement:

  • func GetResourcePath(id string) string { return path.Join(deviceBasePath, id) }
  • func DefaultIncludes() []string { return []string{"facility", "hardware_reservation", "project"} } (this could be an attribute)
  • func NewResource() *Device { return new(Device) }
  • Get like 
    // this could be copy-pasted or template-generated to all the service opts
func (s *DeviceServiceOp) Get(id string, opts *GetOptions) (*Device, *Response, error) {
  resource := s.NewResource(s.GetResourcePath(id))        
  opts = opts.Including(s.DefaultIncludes())
  resp, err := s.Hydrate(resource, opts)
  if err != nil {
    resource = nil
  }
  return resource, resp, err
}

organization.Href = &Href{Href: &href}
resp, err := s.Hydrate(organization, opts)
if err != nil {
return nil, resp, err
}
Expand Down
13 changes: 10 additions & 3 deletions packngo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,14 @@ type Response struct {

// Href is an API link
type Href struct {
Href string `json:"href"`
Href *string `json:"href,omitempty"`
}

func (h *Href) GetHref() string {
if h == nil {
return ""
}
return *h.Href
}

func (r *Response) populateRate() {
Expand Down Expand Up @@ -371,15 +378,15 @@ func NewClientWithBaseURL(consumerToken string, apiKey string, httpClient *http.
c.Connections = &ConnectionServiceOp{client: c}
c.DeviceIPs = &DeviceIPServiceOp{client: c}
c.DevicePorts = &DevicePortServiceOp{client: c}
c.Devices = &DeviceServiceOp{client: c}
c.Devices = &DeviceServiceOp{serviceOp: &serviceOp{client: c}} // TODO: NewDeviceServiceOp(c)
c.Emails = &EmailServiceOp{client: c}
c.Events = &EventServiceOp{client: c}
c.Facilities = &FacilityServiceOp{client: c}
c.HardwareReservations = &HardwareReservationServiceOp{client: c}
c.Metros = &MetroServiceOp{client: c}
c.Notifications = &NotificationServiceOp{client: c}
c.OperatingSystems = &OSServiceOp{client: c}
c.Organizations = &OrganizationServiceOp{client: c}
c.Organizations = &OrganizationServiceOp{serviceOp: &serviceOp{client: c}}
c.Plans = &PlanServiceOp{client: c}
c.Ports = &PortServiceOp{client: c}
c.ProjectIPs = &ProjectIPServiceOp{client: c}
Expand Down
2 changes: 1 addition & 1 deletion projects_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ func TestAccProjectListSSHKeys(t *testing.T) {

for _, k := range keys {
if k.ID == key.ID {
if len(k.Owner.Href) == 0 {
if len(k.Owner.GetHref()) == 0 {
t.Error("new Key doesn't have owner URL set")
}
return
Expand Down
Loading