From 9c900e63346fcb7225a5cc221e0f581286e2c46f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20D=C3=B6tsch?= Date: Fri, 13 Feb 2026 14:04:49 +0100 Subject: [PATCH] improve MultiAttr + overall code cleanup --- LICENSE | 2 +- Makefile | 2 +- adminapi/commit.go | 22 +-- adminapi/commit_test.go | 89 ++++++---- adminapi/create_object.go | 64 ++++++++ adminapi/create_object_test.go | 248 ++++++++++++++++++++++++++++ adminapi/errors.go | 32 ++++ adminapi/filters.go | 42 +++-- adminapi/multiattr.go | 79 +++++++++ adminapi/multiattr_test.go | 285 +++++++++++++++++++++++++++++++++ adminapi/query.go | 69 +++----- adminapi/query_test.go | 8 + adminapi/server_object.go | 82 +++++++--- adminapi/server_object_test.go | 272 ++++++++++++++++++++++++++++--- adminapi/transport.go | 49 ++---- adminapi/transport_test.go | 4 + examples/helpers.go | 10 -- examples/query_examples.go | 83 +++++----- examples/real.go | 62 +++---- examples/update_example.go | 40 ++--- 20 files changed, 1250 insertions(+), 294 deletions(-) create mode 100644 adminapi/create_object.go create mode 100644 adminapi/create_object_test.go create mode 100644 adminapi/errors.go create mode 100644 adminapi/multiattr.go create mode 100644 adminapi/multiattr_test.go delete mode 100644 examples/helpers.go diff --git a/LICENSE b/LICENSE index 2be0553..14fac91 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2025 +Copyright (c) 2026 Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/Makefile b/Makefile index b848153..2c45087 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ test-race: go test -race ./... test-coverage: - go test -v ./... -coverprofile=coverage.out + go test -v ./adminapi/... -coverprofile=coverage.out go tool cover -html=coverage.out -o coverage.html linter: diff --git a/adminapi/commit.go b/adminapi/commit.go index f1b3f48..b3697a7 100644 --- a/adminapi/commit.go +++ b/adminapi/commit.go @@ -8,9 +8,9 @@ import ( // commitRequest is the payload sent to /api/dataset/commit type commitRequest struct { - Created []map[string]any `json:"created"` - Changed []map[string]any `json:"changed"` - Deleted []any `json:"deleted"` + Created []Attributes `json:"created"` + Changed []Attributes `json:"changed"` + Deleted []int `json:"deleted"` // the object-ids } type commitResponse struct { @@ -79,19 +79,21 @@ func (s *ServerObject) Commit() (int, error) { func buildCommit(objects ServerObjects) commitRequest { commit := commitRequest{ - Created: []map[string]any{}, - Changed: []map[string]any{}, - Deleted: []any{}, + Created: []Attributes{}, + Changed: []Attributes{}, + Deleted: []int{}, // the object-ids } for _, obj := range objects { switch obj.CommitState() { - case "created": + case StateCreated: commit.Created = append(commit.Created, obj.attributes) - case "changed": + case StateChanged: commit.Changed = append(commit.Changed, obj.serializeChanges()) - case "deleted": - commit.Deleted = append(commit.Deleted, obj.Get("object_id")) + case StateDeleted: + commit.Deleted = append(commit.Deleted, obj.ObjectID()) + case StateConsistent: + // No changes to commit } } diff --git a/adminapi/commit_test.go b/adminapi/commit_test.go index 9eb65aa..0d10082 100644 --- a/adminapi/commit_test.go +++ b/adminapi/commit_test.go @@ -28,8 +28,8 @@ func TestCommitSingle(t *testing.T) { t.Setenv("SERVERADMIN_BASE_URL", server.URL) obj := &ServerObject{ - attributes: map[string]any{"hostname": "new.local", "object_id": float64(42)}, - oldValues: map[string]any{"hostname": "old.local"}, + attributes: Attributes{"hostname": "new.local", "object_id": float64(42)}, + oldValues: Attributes{"hostname": "old.local"}, } commitID, err := obj.Commit() @@ -42,7 +42,7 @@ func TestCommitSingle(t *testing.T) { assert.Empty(t, receivedBody.Deleted) // State should be reset after commit - assert.Equal(t, "consistent", obj.CommitState()) + assert.Equal(t, StateConsistent, obj.CommitState()) assert.Empty(t, obj.oldValues) } @@ -64,16 +64,16 @@ func TestCommitResultSet(t *testing.T) { objects := ServerObjects{ { - attributes: map[string]any{"hostname": "changed.local", "object_id": float64(1)}, - oldValues: map[string]any{"hostname": "orig1.local"}, + attributes: Attributes{"hostname": "changed.local", "object_id": float64(1)}, + oldValues: Attributes{"hostname": "orig1.local"}, }, { - attributes: map[string]any{"hostname": "unchanged.local", "object_id": float64(2)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "unchanged.local", "object_id": float64(2)}, + oldValues: Attributes{}, }, { - attributes: map[string]any{"hostname": "deleted.local", "object_id": float64(3)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "deleted.local", "object_id": float64(3)}, + oldValues: Attributes{}, deleted: true, }, } @@ -90,12 +90,12 @@ func TestCommitResultSet(t *testing.T) { func TestServerObjectsSetSuccess(t *testing.T) { objects := ServerObjects{ { - attributes: map[string]any{"hostname": "server1", "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server1", "object_id": float64(1)}, + oldValues: Attributes{}, }, { - attributes: map[string]any{"hostname": "server2", "object_id": float64(2)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server2", "object_id": float64(2)}, + oldValues: Attributes{}, }, } @@ -111,12 +111,12 @@ func TestServerObjectsSetSuccess(t *testing.T) { func TestServerObjectsSetAllErrors(t *testing.T) { objects := ServerObjects{ { - attributes: map[string]any{"hostname": "server1", "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server1", "object_id": float64(1)}, + oldValues: Attributes{}, }, { - attributes: map[string]any{"hostname": "server2", "object_id": float64(2)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server2", "object_id": float64(2)}, + oldValues: Attributes{}, }, } @@ -126,18 +126,18 @@ func TestServerObjectsSetAllErrors(t *testing.T) { // Should contain errors for both objects assert.Contains(t, err.Error(), "object 0") assert.Contains(t, err.Error(), "object 1") - assert.Contains(t, err.Error(), "does not exist") + assert.ErrorIs(t, err, ErrUnknownAttribute) } func TestServerObjectsSetPartialErrors(t *testing.T) { objects := ServerObjects{ { - attributes: map[string]any{"hostname": "server1", "memory": 16, "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server1", "memory": 16, "object_id": float64(1)}, + oldValues: Attributes{}, }, { - attributes: map[string]any{"hostname": "server2", "object_id": float64(2)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server2", "object_id": float64(2)}, + oldValues: Attributes{}, }, } @@ -164,12 +164,12 @@ func TestServerObjectsSetEmpty(t *testing.T) { func TestServerObjectsDelete(t *testing.T) { objects := ServerObjects{ { - attributes: map[string]any{"hostname": "server1", "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server1", "object_id": float64(1)}, + oldValues: Attributes{}, }, { - attributes: map[string]any{"hostname": "server2", "object_id": float64(2)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server2", "object_id": float64(2)}, + oldValues: Attributes{}, }, } @@ -177,8 +177,8 @@ func TestServerObjectsDelete(t *testing.T) { assert.True(t, objects[0].deleted) assert.True(t, objects[1].deleted) - assert.Equal(t, "deleted", objects[0].CommitState()) - assert.Equal(t, "deleted", objects[1].CommitState()) + assert.Equal(t, StateDeleted, objects[0].CommitState()) + assert.Equal(t, StateDeleted, objects[1].CommitState()) } func TestServerObjectsDeleteEmpty(_ *testing.T) { @@ -199,12 +199,12 @@ func TestServerObjectsSetWithCommit(t *testing.T) { objects := ServerObjects{ { - attributes: map[string]any{"hostname": "server1", "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server1", "object_id": float64(1)}, + oldValues: Attributes{}, }, { - attributes: map[string]any{"hostname": "server2", "object_id": float64(2)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "server2", "object_id": float64(2)}, + oldValues: Attributes{}, }, } @@ -218,6 +218,27 @@ func TestServerObjectsSetWithCommit(t *testing.T) { assert.Equal(t, 999, commitID) // State should be consistent after commit - assert.Equal(t, "consistent", objects[0].CommitState()) - assert.Equal(t, "consistent", objects[1].CommitState()) + assert.Equal(t, StateConsistent, objects[0].CommitState()) + assert.Equal(t, StateConsistent, objects[1].CommitState()) +} + +func TestServerObjectsRollback(t *testing.T) { + objects := ServerObjects{ + { + attributes: Attributes{"hostname": "server1", "object_id": float64(1)}, + oldValues: Attributes{}, + }, + { + attributes: Attributes{"hostname": "server2", "object_id": float64(2)}, + oldValues: Attributes{}, + deleted: true, + }, + } + + objects[0].Set("hostname", "modified") + objects.Rollback() + + assert.Equal(t, "server1", objects[0].GetString("hostname")) + assert.Equal(t, StateConsistent, objects[0].CommitState()) + assert.Equal(t, StateConsistent, objects[1].CommitState()) } diff --git a/adminapi/create_object.go b/adminapi/create_object.go new file mode 100644 index 0000000..1c77359 --- /dev/null +++ b/adminapi/create_object.go @@ -0,0 +1,64 @@ +package adminapi + +import ( + "encoding/json" + "fmt" + "net/url" +) + +// NewObject creates a new server object with the given attributes, commits it, +// and returns the fully populated object with a server-assigned object_id. +// The attributes map must include "hostname". +func NewObject(serverType string, attributes Attributes) (*ServerObject, error) { + if !attributes.Has("hostname") { + return nil, fmt.Errorf("attributes must include %q: %w", "hostname", ErrUnknownAttribute) + } + + server := &ServerObject{ + oldValues: Attributes{}, + } + + // Fetch default attributes from the API + params := url.Values{} + params.Add("servertype", serverType) + fullURL := apiEndpointNewObject + "?" + params.Encode() + + resp, err := sendRequest(fullURL, nil) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + var response struct { + Result Attributes `json:"result"` + } + if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { + return nil, err + } + server.attributes = response.Result + + // Ensure object_id is nil so CommitState() returns "created" + server.attributes["object_id"] = nil + + // Apply caller-provided attributes (validates each exists in schema) + for key, value := range attributes { + if err := server.Set(key, value); err != nil { + return nil, fmt.Errorf("setting attribute %q: %w", key, err) + } + } + + // Commit the new object + if _, err := server.Commit(); err != nil { + return nil, fmt.Errorf("committing new object: %w", err) + } + + // Re-query to get the server-assigned object_id + q := NewQuery(Filters{"hostname": attributes["hostname"]}) + created, err := q.One() + if err != nil { + return nil, fmt.Errorf("re-querying created object: %w", err) + } + _ = server.Set("object_id", created.ObjectID()) + + return created, nil +} diff --git a/adminapi/create_object_test.go b/adminapi/create_object_test.go new file mode 100644 index 0000000..f326aae --- /dev/null +++ b/adminapi/create_object_test.go @@ -0,0 +1,248 @@ +package adminapi + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewObject(t *testing.T) { + tests := []struct { + name string + serverType string + attributes Attributes + newObjResp string + commitResp string + queryResp string + expectError bool + expectedErrIs string + }{ + { + name: "successful object creation", + serverType: "vm", + attributes: Attributes{ + "hostname": "new-server.local", + "environment": "development", + }, + newObjResp: `{ + "status": "success", + "result": { + "hostname": "", + "environment": "", + "num_cpu": 4, + "memory": 8192, + "servertype": "vm" + } + }`, + commitResp: `{"status": "success", "commit_id": 42}`, + queryResp: `{ + "status": "success", + "result": [{ + "object_id": 12345, + "hostname": "new-server.local", + "environment": "development" + }] + }`, + }, + { + name: "minimal attributes", + serverType: "loadbalancer", + attributes: Attributes{ + "hostname": "lb-1.local", + }, + newObjResp: `{ + "status": "success", + "result": { + "hostname": "", + "servertype": "loadbalancer" + } + }`, + commitResp: `{"status": "success", "commit_id": 43}`, + queryResp: `{ + "status": "success", + "result": [{ + "object_id": 99, + "hostname": "lb-1.local" + }] + }`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + callCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + switch r.URL.Path { + case "/api/dataset/new_object": + assert.Equal(t, tt.serverType, r.URL.Query().Get("servertype")) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(tt.newObjResp)) + case "/api/dataset/commit": + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(tt.commitResp)) + case "/api/dataset/query": + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(tt.queryResp)) + default: + t.Fatalf("unexpected request to %s", r.URL.Path) + } + })) + defer server.Close() + + resetConfig() + t.Setenv("SERVERADMIN_TOKEN", "test-token-1234") + t.Setenv("SERVERADMIN_BASE_URL", server.URL) + + obj, err := NewObject(tt.serverType, tt.attributes) + + require.NoError(t, err) + require.NotNil(t, obj) + + // Verify object_id is set from re-query + assert.NotNil(t, obj.Get("object_id"), "object_id should be set after creation") + assert.Positive(t, obj.ObjectID(), "object_id should be positive") + + // Verify hostname is present + assert.Equal(t, tt.attributes["hostname"], obj.GetString("hostname")) + + // Verify state is consistent (committed) + assert.Equal(t, StateConsistent, obj.CommitState()) + + // Verify all 3 API calls were made + assert.Equal(t, 3, callCount, "should make new_object, commit, and query calls") + }) + } +} + +func TestNewObject_MissingHostname(t *testing.T) { + obj, err := NewObject("vm", Attributes{"environment": "dev"}) + + require.Error(t, err) + assert.Nil(t, obj) + require.ErrorIs(t, err, ErrUnknownAttribute) + assert.Contains(t, err.Error(), "hostname") +} + +func TestNewObject_UnknownAttribute(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{ + "status": "success", + "result": { + "hostname": "", + "servertype": "vm" + } + }`)) + })) + defer server.Close() + + resetConfig() + t.Setenv("SERVERADMIN_TOKEN", "test-token-1234") + t.Setenv("SERVERADMIN_BASE_URL", server.URL) + + obj, err := NewObject("vm", Attributes{ + "hostname": "test.local", + "nonexistent_field": "value", + }) + + require.Error(t, err) + assert.Nil(t, obj) + assert.Contains(t, err.Error(), "nonexistent_field") +} + +func TestNewObject_HTTPError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error": {"message": "Bad Request: Invalid servertype"}}`)) + })) + defer server.Close() + + resetConfig() + t.Setenv("SERVERADMIN_TOKEN", "test-token-1234") + t.Setenv("SERVERADMIN_BASE_URL", server.URL) + + obj, err := NewObject("invalid-type", Attributes{"hostname": "test.local"}) + + require.Error(t, err) + assert.Nil(t, obj) + assert.Contains(t, err.Error(), "HTTP error 400 Bad Request") +} + +func TestNewObject_CommitFailure(t *testing.T) { + callCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + switch r.URL.Path { + case "/api/dataset/new_object": + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{ + "status": "success", + "result": {"hostname": "", "servertype": "vm"} + }`)) + case "/api/dataset/commit": + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"status": "error", "message": "validation failed"}`)) + default: + t.Fatalf("unexpected request to %s", r.URL.Path) + } + })) + defer server.Close() + + resetConfig() + t.Setenv("SERVERADMIN_TOKEN", "test-token-1234") + t.Setenv("SERVERADMIN_BASE_URL", server.URL) + + obj, err := NewObject("vm", Attributes{"hostname": "test.local"}) + + require.Error(t, err) + assert.Nil(t, obj) + assert.Contains(t, err.Error(), "committing new object") + + // Should not have made the query call + assert.Equal(t, 2, callCount, "should only make new_object and commit calls") +} + +func TestNewObject_CommitPayload(t *testing.T) { + var receivedCommit commitRequest + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/dataset/new_object": + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{ + "status": "success", + "result": {"hostname": "", "servertype": "vm", "project": ""} + }`)) + case "/api/dataset/commit": + _ = json.NewDecoder(r.Body).Decode(&receivedCommit) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"status": "success", "commit_id": 1}`)) + case "/api/dataset/query": + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{ + "status": "success", + "result": [{"object_id": 1, "hostname": "test.local", "project": "admin"}] + }`)) + } + })) + defer server.Close() + + resetConfig() + t.Setenv("SERVERADMIN_TOKEN", "test-token-1234") + t.Setenv("SERVERADMIN_BASE_URL", server.URL) + + _, err := NewObject("vm", Attributes{ + "hostname": "test.local", + "project": "admin", + }) + require.NoError(t, err) + + // Verify the commit payload contains a created object + require.Len(t, receivedCommit.Created, 1) + assert.Equal(t, "test.local", receivedCommit.Created[0]["hostname"]) + assert.Equal(t, "admin", receivedCommit.Created[0]["project"]) +} diff --git a/adminapi/errors.go b/adminapi/errors.go new file mode 100644 index 0000000..1b887f4 --- /dev/null +++ b/adminapi/errors.go @@ -0,0 +1,32 @@ +package adminapi + +import ( + "errors" + "fmt" +) + +var ( + // ErrNoResults is returned by One() when the query matches zero objects. + ErrNoResults = errors.New("no server objects found") + + // ErrMultipleResults is returned by One() when the query matches more than one object. + ErrMultipleResults = errors.New("expected exactly one server object, got multiple") + + // ErrUnknownAttribute is returned by Set() when the attribute does not exist on the object. + ErrUnknownAttribute = errors.New("unknown attribute") +) + +// APIError represents an HTTP error response from the Serveradmin API. +// Use errors.As() to inspect status codes and messages from API failures. +type APIError struct { + StatusCode int + Status string + Message string +} + +func (e *APIError) Error() string { + if e.Message != "" { + return fmt.Sprintf("HTTP error %d %s: %s", e.StatusCode, e.Status, e.Message) + } + return fmt.Sprintf("HTTP error %d %s", e.StatusCode, e.Status) +} diff --git a/adminapi/filters.go b/adminapi/filters.go index f5d6318..b1a2ad6 100644 --- a/adminapi/filters.go +++ b/adminapi/filters.go @@ -1,8 +1,13 @@ package adminapi type ( + // Filters maps attribute names to filter values or Filter objects. + // Used as the top-level query predicate: Filters{"hostname": Regexp("web.*"), "state": "online"}. Filters map[string]any - Filter map[string]any + + // Filter represents a single filter operation like Regexp, Not, or Any. + // Unlike Filters, a Filter wraps a named operation with its argument. + Filter map[string]any ) type value interface { @@ -30,62 +35,77 @@ var allFilters = map[string]string{ "startswith": "StartsWith", } -func Regexp(value string) Filter { - return createFilter("Regexp", value) -} - +// Not creates a filter that negates the given filter or value. For example, Not(2) means "!= 2". func Not[V valueOrFilter](filter V) Filter { return createFilter("Not", filter) } +// NotEmpty is a shortcut function for Not(Empty()) func NotEmpty() Filter { return Not(Empty()) } +// Empty matches attributes that have no value (nil or empty). +func Empty() Filter { + return createFilter("Empty", nil) +} + +// Any matches if the attribute matches any of the given values or filters (OR semantics). func Any[V valueOrFilter](values ...V) Filter { return createFilter("Any", values) } +// All matches if the attribute matches all of the given values or filters (AND semantics). func All[V valueOrFilter](values ...V) Filter { return createFilter("All", values) } -func Empty() Filter { - return createFilter("Empty", nil) +// Regexp matches the attribute value against the given regular expression pattern. +func Regexp(value string) Filter { + return createFilter("Regexp", value) } +// StartsWith matches attributes whose value begins with the given prefix. func StartsWith(value string) Filter { return createFilter("StartsWith", value) } -func GreaterThan[N int | float64](value N) Filter { +// GreaterThan matches attributes with a numeric value strictly greater than the given value. +func GreaterThan(value int) Filter { return createFilter("GreaterThan", value) } -func GreaterThanOrEquals[N int | float64](value N) Filter { +// GreaterThanOrEquals matches attributes with a numeric value greater than or equal to the given value. +func GreaterThanOrEquals(value int) Filter { return createFilter("GreaterThanOrEquals", value) } -func LessThan[N int | float64](value N) Filter { +// LessThan matches attributes with a numeric value strictly less than the given value. +func LessThan(value int) Filter { return createFilter("LessThan", value) } -func LessThanOrEquals[N int | float64](value N) Filter { +// LessThanOrEquals matches attributes with a numeric value less than or equal to the given value. +func LessThanOrEquals(value int) Filter { return createFilter("LessThanOrEquals", value) } +// Contains matches multi-valued attributes that contain the given value. func Contains[V valueOrFilter](value V) Filter { return createFilter("Contains", value) } +// ContainedBy matches attributes whose values are a subset of the given value. func ContainedBy[V valueOrFilter](value V) Filter { return createFilter("ContainedBy", value) } +// ContainedOnlyBy matches attributes whose values are exclusively contained by the given value. func ContainedOnlyBy[V valueOrFilter](value V) Filter { return createFilter("ContainedOnlyBy", value) } +// Overlaps matches multi-valued attributes that share at least one element with the given value. func Overlaps[V valueOrFilter](value V) Filter { return createFilter("Overlaps", value) } diff --git a/adminapi/multiattr.go b/adminapi/multiattr.go new file mode 100644 index 0000000..b47374f --- /dev/null +++ b/adminapi/multiattr.go @@ -0,0 +1,79 @@ +package adminapi + +// MultiAttr is a helper type for multi-valued attributes. +// It provides set-like operations on string slices. +// +// MultiAttr maintains set semantics: Add prevents duplicates, Delete removes all +// occurrences. Changes are made in-place but do NOT automatically update a ServerObject. +// Users must call obj.Set() manually after modifications. +// +// Example usage: +// +// tags := serverObject.GetMulti("tags") +// tags.Add("web", "prod") +// tags.Delete("old-tag") +// obj.Set("tags", tags) +type MultiAttr []string + +// Add appends elements to the MultiAttr, preventing duplicates (set semantics). +// If an element already exists, it is not added again. Handles nil receiver +// by allocating a new slice. +// +// Example: +// +// m := MultiAttr{"web", "prod"} +// m.Add("api", "web") // Only "api" is added (web already exists) +// // m is now ["web", "prod", "api"] +func (m *MultiAttr) Add(elems ...string) { + for _, elem := range elems { + if !m.Contains(elem) { + *m = append(*m, elem) + } + } +} + +// Delete removes all occurrences of the element from the MultiAttr. +// If the element doesn't exist, this is a no-op. +// +// Example: +// +// m := MultiAttr{"web", "prod", "web"} +// m.Delete("web") +// // m is now ["prod"] +func (m *MultiAttr) Delete(elem string) { + filtered := make(MultiAttr, 0, len(*m)) + for _, v := range *m { + if v != elem { + filtered = append(filtered, v) + } + } + *m = filtered +} + +// Clear removes all elements from the MultiAttr, resulting in an empty slice. +// +// Example: +// +// m := MultiAttr{"web", "prod"} +// m.Clear() +// // m is now [] +func (m *MultiAttr) Clear() { + *m = MultiAttr{} +} + +// Contains returns true if the element exists in the MultiAttr. +// Returns false for nil or empty slices. +// +// Example: +// +// m := MultiAttr{"web", "prod"} +// m.Contains("web") // true +// m.Contains("api") // false +func (m MultiAttr) Contains(elem string) bool { + for _, v := range m { + if v == elem { + return true + } + } + return false +} diff --git a/adminapi/multiattr_test.go b/adminapi/multiattr_test.go new file mode 100644 index 0000000..3b23452 --- /dev/null +++ b/adminapi/multiattr_test.go @@ -0,0 +1,285 @@ +package adminapi + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMultiAttr_Add(t *testing.T) { + tests := []struct { + name string + initial MultiAttr + toAdd []string + expected MultiAttr + }{ + { + name: "add single element to existing", + initial: MultiAttr{"web", "prod"}, + toAdd: []string{"api"}, + expected: MultiAttr{"web", "prod", "api"}, + }, + { + name: "add multiple elements at once", + initial: MultiAttr{"web"}, + toAdd: []string{"api", "prod", "monitoring"}, + expected: MultiAttr{"web", "api", "prod", "monitoring"}, + }, + { + name: "add duplicates (should skip)", + initial: MultiAttr{"web", "prod"}, + toAdd: []string{"web", "api", "prod"}, + expected: MultiAttr{"web", "prod", "api"}, + }, + { + name: "add to empty MultiAttr", + initial: MultiAttr{}, + toAdd: []string{"web", "prod"}, + expected: MultiAttr{"web", "prod"}, + }, + { + name: "add nothing", + initial: MultiAttr{"web"}, + toAdd: []string{}, + expected: MultiAttr{"web"}, + }, + { + name: "add empty string", + initial: MultiAttr{"web"}, + toAdd: []string{""}, + expected: MultiAttr{"web", ""}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := tt.initial + m.Add(tt.toAdd...) + assert.Equal(t, tt.expected, m) + }) + } +} + +func TestMultiAttr_AddToNil(t *testing.T) { + var m MultiAttr + m.Add("web", "prod") + assert.Equal(t, MultiAttr{"web", "prod"}, m) +} + +func TestMultiAttr_Delete(t *testing.T) { + tests := []struct { + name string + initial MultiAttr + toDelete string + expected MultiAttr + }{ + { + name: "delete existing element", + initial: MultiAttr{"web", "prod", "api"}, + toDelete: "prod", + expected: MultiAttr{"web", "api"}, + }, + { + name: "delete non-existent element (no-op)", + initial: MultiAttr{"web", "prod"}, + toDelete: "api", + expected: MultiAttr{"web", "prod"}, + }, + { + name: "delete all occurrences of duplicate", + initial: MultiAttr{"web", "prod", "web", "api"}, + toDelete: "web", + expected: MultiAttr{"prod", "api"}, + }, + { + name: "delete from empty MultiAttr (no-op)", + initial: MultiAttr{}, + toDelete: "web", + expected: MultiAttr{}, + }, + { + name: "delete last element", + initial: MultiAttr{"web"}, + toDelete: "web", + expected: MultiAttr{}, + }, + { + name: "delete empty string", + initial: MultiAttr{"web", "", "prod"}, + toDelete: "", + expected: MultiAttr{"web", "prod"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := tt.initial + m.Delete(tt.toDelete) + assert.Equal(t, tt.expected, m) + }) + } +} + +func TestMultiAttr_Clear(t *testing.T) { + tests := []struct { + name string + initial MultiAttr + }{ + { + name: "clear non-empty MultiAttr", + initial: MultiAttr{"web", "prod", "api"}, + }, + { + name: "clear already empty MultiAttr", + initial: MultiAttr{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := tt.initial + m.Clear() + assert.Empty(t, m) + assert.Equal(t, MultiAttr{}, m) + }) + } +} + +func TestMultiAttr_ClearNil(t *testing.T) { + var m MultiAttr + m.Clear() // Should not panic + assert.Empty(t, m) + assert.Equal(t, MultiAttr{}, m) +} + +func TestMultiAttr_Contains(t *testing.T) { + tests := []struct { + name string + m MultiAttr + elem string + expected bool + }{ + { + name: "contains existing element", + m: MultiAttr{"web", "prod", "api"}, + elem: "prod", + expected: true, + }, + { + name: "contains non-existent element", + m: MultiAttr{"web", "prod"}, + elem: "api", + expected: false, + }, + { + name: "contains on empty MultiAttr", + m: MultiAttr{}, + elem: "web", + expected: false, + }, + { + name: "contains on nil MultiAttr", + m: nil, + elem: "web", + expected: false, + }, + { + name: "contains empty string", + m: MultiAttr{"web", "", "prod"}, + elem: "", + expected: true, + }, + { + name: "case sensitive", + m: MultiAttr{"web", "PROD"}, + elem: "prod", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.m.Contains(tt.elem) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestMultiAttr_Integration(t *testing.T) { + // Simulate full workflow with ServerObject + obj := &ServerObject{ + attributes: Attributes{ + "tags": []any{"web", "old-tag"}, + "object_id": float64(42), + }, + oldValues: Attributes{}, + } + + // Get tags as MultiAttr + tags := obj.GetMulti("tags") + + // Manipulate with MultiAttr methods + tags.Add("api", "prod") + tags.Delete("old-tag") + + // Verify state before Set + assert.True(t, tags.Contains("web")) + assert.True(t, tags.Contains("api")) + assert.True(t, tags.Contains("prod")) + assert.False(t, tags.Contains("old-tag")) + + // Set back to ServerObject + err := obj.Set("tags", []string(tags)) + require.NoError(t, err) + + // Serialize changes and verify correct add/remove sets + changes := obj.serializeChanges() + + tagChange, ok := changes["tags"].(map[string]any) + require.True(t, ok, "tags change should be a map") + assert.Equal(t, "multi", tagChange["action"]) + + add := tagChange["add"].([]any) + remove := tagChange["remove"].([]any) + + // Should add: api, prod + // Should remove: old-tag + // Should keep: web (unchanged) + assert.ElementsMatch(t, []any{"api", "prod"}, add) + assert.ElementsMatch(t, []any{"old-tag"}, remove) +} + +func TestMultiAttr_ChainedOperations(t *testing.T) { + // Test complex sequences of operations + m := MultiAttr{"web", "prod"} + + m.Add("api") + assert.ElementsMatch(t, []string{"web", "prod", "api"}, m) + + m.Add("web") // Duplicate, should not add + assert.Len(t, m, 3) + + m.Delete("prod") + assert.ElementsMatch(t, []string{"web", "api"}, m) + + m.Add("monitoring", "logging") + assert.ElementsMatch(t, []string{"web", "api", "monitoring", "logging"}, m) + + m.Clear() + assert.Empty(t, m) + + m.Add("new") + assert.Equal(t, MultiAttr{"new"}, m) +} + +func TestMultiAttr_OrderPreservation(t *testing.T) { + // While we use set semantics, order should be preserved for existing elements + m := MultiAttr{"first", "second", "third"} + + m.Add("fourth") + assert.Equal(t, MultiAttr{"first", "second", "third", "fourth"}, m) + + m.Delete("second") + assert.Equal(t, MultiAttr{"first", "third", "fourth"}, m) +} diff --git a/adminapi/query.go b/adminapi/query.go index f6f148b..e5acffc 100644 --- a/adminapi/query.go +++ b/adminapi/query.go @@ -3,7 +3,6 @@ package adminapi import ( "encoding/json" "fmt" - "net/url" "slices" ) @@ -16,6 +15,15 @@ type Query struct { serverObjects ServerObjects } +// Attributes is a map of attributes, indexed by attribute name +type Attributes map[string]any + +// Has checks if the given key exists in the attributes map +func (a Attributes) Has(key string) bool { + _, ok := a[key] + return ok +} + // FromQuery creates a new Query object from a query string func FromQuery(query string) (Query, error) { filters, err := ParseQuery(query) @@ -75,17 +83,21 @@ func (q *Query) All() (ServerObjects, error) { } // One returns exactly one matching SA object. If there is none or more than one, an error is returned. +// Returns ErrNoResults if no objects match, or a wrapped ErrMultipleResults if more than one matches. func (q *Query) One() (*ServerObject, error) { err := q.load() if err != nil { return nil, err } - if len(q.serverObjects) != 1 { - return nil, fmt.Errorf("expected exactly one server object, got %d", len(q.serverObjects)) + switch len(q.serverObjects) { + case 1: + return q.serverObjects[0], nil + case 0: + return nil, ErrNoResults + default: + return nil, fmt.Errorf("got %d: %w", len(q.serverObjects), ErrMultipleResults) } - - return q.serverObjects[0], nil } func (q *Query) load() error { @@ -101,60 +113,31 @@ func (q *Query) load() error { request := queryRequest{ Filters: q.filters, Restricted: q.restrictedAttributes, - OrderBy: q.orderBy, + OrderBy: q.orderBy, // todo fix serverside ordering in API or do it on client side } resp, err := sendRequest(apiEndpointQuery, request) if err != nil { - return err + return fmt.Errorf("querying %s: %w", apiEndpointQuery, err) } defer resp.Body.Close() respServer := queryResponse{} - err = json.NewDecoder(resp.Body).Decode(&respServer) + if err = json.NewDecoder(resp.Body).Decode(&respServer); err != nil { + return fmt.Errorf("decoding query response: %w", err) + } // map attribute map into ServerObject objects q.serverObjects = make(ServerObjects, len(respServer.Result)) for idx, object := range respServer.Result { q.serverObjects[idx] = &ServerObject{ attributes: object, - oldValues: map[string]any{}, + oldValues: Attributes{}, } } q.loaded = true - return err -} - -// NewObject creates a new server object (fetches default attributes from SA) -func NewObject(serverType string) (*ServerObject, error) { - server := &ServerObject{ - oldValues: map[string]any{}, - } - - // Use url.Values for safe query string encoding - params := url.Values{} - params.Add("servertype", serverType) - fullURL := apiEndpointNewObject + "?" + params.Encode() - - resp, err := sendRequest(fullURL, nil) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - var response struct { - Result map[string]any `json:"result"` - } - if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { - return nil, err - } - server.attributes = response.Result - - // Ensure object_id is nil so CommitState() returns "created" - server.attributes["object_id"] = nil - - return server, nil + return nil } // like {"Filters": {"hostname": {"Regexp": "foo.local.*"}}, "restrict": ["hostname", "object_id"]} @@ -166,6 +149,6 @@ type queryRequest struct { // like {"status": "success", "result": [{"object_id": 483903, "hostname": "foo.local"}]} type queryResponse struct { - Status string `json:"status"` - Result []map[string]any `json:"result"` + Status string `json:"status"` + Result []Attributes `json:"result"` } diff --git a/adminapi/query_test.go b/adminapi/query_test.go index 24fc7bd..7f7e503 100644 --- a/adminapi/query_test.go +++ b/adminapi/query_test.go @@ -60,3 +60,11 @@ func TestFromQuery(t *testing.T) { "instance": 1, }, q.filters) } + +func TestFromQueryWithError(t *testing.T) { + q, err := FromQuery("hostname=not(empty(") + + require.Error(t, err) + assert.Contains(t, err.Error(), "unmatched ( found") + assert.Equal(t, Query{}, q, "query should be zero value on error") +} diff --git a/adminapi/server_object.go b/adminapi/server_object.go index ae6ad14..835d32f 100644 --- a/adminapi/server_object.go +++ b/adminapi/server_object.go @@ -11,8 +11,8 @@ type ServerObjects []*ServerObject // ServerObject is a map of key-value attributes of a SA object type ServerObject struct { - attributes map[string]any - oldValues map[string]any // tracks original values before first modification + attributes Attributes + oldValues Attributes // tracks original values before first modification deleted bool } @@ -36,6 +36,32 @@ func (s *ServerObject) GetString(attribute string) string { return "" } +// GetMulti safely retrieves a multi-valued attribute as a MultiAttr. +// Returns an empty MultiAttr if the attribute is missing, nil, or not a slice of strings. +func (s *ServerObject) GetMulti(attribute string) MultiAttr { + val, ok := s.attributes[attribute] + if !ok || val == nil { + return MultiAttr{} + } + + switch v := val.(type) { + case MultiAttr: + return v + case []string: + return v + case []any: + result := make(MultiAttr, 0, len(v)) + for _, elem := range v { + if str, ok := elem.(string); ok { + result = append(result, str) + } + } + return result + default: + return MultiAttr{} + } +} + // ObjectID returns the "object_id" attribute of the ServerObject func (s *ServerObject) ObjectID() int { val := s.Get("object_id") @@ -45,10 +71,24 @@ func (s *ServerObject) ObjectID() int { return 0 } +// CommitState represents the state of a ServerObject with respect to pending changes. +type CommitState string + +const ( + // StateCreated indicates the object is new and has not been committed yet. + StateCreated CommitState = "created" + // StateDeleted indicates the object has been marked for deletion. + StateDeleted CommitState = "deleted" + // StateChanged indicates the object has local modifications pending commit. + StateChanged CommitState = "changed" + // StateConsistent indicates the object has no pending changes. + StateConsistent CommitState = "consistent" +) + // Set modifies an attribute value and tracks the change for commit. func (s *ServerObject) Set(key string, value any) error { if _, exists := s.attributes[key]; !exists { - return fmt.Errorf("attribute %q does not exist", key) + return fmt.Errorf("attribute %q: %w", key, ErrUnknownAttribute) } // Save the original value on first modification only @@ -73,35 +113,35 @@ func (s *ServerObject) Delete() { s.deleted = true } -// CommitState returns the current state: "created", "deleted", "changed", or "consistent". -func (s *ServerObject) CommitState() string { +// Rollback reverts all local changes, restoring original attribute values. +func (s *ServerObject) Rollback() { + s.deleted = false + for key, oldVal := range s.oldValues { + s.attributes[key] = oldVal + } + s.oldValues = Attributes{} +} + +// CommitState returns the current state of the object with respect to pending changes. +func (s *ServerObject) CommitState() CommitState { if s.attributes["object_id"] == nil { - return "created" + return StateCreated } if s.deleted { - return "deleted" + return StateDeleted } for key, oldVal := range s.oldValues { newVal := s.attributes[key] if !jsonEqual(oldVal, newVal) { - return "changed" + return StateChanged } } - return "consistent" -} - -// Rollback reverts all local changes, restoring original attribute values. -func (s *ServerObject) Rollback() { - s.deleted = false - for key, oldVal := range s.oldValues { - s.attributes[key] = oldVal - } - s.oldValues = map[string]any{} + return StateConsistent } // serializeChanges builds the change delta for commit payload. -func (s *ServerObject) serializeChanges() map[string]any { - changes := map[string]any{"object_id": s.Get("object_id")} +func (s *ServerObject) serializeChanges() Attributes { + changes := Attributes{"object_id": s.ObjectID()} for key, oldVal := range s.oldValues { newVal := s.attributes[key] @@ -134,7 +174,7 @@ func (s *ServerObject) serializeChanges() map[string]any { } func (s *ServerObject) confirmChanges() { - s.oldValues = map[string]any{} + s.oldValues = Attributes{} if s.deleted { s.attributes["object_id"] = nil s.deleted = false diff --git a/adminapi/server_object_test.go b/adminapi/server_object_test.go index 3a6b03b..4e90397 100644 --- a/adminapi/server_object_test.go +++ b/adminapi/server_object_test.go @@ -9,8 +9,8 @@ import ( func TestSet(t *testing.T) { obj := &ServerObject{ - attributes: map[string]any{"hostname": "old.local", "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "old.local", "object_id": float64(1)}, + oldValues: Attributes{}, } err := obj.Set("hostname", "new.local") @@ -26,47 +26,47 @@ func TestSet(t *testing.T) { func TestSetNonexistent(t *testing.T) { obj := &ServerObject{ - attributes: map[string]any{"hostname": "test", "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "test", "object_id": float64(1)}, + oldValues: Attributes{}, } err := obj.Set("nonexistent", "value") require.Error(t, err) - assert.Contains(t, err.Error(), "does not exist") + assert.ErrorIs(t, err, ErrUnknownAttribute) } func TestCommitState(t *testing.T) { // Consistent: no changes obj := &ServerObject{ - attributes: map[string]any{"hostname": "test", "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "test", "object_id": float64(1)}, + oldValues: Attributes{}, } - assert.Equal(t, "consistent", obj.CommitState()) + assert.Equal(t, StateConsistent, obj.CommitState()) // Changed: attribute modified obj.Set("hostname", "changed") - assert.Equal(t, "changed", obj.CommitState()) + assert.Equal(t, StateChanged, obj.CommitState()) // Deleted obj2 := &ServerObject{ - attributes: map[string]any{"hostname": "test", "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "test", "object_id": float64(1)}, + oldValues: Attributes{}, deleted: true, } - assert.Equal(t, "deleted", obj2.CommitState()) + assert.Equal(t, StateDeleted, obj2.CommitState()) // Created: no object_id obj3 := &ServerObject{ - attributes: map[string]any{"hostname": "test", "object_id": nil}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "test", "object_id": nil}, + oldValues: Attributes{}, } - assert.Equal(t, "created", obj3.CommitState()) + assert.Equal(t, StateCreated, obj3.CommitState()) } func TestSerializeChanges(t *testing.T) { obj := &ServerObject{ - attributes: map[string]any{"hostname": "new.local", "object_id": float64(42)}, - oldValues: map[string]any{"hostname": "old.local"}, + attributes: Attributes{"hostname": "new.local", "object_id": float64(42)}, + oldValues: Attributes{"hostname": "old.local"}, } changes := obj.serializeChanges() @@ -80,11 +80,11 @@ func TestSerializeChanges(t *testing.T) { func TestSerializeChangesMulti(t *testing.T) { obj := &ServerObject{ - attributes: map[string]any{ + attributes: Attributes{ "tags": []any{"web", "new-tag"}, "object_id": float64(42), }, - oldValues: map[string]any{ + oldValues: Attributes{ "tags": []any{"web", "old-tag"}, }, } @@ -98,8 +98,8 @@ func TestSerializeChangesMulti(t *testing.T) { func TestRollback(t *testing.T) { obj := &ServerObject{ - attributes: map[string]any{"hostname": "original", "object_id": float64(1)}, - oldValues: map[string]any{}, + attributes: Attributes{"hostname": "original", "object_id": float64(1)}, + oldValues: Attributes{}, } obj.Set("hostname", "modified") @@ -108,7 +108,231 @@ func TestRollback(t *testing.T) { obj.Rollback() assert.Equal(t, "original", obj.GetString("hostname")) assert.Empty(t, obj.oldValues) - assert.Equal(t, "consistent", obj.CommitState()) + assert.Equal(t, StateConsistent, obj.CommitState()) +} + +func TestRollback_Comprehensive(t *testing.T) { + tests := []struct { + name string + initialAttrs Attributes + initialOldVals Attributes + initialDeleted bool + modifications func(*ServerObject) + expectedAttrs Attributes + expectedDeleted bool + expectedState CommitState + }{ + { + name: "rollback single attribute change", + initialAttrs: Attributes{ + "hostname": "original.local", + "object_id": float64(1), + }, + initialOldVals: Attributes{}, + modifications: func(obj *ServerObject) { + obj.Set("hostname", "modified.local") + }, + expectedAttrs: Attributes{ + "hostname": "original.local", + "object_id": float64(1), + }, + expectedDeleted: false, + expectedState: StateConsistent, + }, + { + name: "rollback multiple attribute changes", + initialAttrs: Attributes{ + "hostname": "original.local", + "environment": "development", + "object_id": float64(2), + }, + initialOldVals: Attributes{}, + modifications: func(obj *ServerObject) { + obj.Set("hostname", "new.local") + obj.Set("environment", "production") + }, + expectedAttrs: Attributes{ + "hostname": "original.local", + "environment": "development", + "object_id": float64(2), + }, + expectedDeleted: false, + expectedState: StateConsistent, + }, + { + name: "rollback multi-attribute (slice) changes", + initialAttrs: Attributes{ + "tags": []any{"web", "original"}, + "object_id": float64(3), + }, + initialOldVals: Attributes{}, + modifications: func(obj *ServerObject) { + obj.Set("tags", []string{"web", "modified", "new"}) + }, + expectedAttrs: Attributes{ + "tags": []any{"web", "original"}, + "object_id": float64(3), + }, + expectedDeleted: false, + expectedState: StateConsistent, + }, + { + name: "rollback deleted object", + initialAttrs: Attributes{ + "hostname": "test.local", + "object_id": float64(4), + }, + initialOldVals: Attributes{}, + modifications: func(obj *ServerObject) { + obj.Delete() + }, + expectedAttrs: Attributes{ + "hostname": "test.local", + "object_id": float64(4), + }, + expectedDeleted: false, + expectedState: StateConsistent, + }, + { + name: "rollback deleted object with attribute changes", + initialAttrs: Attributes{ + "hostname": "original.local", + "object_id": float64(5), + }, + initialOldVals: Attributes{}, + modifications: func(obj *ServerObject) { + obj.Set("hostname", "modified.local") + obj.Delete() + }, + expectedAttrs: Attributes{ + "hostname": "original.local", + "object_id": float64(5), + }, + expectedDeleted: false, + expectedState: StateConsistent, + }, + { + name: "rollback with no changes", + initialAttrs: Attributes{ + "hostname": "unchanged.local", + "object_id": float64(6), + }, + initialOldVals: Attributes{}, + modifications: func(_ *ServerObject) {}, + expectedAttrs: Attributes{ + "hostname": "unchanged.local", + "object_id": float64(6), + }, + expectedDeleted: false, + expectedState: StateConsistent, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := &ServerObject{ + attributes: tt.initialAttrs, + oldValues: tt.initialOldVals, + deleted: tt.initialDeleted, + } + + // Apply modifications + tt.modifications(obj) + + // Rollback + obj.Rollback() + + // Verify attributes are restored + assert.Equal(t, tt.expectedAttrs, obj.attributes, + "attributes should be restored to original values") + + // Verify oldValues is cleared + assert.Empty(t, obj.oldValues, "oldValues should be empty after rollback") + + // Verify deleted flag is reset + assert.Equal(t, tt.expectedDeleted, obj.deleted, + "deleted flag should be reset") + + // Verify commit state + assert.Equal(t, tt.expectedState, obj.CommitState(), + "commit state should be consistent after rollback") + }) + } +} + +func TestGetMulti(t *testing.T) { + tests := []struct { + name string + attrs Attributes + key string + expected MultiAttr + }{ + { + name: "[]any with strings", + attrs: Attributes{"tags": []any{"web", "prod"}}, + key: "tags", + expected: MultiAttr{"web", "prod"}, + }, + { + name: "[]string", + attrs: Attributes{"tags": []string{"web", "prod"}}, + key: "tags", + expected: MultiAttr{"web", "prod"}, + }, + { + name: "MultiAttr directly", + attrs: Attributes{"tags": MultiAttr{"web"}}, + key: "tags", + expected: MultiAttr{"web"}, + }, + { + name: "missing attribute", + attrs: Attributes{"hostname": "test"}, + key: "tags", + expected: MultiAttr{}, + }, + { + name: "nil value", + attrs: Attributes{"tags": nil}, + key: "tags", + expected: MultiAttr{}, + }, + { + name: "non-slice value (string)", + attrs: Attributes{"tags": "not-a-slice"}, + key: "tags", + expected: MultiAttr{}, + }, + { + name: "non-slice value (int)", + attrs: Attributes{"tags": 42}, + key: "tags", + expected: MultiAttr{}, + }, + { + name: "[]any with mixed types keeps only strings", + attrs: Attributes{"tags": []any{"web", 42, true, "prod"}}, + key: "tags", + expected: MultiAttr{"web", "prod"}, + }, + { + name: "empty []any", + attrs: Attributes{"tags": []any{}}, + key: "tags", + expected: MultiAttr{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := &ServerObject{ + attributes: tt.attrs, + oldValues: Attributes{}, + } + result := obj.GetMulti(tt.key) + assert.Equal(t, tt.expected, result) + }) + } } func TestSetMultiAttribute_WithStringSlice(t *testing.T) { @@ -122,7 +346,7 @@ func TestSetMultiAttribute_WithStringSlice(t *testing.T) { obj := &ServerObject{ attributes: attributes, - oldValues: map[string]any{}, + oldValues: Attributes{}, } // User sets the attribute using []string (common usage) @@ -158,7 +382,7 @@ func TestSetMultiAttribute_WithIntSlice(t *testing.T) { obj := &ServerObject{ attributes: attributes, - oldValues: map[string]any{}, + oldValues: Attributes{}, } // User passes []int diff --git a/adminapi/transport.go b/adminapi/transport.go index 6f61204..e4dc689 100644 --- a/adminapi/transport.go +++ b/adminapi/transport.go @@ -2,7 +2,6 @@ package adminapi import ( "bytes" - "compress/gzip" "context" "crypto/hmac" "crypto/rand" @@ -10,7 +9,6 @@ import ( "encoding/base64" "encoding/hex" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -45,7 +43,6 @@ func sendRequest(endpoint string, postData any) (*http.Response, error) { req.Header.Set("Content-Type", "application/x-json") req.Header.Set("X-Timestamp", strconv.FormatInt(now, 10)) req.Header.Set("User-Agent", userAgent) - req.Header.Set("Accept-Encoding", "gzip") if config.sshSigner != nil { // sign with private key or SSH agent @@ -66,16 +63,21 @@ func sendRequest(endpoint string, postData any) (*http.Response, error) { resp, err := http.DefaultClient.Do(req) if err != nil { - return nil, err + return nil, fmt.Errorf("sending request to %s: %w", endpoint, err) } + // special error handling if resp.StatusCode < 200 || resp.StatusCode >= 300 { defer resp.Body.Close() + apiErr := &APIError{ + StatusCode: resp.StatusCode, + Status: http.StatusText(resp.StatusCode), + } + bodyBytes, readErr := io.ReadAll(resp.Body) if readErr != nil { - return nil, fmt.Errorf("HTTP error %d %s (failed to read error details: %w)", - resp.StatusCode, http.StatusText(resp.StatusCode), readErr) + return nil, apiErr } var nestedErrorResp struct { @@ -84,46 +86,15 @@ func sendRequest(endpoint string, postData any) (*http.Response, error) { } `json:"error"` } if jsonErr := json.Unmarshal(bodyBytes, &nestedErrorResp); jsonErr == nil && nestedErrorResp.Error.Message != "" { - return nil, fmt.Errorf("HTTP error %d %s: %s", - resp.StatusCode, http.StatusText(resp.StatusCode), nestedErrorResp.Error.Message) + apiErr.Message = nestedErrorResp.Error.Message } - // If body is empty, just return the status code - return nil, fmt.Errorf("HTTP error %d %s", resp.StatusCode, http.StatusText(resp.StatusCode)) - } - - // If the server responded with gzip encoding, wrap the response body accordingly. - if resp.Header.Get("Content-Encoding") == "gzip" { - gz, err := gzip.NewReader(resp.Body) - if err != nil { - resp.Body.Close() - return nil, fmt.Errorf("failed to create gzip reader: %w", err) - } - - // Replace the resp.Body with our gzip-aware ReadCloser - resp.Body = &gzipReadCloser{ - Reader: gz, - body: resp.Body, - gz: gz, - } + return nil, apiErr } return resp, nil } -// gzipReadCloser wraps a gzip.Reader so that -// closing it also closes the underlying body. -type gzipReadCloser struct { - io.Reader - body io.Closer - gz *gzip.Reader -} - -// Close closes the gzip.Reader and the underlying body. -func (grc *gzipReadCloser) Close() error { - return errors.Join(grc.gz.Close(), grc.body.Close()) -} - // calcSecurityToken calculates HMAC-SHA1 of timestamp:data func calcSecurityToken(authToken []byte, timestamp int64, data []byte) string { mac := hmac.New(sha1.New, authToken) diff --git a/adminapi/transport_test.go b/adminapi/transport_test.go index 90b91f8..2c2150f 100644 --- a/adminapi/transport_test.go +++ b/adminapi/transport_test.go @@ -37,6 +37,10 @@ func TestFakeServer(t *testing.T) { require.NoError(t, err) assert.Len(t, servers, 1) + count, err := query.Count() + require.NoError(t, err) + assert.Equal(t, 1, count) + object := servers[0] assert.Equal(t, "foo.bar.local", object.Get("hostname")) assert.Equal(t, "foo.bar.local", object.GetString("hostname")) diff --git a/examples/helpers.go b/examples/helpers.go deleted file mode 100644 index 8dfba0b..0000000 --- a/examples/helpers.go +++ /dev/null @@ -1,10 +0,0 @@ -package main - -import "log" - -// checkErr is a helper function for examples that exits on error -func checkErr(err error) { - if err != nil { - log.Fatal(err) - } -} diff --git a/examples/query_examples.go b/examples/query_examples.go index d38c6c4..7b733e5 100644 --- a/examples/query_examples.go +++ b/examples/query_examples.go @@ -1,7 +1,7 @@ package main import ( - "fmt" + "log" api "github.com/innogames/serveradmin-go-client/adminapi" ) @@ -14,7 +14,7 @@ func stringQueryExample() { servers, err := q.All() checkErr(err) - fmt.Printf("Found %d servers using string query\n", len(servers)) + log.Printf("Found %d servers using string query\n", len(servers)) } func simpleFilterExample() { @@ -22,14 +22,14 @@ func simpleFilterExample() { q := api.NewQuery(api.Filters{ "environment": "production", "state": "online", - "num_cpu": 8, + "num_cpu": api.LessThanOrEquals(4), }) q.SetAttributes("hostname", "num_cpu", "memory") servers, err := q.All() checkErr(err) - fmt.Printf("Found %d production servers with 8 CPUs\n", len(servers)) + log.Printf("Found %d production servers with 8 CPUs\n", len(servers)) } func regexpFilterExample() { @@ -42,52 +42,19 @@ func regexpFilterExample() { servers, err := q.All() checkErr(err) - fmt.Printf("Found %d web servers matching pattern\n", len(servers)) + log.Printf("Found %d web servers matching pattern\n", len(servers)) } -func anyAllFilterExample() { +func anyAnyFilterExample() { // Use Any filter to match multiple possible values q := api.NewQuery(api.Filters{ - "game_world": api.Any(1, 2, 3), + "game_world": api.GreaterThan(1), "state": api.Any("online", "maintenance"), }) servers, err := q.All() checkErr(err) - - fmt.Printf("Found %d servers in game worlds 1, 2, or 3\n", len(servers)) - - // Use All filter to match all conditions - q2 := api.NewQuery(api.Filters{ - "tags": api.All("backup", "critical"), - }) - - servers2, err := q2.All() - checkErr(err) - - fmt.Printf("Found %d servers with both 'backup' and 'critical' tags\n", len(servers2)) -} - -func notFilterExample() { - // Use Not with Empty to find servers with non-empty values - q := api.NewQuery(api.Filters{ - "backup_disabled": false, - "comment": api.NotEmpty(), - }) - - servers, err := q.All() - checkErr(err) - - fmt.Printf("Found %d servers with backup_disabled and comment set\n", len(servers)) - - // Use Not with specific value - q2 := api.NewQuery(api.Filters{}) - q2.AddFilter("environment", api.Not("development")) - - servers2, err := q2.All() - checkErr(err) - - fmt.Printf("Found %d non-development servers\n", len(servers2)) + log.Printf("Found %d servers:", len(servers)) } func nestedFilterExample() { @@ -107,11 +74,10 @@ func nestedFilterExample() { servers, err := q.All() checkErr(err) - fmt.Printf("Found %d servers with complex nested filters\n", len(servers)) + log.Printf("Found %d servers with complex nested filters\n", len(servers)) } func combinedFilterExample() { - // Real-world example: Find suitable servers for migration q := api.NewQuery(api.Filters{}) q.AddFilter("servertype", "server") @@ -142,9 +108,9 @@ func combinedFilterExample() { servers, err := q.All() checkErr(err) - fmt.Printf("Found %d servers suitable for migration:\n", len(servers)) + log.Printf("Found %d servers suitable for migration:\n", len(servers)) for _, server := range servers { - fmt.Printf(" - %s: %v CPUs, %v GB RAM, project: %s\n", + log.Printf(" - %s: %v CPUs, %v GB RAM, project: %s\n", server.GetString("hostname"), server.Get("num_cpu"), server.Get("memory"), @@ -152,3 +118,30 @@ func combinedFilterExample() { ) } } + +func multiAttrExample() { + // Fetch a server with multi-valued attributes + q, err := api.FromQuery("hostname=webserver01") + checkErr(err) + + server, err := q.One() + checkErr(err) + + // Get tags as MultiAttr + tags := server.GetMulti("tags") + + // Use MultiAttr convenience methods + tags.Add("monitoring", "web") // Add tags (web is duplicate, won't be added) + tags.Delete("old-tag") // Remove old tag + + if tags.Contains("monitoring") { + log.Println("Server has monitoring tag") + } + + // Set back to ServerObject and commit + checkErr(server.Set("tags", []string(tags))) + commitID, err := server.Commit() + checkErr(err) + + log.Printf("Updated tags for %s (commit %d)\n", server.GetString("hostname"), commitID) +} diff --git a/examples/real.go b/examples/real.go index 7d2a895..27c9977 100644 --- a/examples/real.go +++ b/examples/real.go @@ -1,16 +1,23 @@ package main import ( - "fmt" + "log" api "github.com/innogames/serveradmin-go-client/adminapi" ) +// checkErr is a helper function for examples that exits on error +func checkErr(err error) { + if err != nil { + log.Fatal(err) + } +} + func main() { var commitID int // Step 1: Check if object already exists - fmt.Println("=== Checking for existing public_domain object ===") + log.Println("=== Checking for existing public_domain object ===") q, err := api.FromQuery("hostname=test.foo.com servertype=public_domain") checkErr(err) q.AddAttributes("dns_txt") @@ -18,54 +25,47 @@ func main() { publicURL, err := q.One() if err != nil { // Object doesn't exist, create it - fmt.Println("=== Object not found, creating new public_domain object ===") - publicURL, err = api.NewObject("public_domain") - checkErr(err) - - // Set required attributes - publicURL.Set("hostname", "test.foo.com") - publicURL.Set("project", "admin") - - // Commit the new object - commitID, err = publicURL.Commit() - checkErr(err) - fmt.Printf("Created public_url %s (commit ID: %d)\n", publicURL.GetString("hostname"), commitID) - - // Re-query to get the server-assigned object_id - q, err = api.FromQuery("hostname=test.foo.com servertype=public_domain") - checkErr(err) - q.AddAttributes("dns_txt") - publicURL, err = q.One() + log.Println("=== Object not found, creating new public_domain object ===") + publicURL, err = api.NewObject("public_domain", api.Attributes{ + "hostname": "test.foo.com", + "project": "admin", + "dns_txt": api.MultiAttr{}, + }) checkErr(err) - fmt.Printf("Re-queried object_id: %d\n", publicURL.ObjectID()) + log.Printf("Created public_url %s (object_id: %d)\n", publicURL.GetString("hostname"), publicURL.ObjectID()) } else { - fmt.Printf("Found existing object with object_id: %d\n", publicURL.ObjectID()) + log.Printf("Found existing object with object_id: %d\n", publicURL.ObjectID()) } // Step 2: Set dns_txt to "foobar" - fmt.Println("\n=== Setting dns_txt to foobar ===") - publicURL.Set("dns_txt", []string{"foobar"}) + log.Println("\n=== Setting dns_txt to foobar ===") + dnsTxt := publicURL.GetMulti("dns_txt") + dnsTxt.Clear() + dnsTxt.Add("foobar") + publicURL.Set("dns_txt", dnsTxt) // Commit the update commitID, err = publicURL.Commit() checkErr(err) - fmt.Printf("Set dns_txt to %v (commit ID: %d)\n", publicURL.Get("dns_txt"), commitID) + log.Printf("Set dns_txt to %v (commit ID: %d)\n", publicURL.Get("dns_txt"), commitID) // Step 3: Add a random string to dns_txt - fmt.Println("\n=== Adding random string to dns_txt ===") - publicURL.Set("dns_txt", []string{"foobar", "random_value_xyz123"}) + log.Println("\n=== Adding random string to dns_txt ===") + dnsTxt = publicURL.GetMulti("dns_txt") + dnsTxt.Add("random_value_xyz123") + publicURL.Set("dns_txt", dnsTxt) // Commit the second update commitID, err = publicURL.Commit() checkErr(err) - fmt.Printf("Added to dns_txt, now: %v (commit ID: %d)\n", publicURL.Get("dns_txt"), commitID) + log.Printf("Added to dns_txt, now: %v (commit ID: %d)\n", publicURL.Get("dns_txt"), commitID) // Step 4: Delete the object - fmt.Println("\n=== Deleting object ===") + log.Println("\n=== Deleting object ===") publicURL.Delete() commitID, err = publicURL.Commit() checkErr(err) - fmt.Printf("Deleted public_url (commit ID: %d)\n", commitID) + log.Printf("Deleted public_url (commit ID: %d)\n", commitID) - fmt.Println("\n=== Complete ===") + log.Println("\n=== Complete ===") } diff --git a/examples/update_example.go b/examples/update_example.go index 3d1e0c9..c2fd3b0 100644 --- a/examples/update_example.go +++ b/examples/update_example.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "log" api "github.com/innogames/serveradmin-go-client/adminapi" ) @@ -10,7 +9,7 @@ import ( func singleObjectExample() { q, err := api.FromQuery("hostname=webserver01") checkErr(err) - q.AddAttributes("backup_disabled") + q.AddAttributes("backup_disabled", "tags") server, err := q.One() checkErr(err) @@ -18,6 +17,11 @@ func singleObjectExample() { // Modify attributes server.Set("backup_disabled", true) + // Get and update multi-valued attribute as MultiAttr + tags := server.GetMulti("tags") + tags.Add("monitoring", "web") + tags.Delete("old-tag") + // Commit changes commitID, err := server.Commit() checkErr(err) @@ -44,20 +48,16 @@ func multiObjectExample() { } func createObjectExample() { - // Create a new VM object - newVM, err := api.NewObject("vm") - checkErr(err) - - // Set required attributes - newVM.Set("hostname", "newserver.example.com") - newVM.Set("environment", "development") - newVM.Set("num_cpu", 4) - - // Commit creates the object on the server - commitID, err := newVM.Commit() + // Create a new VM object — NewObject fetches defaults, sets attributes, commits, + // and re-queries to populate object_id in a single call. + newVM, err := api.NewObject("vm", api.Attributes{ + "hostname": "newserver.example.com", + "environment": "development", + "num_cpu": 4, + }) checkErr(err) - fmt.Printf("Created new VM %s (commit %d)\n", newVM.GetString("hostname"), commitID) + fmt.Printf("Created new VM %s (object_id: %d)\n", newVM.GetString("hostname"), newVM.ObjectID()) } func deleteObjectExample() { @@ -78,13 +78,13 @@ func deleteObjectExample() { } func batchDeleteExample() { - q, err := api.FromQuery("state=decommissioned") + q, err := api.FromQuery("servertype=domain state=retired") checkErr(err) servers, err := q.All() checkErr(err) - // Delete ALL decommissioned servers using batch Delete() + // Delete ALL retired domains using batch Delete() servers.Delete() // Commit all deletions in a single API call @@ -102,18 +102,10 @@ func rollbackExample() { checkErr(err) // Make some changes - originalHostname := server.GetString("hostname") server.Set("hostname", "modified-name.local") fmt.Printf("Modified hostname: %s\n", server.GetString("hostname")) // Rollback the changes server.Rollback() fmt.Printf("After rollback: %s\n", server.GetString("hostname")) - - // Check commit state - fmt.Printf("Commit state: %s\n", server.CommitState()) // Should be "consistent" - - if server.GetString("hostname") != originalHostname { - log.Fatal("Rollback failed!") - } }