Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit c64fbf6

Browse files
feat(api): return variable configurations in /apps/:id/bricks and apps/:id/bricks/:id (#18)
* add variables_details * add unit test * update openapi * add test e2e * fix bug * add deprecated field description for openapi * refactoring test constants * restore license header * rename variables_details to config_variables * rename variable name * udate docs and tests e2e * refactoring variables
1 parent cd53bab commit c64fbf6

File tree

8 files changed

+205
-43
lines changed

8 files changed

+205
-43
lines changed

‎internal/api/docs/openapi.yaml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,6 +1276,17 @@ components:
12761276
name:
12771277
type: string
12781278
type: object
1279+
BrickConfigVariable:
1280+
properties:
1281+
description:
1282+
type: string
1283+
name:
1284+
type: string
1285+
required:
1286+
type: boolean
1287+
value:
1288+
type: string
1289+
type: object
12791290
BrickCreateUpdateRequest:
12801291
properties:
12811292
model:
@@ -1325,6 +1336,10 @@ components:
13251336
type: string
13261337
category:
13271338
type: string
1339+
config_variables:
1340+
items:
1341+
$ref: '#/components/schemas/BrickConfigVariable'
1342+
type: array
13281343
id:
13291344
type: string
13301345
model:
@@ -1336,6 +1351,8 @@ components:
13361351
variables:
13371352
additionalProperties:
13381353
type: string
1354+
description: 'Deprecated: use config_variables instead. This field is kept
1355+
for backward compatibility.'
13391356
type: object
13401357
type: object
13411358
BrickListItem:

‎internal/e2e/client/client.gen.go‎

Lines changed: 17 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/e2e/daemon/app_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ func TestDeleteApp(t *testing.T) {
470470

471471
t.Run("DeletingExampleApp_Fail", func(t *testing.T) {
472472
var actualResponseBody models.ErrorResponse
473-
deleteResp, err := httpClient.DeleteApp(t.Context(), noExisitingExample)
473+
deleteResp, err := httpClient.DeleteApp(t.Context(), "ZXhhbXBsZXM6anVzdGJsaW5f")
474474
require.NoError(t, err)
475475
defer deleteResp.Body.Close()
476476

@@ -818,7 +818,7 @@ func TestAppPorts(t *testing.T) {
818818
respBrick, err := httpClient.UpsertAppBrickInstanceWithResponse(
819819
t.Context(),
820820
*createResp.JSON201.Id,
821-
StreamLitUi,
821+
"arduino:streamlit_ui",
822822
client.BrickCreateUpdateRequest{},
823823
func(ctx context.Context, req *http.Request) error { return nil },
824824
)

‎internal/e2e/daemon/const.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,7 @@
1616
package daemon
1717

1818
const (
19-
ImageClassifactionBrickID = "arduino:image_classification"
20-
StreamLitUi = "arduino:streamlit_ui"
21-
expectedDetailsAppNotfound = "unable to find the app"
22-
expectedDetailsAppInvalidAppId = "invalid app id"
23-
noExistingApp = "dXNlcjp0ZXN0LWFwcAw"
24-
malformedAppId = "this-is-definitely-not-base64"
25-
noExisitingExample = "ZXhhbXBsZXM6anVzdGJsaW5f"
19+
ImageClassifactionBrickID = "arduino:image_classification"
20+
noExistingApp = "dXNlcjp0ZXN0LWFwcAw"
21+
malformedAppId = "this-is-definitely-not-base64"
2622
)

‎internal/e2e/daemon/instance_bricks_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,28 @@ import (
3131
"github.com/arduino/arduino-app-cli/internal/e2e/client"
3232
)
3333

34+
const (
35+
expectedDetailsAppInvalidAppId = "invalid app id"
36+
expectedDetailsAppNotfound = "unable to find the app"
37+
)
38+
39+
var (
40+
expectedConfigVariables = []client.BrickConfigVariable{
41+
{
42+
Description: f.Ptr("path to the custom model directory"),
43+
Name: f.Ptr("CUSTOM_MODEL_PATH"),
44+
Required: f.Ptr(false),
45+
Value: f.Ptr("/home/arduino/.arduino-bricks/ei-models"),
46+
},
47+
{
48+
Description: f.Ptr("path to the model file"),
49+
Name: f.Ptr("EI_CLASSIFICATION_MODEL"),
50+
Required: f.Ptr(false),
51+
Value: f.Ptr("/models/ootb/ei/mobilenet-v2-224px.eim"),
52+
},
53+
}
54+
)
55+
3456
func setupTestApp(t *testing.T) (*client.CreateAppResp, *client.ClientWithResponses) {
3557
httpClient := GetHttpclient(t)
3658
createResp, err := httpClient.CreateAppWithResponse(
@@ -68,6 +90,7 @@ func TestGetAppBrickInstances(t *testing.T) {
6890
require.NoError(t, err)
6991
require.Len(t, *brickInstances.JSON200.Bricks, 1)
7092
require.Equal(t, ImageClassifactionBrickID, *(*brickInstances.JSON200.Bricks)[0].Id)
93+
require.Equal(t, expectedConfigVariables, *(*brickInstances.JSON200.Bricks)[0].ConfigVariables)
7194

7295
})
7396

@@ -111,6 +134,7 @@ func TestGetAppBrickInstanceById(t *testing.T) {
111134
require.NoError(t, err)
112135
require.NotEmpty(t, brickInstance.JSON200)
113136
require.Equal(t, ImageClassifactionBrickID, *brickInstance.JSON200.Id)
137+
require.Equal(t, expectedConfigVariables, (*brickInstance.JSON200.ConfigVariables))
114138
})
115139

116140
t.Run("GetAppBrickInstanceByBrickID_InvalidAppID_Fails", func(t *testing.T) {

‎internal/orchestrator/bricks/bricks.go‎

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"errors"
2020
"fmt"
2121
"log/slog"
22-
"maps"
2322
"slices"
2423

2524
"github.com/arduino/go-paths-helper"
@@ -80,15 +79,20 @@ func (s *Service) AppBrickInstancesList(a *app.ArduinoApp) (AppBrickInstancesRes
8079
if !found {
8180
return AppBrickInstancesResult{}, fmt.Errorf("brick not found with id %s", brickInstance.ID)
8281
}
82+
83+
variablesMap, configVariables := getBrickConfigDetails(brick, brickInstance.Variables)
84+
8385
res.BrickInstances[i] = BrickInstance{
84-
ID: brick.ID,
85-
Name: brick.Name,
86-
Author: "Arduino", // TODO: for now we only support our bricks
87-
Category: brick.Category,
88-
Status: "installed",
89-
ModelID: brickInstance.Model, // TODO: in case is not set by the user, should we return the default model?
90-
Variables: brickInstance.Variables, // TODO: do we want to show also the default value of not explicitly set variables?
86+
ID: brick.ID,
87+
Name: brick.Name,
88+
Author: "Arduino", // TODO: for now we only support our bricks
89+
Category: brick.Category,
90+
Status: "installed",
91+
ModelID: brickInstance.Model, // TODO: in case is not set by the user, should we return the default model?
92+
Variables: variablesMap, // TODO: do we want to show also the default value of not explicitly set variables?
93+
ConfigVariables: configVariables,
9194
}
95+
9296
}
9397
return res, nil
9498
}
@@ -104,29 +108,51 @@ func (s *Service) AppBrickInstanceDetails(a *app.ArduinoApp, brickID string) (Br
104108
return BrickInstance{}, fmt.Errorf("brick %s not added in the app", brickID)
105109
}
106110

107-
variables := make(map[string]string, len(brick.Variables))
108-
for _, v := range brick.Variables {
109-
variables[v.Name] = v.DefaultValue
110-
}
111-
// Add/Update the variables with the ones from the app descriptor
112-
maps.Copy(variables, a.Descriptor.Bricks[brickIndex].Variables)
111+
variables, configVariables := getBrickConfigDetails(brick, a.Descriptor.Bricks[brickIndex].Variables)
113112

114113
modelID := a.Descriptor.Bricks[brickIndex].Model
115114
if modelID == "" {
116115
modelID = brick.ModelName
117116
}
118117

119118
return BrickInstance{
120-
ID: brickID,
121-
Name: brick.Name,
122-
Author: "Arduino", // TODO: for now we only support our bricks
123-
Category: brick.Category,
124-
Status: "installed", // For now every Arduino brick are installed
125-
Variables: variables,
126-
ModelID: modelID,
119+
ID: brickID,
120+
Name: brick.Name,
121+
Author: "Arduino", // TODO: for now we only support our bricks
122+
Category: brick.Category,
123+
Status: "installed", // For now every Arduino brick are installed
124+
Variables: variables,
125+
ConfigVariables: configVariables,
126+
ModelID: modelID,
127127
}, nil
128128
}
129129

130+
func getBrickConfigDetails(
131+
brick *bricksindex.Brick, userVariables map[string]string,
132+
) (map[string]string, []BrickConfigVariable) {
133+
variablesMap := make(map[string]string, len(brick.Variables))
134+
variableDetails := make([]BrickConfigVariable, 0, len(brick.Variables))
135+
136+
for _, v := range brick.Variables {
137+
finalValue := v.DefaultValue
138+
139+
userValue, ok := userVariables[v.Name]
140+
if ok {
141+
finalValue = userValue
142+
}
143+
variablesMap[v.Name] = finalValue
144+
145+
variableDetails = append(variableDetails, BrickConfigVariable{
146+
Name: v.Name,
147+
Value: finalValue,
148+
Description: v.Description,
149+
Required: v.IsRequired(),
150+
})
151+
}
152+
153+
return variablesMap, variableDetails
154+
}
155+
130156
func (s *Service) BricksDetails(id string, idProvider *app.IDProvider,
131157
cfg config.Configuration) (BrickDetailsResult, error) {
132158
brick, found := s.bricksIndex.FindBrickByID(id)

‎internal/orchestrator/bricks/bricks_test.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,83 @@ func TestBrickCreate(t *testing.T) {
110110
require.Equal(t, secret, after.Descriptor.Bricks[0].Variables["ARDUINO_SECRET"])
111111
})
112112
}
113+
114+
func TestGetBrickInstanceVariableDetails(t *testing.T) {
115+
tests := []struct {
116+
name string
117+
brick *bricksindex.Brick
118+
userVariables map[string]string
119+
expectedConfigVariables []BrickConfigVariable
120+
expectedVariableMap map[string]string
121+
}{
122+
{
123+
name: "variable is present in the map",
124+
brick: &bricksindex.Brick{
125+
Variables: []bricksindex.BrickVariable{
126+
{Name: "VAR1", Description: "desc"},
127+
},
128+
},
129+
userVariables: map[string]string{"VAR1": "value1"},
130+
expectedConfigVariables: []BrickConfigVariable{
131+
{Name: "VAR1", Value: "value1", Description: "desc", Required: true},
132+
},
133+
expectedVariableMap: map[string]string{"VAR1": "value1"},
134+
},
135+
{
136+
name: "variable not present in the map",
137+
brick: &bricksindex.Brick{
138+
Variables: []bricksindex.BrickVariable{
139+
{Name: "VAR1", Description: "desc"},
140+
},
141+
},
142+
userVariables: map[string]string{},
143+
expectedConfigVariables: []BrickConfigVariable{
144+
{Name: "VAR1", Value: "", Description: "desc", Required: true},
145+
},
146+
expectedVariableMap: map[string]string{"VAR1": ""},
147+
},
148+
{
149+
name: "variable with default value",
150+
brick: &bricksindex.Brick{
151+
Variables: []bricksindex.BrickVariable{
152+
{Name: "VAR1", DefaultValue: "default", Description: "desc"},
153+
},
154+
},
155+
userVariables: map[string]string{},
156+
expectedConfigVariables: []BrickConfigVariable{
157+
{Name: "VAR1", Value: "default", Description: "desc", Required: false},
158+
},
159+
expectedVariableMap: map[string]string{"VAR1": "default"},
160+
},
161+
{
162+
name: "multiple variables",
163+
brick: &bricksindex.Brick{
164+
Variables: []bricksindex.BrickVariable{
165+
{Name: "VAR1", Description: "desc1"},
166+
{Name: "VAR2", DefaultValue: "def2", Description: "desc2"},
167+
},
168+
},
169+
userVariables: map[string]string{"VAR1": "v1"},
170+
expectedConfigVariables: []BrickConfigVariable{
171+
{Name: "VAR1", Value: "v1", Description: "desc1", Required: true},
172+
{Name: "VAR2", Value: "def2", Description: "desc2", Required: false},
173+
},
174+
expectedVariableMap: map[string]string{"VAR1": "v1", "VAR2": "def2"},
175+
},
176+
{
177+
name: "no variables",
178+
brick: &bricksindex.Brick{Variables: []bricksindex.BrickVariable{}},
179+
userVariables: map[string]string{},
180+
expectedConfigVariables: []BrickConfigVariable{},
181+
expectedVariableMap: map[string]string{},
182+
},
183+
}
184+
185+
for _, tt := range tests {
186+
t.Run(tt.name, func(t *testing.T) {
187+
actualVariableMap, actualConfigVariables := getBrickConfigDetails(tt.brick, tt.userVariables)
188+
require.Equal(t, tt.expectedVariableMap, actualVariableMap)
189+
require.Equal(t, tt.expectedConfigVariables, actualConfigVariables)
190+
})
191+
}
192+
}

‎internal/orchestrator/bricks/types.go‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,21 @@ type AppBrickInstancesResult struct {
3434
}
3535

3636
type BrickInstance struct {
37-
ID string `json:"id"`
38-
Name string `json:"name"`
39-
Author string `json:"author"`
40-
Category string `json:"category"`
41-
Status string `json:"status"`
42-
Variables map[string]string `json:"variables,omitempty"`
43-
ModelID string `json:"model,omitempty"`
37+
ID string `json:"id"`
38+
Name string `json:"name"`
39+
Author string `json:"author"`
40+
Category string `json:"category"`
41+
Status string `json:"status"`
42+
Variables map[string]string `json:"variables,omitempty" description:"Deprecated: use config_variables instead. This field is kept for backward compatibility."`
43+
ConfigVariables []BrickConfigVariable `json:"config_variables,omitempty"`
44+
ModelID string `json:"model,omitempty"`
45+
}
46+
47+
type BrickConfigVariable struct {
48+
Name string `json:"name"`
49+
Value string `json:"value"`
50+
Description string `json:"description"`
51+
Required bool `json:"required"`
4452
}
4553

4654
type BrickVariable struct {

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /