[API] Change the GET /v1/bricks/{id} variables field (#126)

* add config variables for brick details

* update unit tests

* update openapi and e2e test

* code review fix

* make lint happy

Author: mirkoCrobu

internal/api/docs/openapi.yaml

@@ -1320,6 +1320,11 @@ components:
             $ref: '#/components/schemas/AIModel'
           nullable: true
           type: array
+        config_variables:
+          items:
+            $ref: '#/components/schemas/BrickConfigVariable'
+          nullable: true
+          type: array
         description:
           type: string
         id:
@@ -1340,6 +1345,9 @@ components:
         variables:
           additionalProperties:
             $ref: '#/components/schemas/BrickVariable'
+          description: 'Deprecated: use config_variables instead. This field is kept
+            for backward compatibility.'
+          nullable: true
           type: object
       type: object
     BrickInstance:

internal/e2e/client/client.gen.go

@@ -127,6 +127,21 @@ func TestBricksDetails(t *testing.T) {
 				Name:        f.Ptr("Person classification"),
 				Description: f.Ptr("Person classification model based on WakeVision dataset. This model is trained to classify images into two categories: person and not-person."),
 			}}
+		expectConfigVariables := []client.BrickConfigVariable{
+			{
+				Name:        f.Ptr("CUSTOM_MODEL_PATH"),
+				Value:       f.Ptr("/home/arduino/.arduino-bricks/ei-models"),
+				Description: f.Ptr("path to the custom model directory"),
+				Required:    f.Ptr(false),
+			},
+			{
+				Name:        f.Ptr("EI_CLASSIFICATION_MODEL"),
+				Value:       f.Ptr("/models/ootb/ei/mobilenet-v2-224px.eim"),
+				Description: f.Ptr("path to the model file"),
+				Required:    f.Ptr(false),
+			},
+		}
+
 		response, err := httpClient.GetBrickDetailsWithResponse(t.Context(), validBrickID, func(ctx context.Context, req *http.Request) error { return nil })
 		require.NoError(t, err)
 		require.Equal(t, http.StatusOK, response.StatusCode(), "status code should be 200 ok")
@@ -147,5 +162,7 @@ func TestBricksDetails(t *testing.T) {
 		require.Equal(t, expectedUsedByApps, *(response.JSON200.UsedByApps))
 		require.NotNil(t, response.JSON200.CompatibleModels, "Models should not be nil")
 		require.Equal(t, expectedModelLiteInfo, *(response.JSON200.CompatibleModels))
+		require.NotNil(t, response.JSON200.ConfigVariables, "ConfigVariables should not be nil")
+		require.Equal(t, expectConfigVariables, *(response.JSON200.ConfigVariables))
 	})
 }

internal/e2e/daemon/brick_test.go

@@ -78,7 +78,7 @@ func (s *Service) AppBrickInstancesList(a *app.ArduinoApp) (AppBrickInstancesRes
 			return AppBrickInstancesResult{}, fmt.Errorf("brick not found with id %s", brickInstance.ID)
 		}
 
