Skip to content

Commit

Permalink
many: add CLI support for getting and setting aspects (canonical#13492)
Browse files Browse the repository at this point in the history
* aspects: namespace Get value at API, not aspects/

Make aspects/ return the retrieved value without the namespace keyed by
the request (per the spec). Since the API supports a GET with many
request paths, it can then build the namespace only once with all the
values it retrieved.

Signed-off-by: Miguel Pires <[email protected]>

* many: add a CLI for aspect get and set

Signed-off-by: Miguel Pires <[email protected]>

---------

Signed-off-by: Miguel Pires <[email protected]>
  • Loading branch information
MiguelPires authored Jan 24, 2024
1 parent e42aa42 commit d3055c9
Show file tree
Hide file tree
Showing 13 changed files with 533 additions and 81 deletions.
5 changes: 2 additions & 3 deletions aspects/aspects.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ func namespaceResult(res interface{}, suffixParts []string) (interface{}, error)

// Get returns the aspect value identified by the request. If either the named
// aspect or the corresponding value can't be found, a NotFoundError is returned.
func (a *Aspect) Get(databag DataBag, request string) (map[string]interface{}, error) {
func (a *Aspect) Get(databag DataBag, request string) (interface{}, error) {
if err := validateAspectDottedPath(request, nil); err != nil {
return nil, badRequestErrorFrom(a, "get", request, err.Error())
}
Expand Down Expand Up @@ -461,8 +461,7 @@ func (a *Aspect) Get(databag DataBag, request string) (map[string]interface{}, e
return nil, notFoundErrorFrom(a, "get", request, "matching rules don't map to any values")
}

// the top level maps the request to the remaining namespace
return map[string]interface{}{request: merged}, nil
return merged, nil
}

func mergeNamespaces(old, new interface{}) (interface{}, error) {
Expand Down
93 changes: 39 additions & 54 deletions aspects/aspects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,31 +152,31 @@ func (*aspectSuite) TestGetAndSetAspects(c *C) {

ssid, err := wsAspect.Get(databag, "ssid")
c.Assert(err, IsNil)
c.Check(ssid, DeepEquals, map[string]interface{}{"ssid": "my-ssid"})
c.Check(ssid, DeepEquals, "my-ssid")

// nested list value
err = wsAspect.Set(databag, "ssids", []string{"one", "two"})
c.Assert(err, IsNil)

ssids, err := wsAspect.Get(databag, "ssids")
c.Assert(err, IsNil)
c.Check(ssids, DeepEquals, map[string]interface{}{"ssids": []interface{}{"one", "two"}})
c.Check(ssids, DeepEquals, []interface{}{"one", "two"})

// top-level string
err = wsAspect.Set(databag, "top-level", "randomValue")
c.Assert(err, IsNil)

topLevel, err := wsAspect.Get(databag, "top-level")
c.Assert(err, IsNil)
c.Check(topLevel, DeepEquals, map[string]interface{}{"top-level": "randomValue"})
c.Check(topLevel, DeepEquals, "randomValue")

// dotted request paths are permitted
err = wsAspect.Set(databag, "dotted.path", 3)
c.Assert(err, IsNil)

num, err := wsAspect.Get(databag, "dotted.path")
c.Assert(err, IsNil)
c.Check(num, DeepEquals, map[string]interface{}{"dotted.path": float64(3)})
c.Check(num, DeepEquals, float64(3))
}

func (s *aspectSuite) TestAspectNotFound(c *C) {
Expand Down Expand Up @@ -369,7 +369,7 @@ func (s *aspectSuite) TestAspectAssertionWithPlaceholder(c *C) {

value, err := aspect.Get(databag, t.request)
c.Assert(err, IsNil, cmt)
c.Assert(value, DeepEquals, map[string]interface{}{t.request: "expectedValue"}, cmt)
c.Assert(value, DeepEquals, "expectedValue", cmt)

getPath, setPath := databag.getLastPaths()
c.Assert(getPath, Equals, t.storage, cmt)
Expand Down Expand Up @@ -476,7 +476,7 @@ func (s *aspectSuite) TestAspectUnsetTopLevelEntry(c *C) {

value, err := aspect.Get(databag, "bar")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"bar": "bval"})
c.Assert(value, DeepEquals, "bval")
}

func (s *aspectSuite) TestAspectUnsetLeafWithSiblings(c *C) {
Expand Down Expand Up @@ -505,7 +505,7 @@ func (s *aspectSuite) TestAspectUnsetLeafWithSiblings(c *C) {
// doesn't affect the other leaf entry under "foo"
value, err := aspect.Get(databag, "baz")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"baz": "bazVal"})
c.Assert(value, DeepEquals, "bazVal")
}

func (s *aspectSuite) TestAspectUnsetWithNestedEntry(c *C) {
Expand Down Expand Up @@ -624,16 +624,16 @@ func (s *aspectSuite) TestAspectGetResultNamespaceMatchesRequest(c *C) {

value, err := aspect.Get(databag, "one.two")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"one.two": "value"})
c.Assert(value, DeepEquals, "value")

value, err = aspect.Get(databag, "onetwo")
c.Assert(err, IsNil)
// the key matches the request, not the storage storage
c.Assert(value, DeepEquals, map[string]interface{}{"onetwo": "value"})
c.Assert(value, DeepEquals, "value")

value, err = aspect.Get(databag, "one")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"one": map[string]interface{}{"two": "value"}})
c.Assert(value, DeepEquals, map[string]interface{}{"two": "value"})
}

func (s *aspectSuite) TestAspectGetMatchesOnPrefix(c *C) {
Expand All @@ -655,11 +655,11 @@ func (s *aspectSuite) TestAspectGetMatchesOnPrefix(c *C) {

value, err := aspect.Get(databag, "snapd.status")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"snapd.status": "active"})
c.Assert(value, DeepEquals, "active")

value, err = aspect.Get(databag, "snapd")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"snapd": map[string]interface{}{"status": "active"}})
c.Assert(value, DeepEquals, map[string]interface{}{"status": "active"})
}

func (s *aspectSuite) TestAspectGetNoMatchRequestLongerThanPattern(c *C) {
Expand Down Expand Up @@ -700,12 +700,11 @@ func (s *aspectSuite) TestAspectManyPrefixMatches(c *C) {

value, err := aspect.Get(databag, "status")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{
"status": map[string]interface{}{
c.Assert(value, DeepEquals,
map[string]interface{}{
"snapd": "disabled",
"firefox": "active",
},
})
})
}

func (s *aspectSuite) TestAspectCombineNamespacesInPrefixMatches(c *C) {
Expand All @@ -732,16 +731,15 @@ func (s *aspectSuite) TestAspectCombineNamespacesInPrefixMatches(c *C) {

value, err := aspect.Get(databag, "status")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{
"status": map[string]interface{}{
c.Assert(value, DeepEquals,
map[string]interface{}{
"foo": map[string]interface{}{
"bar": map[string]interface{}{
"firefox": "active",
},
"snapd": "disabled",
},
},
})
})
}

func (s *aspectSuite) TestGetScalarOverwritesLeafOfMapValue(c *C) {
Expand Down Expand Up @@ -770,7 +768,7 @@ func (s *aspectSuite) TestGetScalarOverwritesLeafOfMapValue(c *C) {

value, err := aspect.Get(databag, "motors")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"motors": map[string]interface{}{"a": map[string]interface{}{"speed": 101.5}}})
c.Assert(value, DeepEquals, map[string]interface{}{"a": map[string]interface{}{"speed": 101.5}})
}

func (s *aspectSuite) TestGetSingleScalarOk(c *C) {
Expand All @@ -789,7 +787,7 @@ func (s *aspectSuite) TestGetSingleScalarOk(c *C) {

value, err := aspect.Get(databag, "foo")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"foo": "bar"})
c.Assert(value, DeepEquals, "bar")
}

func (s *aspectSuite) TestGetMatchScalarAndMapError(c *C) {
Expand Down Expand Up @@ -831,23 +829,23 @@ func (s *aspectSuite) TestGetRulesAreSortedByParentage(c *C) {
value, err := aspect.Get(databag, "foo")
c.Assert(err, IsNil)
// returned the value read by entry "foo"
c.Assert(value, DeepEquals, map[string]interface{}{"foo": map[string]interface{}{"bar": map[string]interface{}{"baz": "first"}}})
c.Assert(value, DeepEquals, map[string]interface{}{"bar": map[string]interface{}{"baz": "first"}})

err = databag.Set("second", map[string]interface{}{"baz": "second"})
c.Assert(err, IsNil)

value, err = aspect.Get(databag, "foo")
c.Assert(err, IsNil)
// the leaf is replaced by a value read from a rule that is nested
c.Assert(value, DeepEquals, map[string]interface{}{"foo": map[string]interface{}{"bar": map[string]interface{}{"baz": "second"}}})
c.Assert(value, DeepEquals, map[string]interface{}{"bar": map[string]interface{}{"baz": "second"}})

err = databag.Set("third", "third")
c.Assert(err, IsNil)

value, err = aspect.Get(databag, "foo")
c.Assert(err, IsNil)
// lastly, it reads the value from "foo.bar.baz" the most nested entry
c.Assert(value, DeepEquals, map[string]interface{}{"foo": map[string]interface{}{"bar": map[string]interface{}{"baz": "third"}}})
c.Assert(value, DeepEquals, map[string]interface{}{"bar": map[string]interface{}{"baz": "third"}})
}

func (s *aspectSuite) TestGetUnmatchedPlaceholderReturnsAll(c *C) {
Expand All @@ -871,7 +869,7 @@ func (s *aspectSuite) TestGetUnmatchedPlaceholderReturnsAll(c *C) {

value, err := aspect.Get(databag, "snaps")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"snaps": map[string]interface{}{"snapd": float64(1), "foo": map[string]interface{}{"bar": float64(2)}}})
c.Assert(value, DeepEquals, map[string]interface{}{"snapd": float64(1), "foo": map[string]interface{}{"bar": float64(2)}})
}