-		variablesMap, configVariables := getBrickConfigDetails(brick, brickInstance.Variables)
+		variablesMap, configVariables := getInstanceBrickConfigVariableDetails(brick, brickInstance.Variables)
 
 		res.BrickInstances[i] = BrickInstanceListItem{
 			ID:              brick.ID,
@@ -107,7 +107,7 @@ func (s *Service) AppBrickInstanceDetails(a *app.ArduinoApp, brickID string) (Br
 		return BrickInstance{}, fmt.Errorf("brick %s not added in the app", brickID)
 	}
 
-	variables, configVariables := getBrickConfigDetails(brick, a.Descriptor.Bricks[brickIndex].Variables)
+	variables, configVariables := getInstanceBrickConfigVariableDetails(brick, a.Descriptor.Bricks[brickIndex].Variables)
 
 	modelID := a.Descriptor.Bricks[brickIndex].Model
 	if modelID == "" {
@@ -134,7 +134,7 @@ func (s *Service) AppBrickInstanceDetails(a *app.ArduinoApp, brickID string) (Br
 	}, nil
 }
 
-func getBrickConfigDetails(
+func getInstanceBrickConfigVariableDetails(
 	brick *bricksindex.Brick, userVariables map[string]string,
 ) (map[string]string, []BrickConfigVariable) {
 	variablesMap := make(map[string]string, len(brick.Variables))
@@ -167,15 +167,6 @@ func (s *Service) BricksDetails(id string, idProvider *app.IDProvider,
 		return BrickDetailsResult{}, ErrBrickNotFound
 	}
 
-	variables := make(map[string]BrickVariable, len(brick.Variables))
-	for _, v := range brick.Variables {
-		variables[v.Name] = BrickVariable{
-			DefaultValue: v.DefaultValue,
-			Description:  v.Description,
-			Required:     v.IsRequired(),
-		}
-	}
-
 	readme, err := s.staticStore.GetBrickReadmeFromID(brick.ID)
 	if err != nil {
 		return BrickDetailsResult{}, fmt.Errorf("cannot open docs for brick %s: %w", id, err)
@@ -200,6 +191,9 @@ func (s *Service) BricksDetails(id string, idProvider *app.IDProvider,
 	if err != nil {
 		return BrickDetailsResult{}, fmt.Errorf("unable to get used by apps: %w", err)
 	}
+
+	variables, configVariables := getBrickConfigVariableDetails(brick)
+
 	return BrickDetailsResult{
 		ID:           id,
 		Name:         brick.Name,
@@ -220,9 +214,33 @@ func (s *Service) BricksDetails(id string, idProvider *app.IDProvider,
 				Description: m.ModuleDescription,
 			}
 		}),
+		ConfigVariables: configVariables,
 	}, nil
 }
 
+func getBrickConfigVariableDetails(
+	brick *bricksindex.Brick) (map[string]BrickVariable, []BrickConfigVariable) {
+	variablesMap := make(map[string]BrickVariable, len(brick.Variables))
+	variableDetails := make([]BrickConfigVariable, 0, len(brick.Variables))
+
+	for _, v := range brick.Variables {
+		variablesMap[v.Name] = BrickVariable{
+			DefaultValue: v.DefaultValue,
+			Description:  v.Description,
+			Required:     v.IsRequired(),
+		}
+
+		variableDetails = append(variableDetails, BrickConfigVariable{
+			Name:        v.Name,
+			Value:       v.DefaultValue,
+			Description: v.Description,
+			Required:    v.IsRequired(),
+		})
+	}
+
+	return variablesMap, variableDetails
+}
+
 func getUsedByApps(
 	cfg config.Configuration, brickId string, idProvider *app.IDProvider) ([]AppReference, error) {
 	var (

internal/orchestrator/bricks/bricks.go

@@ -317,7 +317,7 @@ func TestGetBrickInstanceVariableDetails(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			actualVariableMap, actualConfigVariables := getBrickConfigDetails(tt.brick, tt.userVariables)
+			actualVariableMap, actualConfigVariables := getInstanceBrickConfigVariableDetails(tt.brick, tt.userVariables)
 			require.Equal(t, tt.expectedVariableMap, actualVariableMap)
 			require.Equal(t, tt.expectedConfigVariables, actualConfigVariables)
 		})
@@ -402,6 +402,21 @@ func TestBricksDetails(t *testing.T) {
 	})
 
 	t.Run("Success - Full Details - multiple models", func(t *testing.T) {
+		expectConfigVariables := []BrickConfigVariable{
+			{
+				Name:        "EI_OBJ_DETECTION_MODEL",
+				Value:       "default_path",
+				Description: "path to the model file",
+				Required:    false,
+			},
+			{
+				Name:        "CUSTOM_MODEL_PATH",
+				Value:       "/home/arduino/.arduino-bricks/ei-models",
+				Description: "path to the custom model directory",
+				Required:    false,
+			},
+		}
+
 		res, err := svc.BricksDetails("arduino:object_detection", idProvider, cfg)
 		require.NoError(t, err)
 
@@ -425,6 +440,8 @@ func TestBricksDetails(t *testing.T) {
 		require.Equal(t, "face-detection", res.CompatibleModels[1].ID)
 		require.Equal(t, "Lightweight-Face-Detection", res.CompatibleModels[1].Name)
 		require.Equal(t, "", res.CompatibleModels[1].Description)
+		require.Len(t, res.ConfigVariables, 2)
+		require.Equal(t, expectConfigVariables, res.ConfigVariables)
 	})
 
 	t.Run("Success - Full Details - no models", func(t *testing.T) {
@@ -444,6 +461,7 @@ func TestBricksDetails(t *testing.T) {
 		require.Equal(t, "My App", res.UsedByApps[0].Name)
 		require.NotEmpty(t, res.UsedByApps[0].ID)
 		require.Len(t, res.CompatibleModels, 0)
+		require.Empty(t, res.ConfigVariables)
 	})
 
 	t.Run("Success - Full Details - one model", func(t *testing.T) {
@@ -456,6 +474,8 @@ func TestBricksDetails(t *testing.T) {
 		require.Equal(t, "face-detection", res.CompatibleModels[0].ID)
 		require.Equal(t, "Lightweight-Face-Detection", res.CompatibleModels[0].Name)
 		require.Equal(t, "", res.CompatibleModels[0].Description)
+		require.Empty(t, res.ConfigVariables)
+		require.Empty(t, res.Variables)
 	})
 }
 

internal/orchestrator/bricks/bricks_test.go

@@ -91,10 +91,11 @@ type BrickDetailsResult struct {
 	Category         string                   `json:"category"`
 	Status           string                   `json:"status"`
 	RequireModel     bool                     `json:"require_model"`
-	Variables        map[string]BrickVariable `json:"variables,omitempty"`
+	Variables        map[string]BrickVariable `json:"variables,omitempty" description:"Deprecated: use config_variables instead. This field is kept for backward compatibility."`
 	Readme           string                   `json:"readme"`
 	ApiDocsPath      string                   `json:"api_docs_path"`
 	CodeExamples     []CodeExample            `json:"code_examples"`
 	UsedByApps       []AppReference           `json:"used_by_apps"`
 	CompatibleModels []AIModel                `json:"compatible_models"`
+	ConfigVariables  []BrickConfigVariable    `json:"config_variables"`
 }