func (s *aspectSuite) TestGetUnmatchedPlaceholdersWithNestedValues(c *C) {
Expand All @@ -897,7 +895,7 @@ func (s *aspectSuite) TestGetUnmatchedPlaceholdersWithNestedValues(c *C) {

value, err := asp.Get(databag, "snaps")
c.Assert(err, IsNil)
c.Assert(value, DeepEquals, map[string]interface{}{"snaps": map[string]interface{}{"snapd": map[string]interface{}{"status": "active"}}})
c.Assert(value, DeepEquals, map[string]interface{}{"snapd": map[string]interface{}{"status": "active"}})
}

func (s *aspectSuite) TestGetSeveralUnmatchedPlaceholders(c *C) {
Expand Down Expand Up @@ -937,12 +935,10 @@ func (s *aspectSuite) TestGetSeveralUnmatchedPlaceholders(c *C) {
value, err := asp.Get(databag, "a")
c.Assert(err, IsNil)
expected := map[string]interface{}{
"a": map[string]interface{}{
"b1": map[string]interface{}{
"c": map[string]interface{}{
"d1": map[string]interface{}{
"e": "end",
},
"b1": map[string]interface{}{
"c": map[string]interface{}{
"d1": map[string]interface{}{
"e": "end",
},
},
},
Expand Down Expand Up @@ -978,12 +974,10 @@ func (s *aspectSuite) TestGetMergeAtDifferentLevels(c *C) {
value, err := asp.Get(databag, "a")
c.Assert(err, IsNil)
expected := map[string]interface{}{
"a": map[string]interface{}{
"b": map[string]interface{}{
"c": map[string]interface{}{
"d": map[string]interface{}{
"e": "end",
},
"b": map[string]interface{}{
"c": map[string]interface{}{
"d": map[string]interface{}{
"e": "end",
},
},
},
Expand Down Expand Up @@ -1137,19 +1131,14 @@ func (s *aspectSuite) TestReadWriteRead(c *C) {

data, err := asp.Get(databag, "a")
c.Assert(err, IsNil)
c.Assert(data, DeepEquals, map[string]interface{}{
"a": initData,
})
c.Assert(data, DeepEquals, initData)

// we return the data in a map keyed the request so unwrap before setting
err = asp.Set(databag, "a", data["a"])
err = asp.Set(databag, "a", data)
c.Assert(err, IsNil)

data, err = asp.Get(databag, "a")
c.Assert(err, IsNil)
c.Assert(data, DeepEquals, map[string]interface{}{
"a": initData,
})
c.Assert(data, DeepEquals, initData)
}

func (s *aspectSuite) TestReadWriteSameDataAtDifferentLevels(c *C) {
Expand All @@ -1172,11 +1161,9 @@ func (s *aspectSuite) TestReadWriteSameDataAtDifferentLevels(c *C) {
c.Assert(err, IsNil)

for _, req := range []string{"a", "a.b", "a.b.c"} {
namespacedVal, err := asp.Get(databag, req)
val, err := asp.Get(databag, req)
c.Assert(err, IsNil)

// Get returns the data in a collapsed top-level namespace so we must remove it before setting back
val := namespacedVal[req]
err = asp.Set(databag, req, val)
c.Assert(err, IsNil)
}
Expand Down Expand Up @@ -1249,10 +1236,8 @@ func (s *aspectSuite) TestGetReadsStorageLessNestedNamespaceBefore(c *C) {
data, err := asp.Get(databag, "snaps")
c.Assert(err, IsNil)
c.Assert(data, DeepEquals, map[string]interface{}{
"snaps": map[string]interface{}{
"snapd": map[string]interface{}{
"version": float64(2),
},
"snapd": map[string]interface{}{
"version": float64(2),
},
})
}
Expand Down
54 changes: 54 additions & 0 deletions client/aspects.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2024 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

package client

import (
"bytes"
"encoding/json"
"fmt"
"net/url"
"strings"
)

func (c *Client) AspectGet(aspectID string, requests []string) (result map[string]interface{}, err error) {
query := url.Values{}
query.Add("fields", strings.Join(requests, ","))

endpoint := fmt.Sprintf("/v2/aspects/%s", aspectID)
_, err = c.doSync("GET", endpoint, query, nil, nil, &result)
if err != nil {
return nil, err
}

return result, nil
}

func (c *Client) AspectSet(aspectID string, requestValues map[string]interface{}) (changeID string, err error) {
body, err := json.Marshal(requestValues)
if err != nil {
return "", err
}

headers := make(map[string]string)
headers["Content-Type"] = "application/json"

endpoint := fmt.Sprintf("/v2/aspects/%s", aspectID)
return c.doAsync("PUT", endpoint, nil, headers, bytes.NewReader(body))
}
Loading

0 comments on commit d3055c9

Please sign in to comment.