Merge pull request #218 from onflow/bastian/keep-order

Preserve flow.json key order across load/save

Author: turbolent

config/json/account.go

@@ -34,7 +34,9 @@ import (
 	"github.com/onflow/flowkit/v2/config"
 )
 
-type jsonAccounts map[string]account
+type jsonAccounts struct {
+	orderedMap[account]
+}
 
 const (
 	defaultHashAlgo = crypto.SHA3_256
@@ -239,36 +241,33 @@ func transformAdvancedToConfig(accountName string, a advancedAccount) (*config.A
 func (j jsonAccounts) transformToConfig() (config.Accounts, error) {
 	accounts := make(config.Accounts, 0)
 
-	for accountName, a := range j {
-		var account *config.Account
+	for accountName, a := range j.All {
+		var acc *config.Account
 		var err error
 		if a.Simple.Address != "" {
-			account, err = transformSimpleToConfig(accountName, a.Simple)
-			if err != nil {
-				return nil, err
-			}
+			acc, err = transformSimpleToConfig(accountName, a.Simple)
 		} else { // advanced format
-			account, err = transformAdvancedToConfig(accountName, a.Advanced)
-			if err != nil {
-				return nil, err
-			}
+			acc, err = transformAdvancedToConfig(accountName, a.Advanced)
+		}
+		if err != nil {
+			return nil, err
 		}
 
-		accounts = append(accounts, *account)
+		accounts = append(accounts, *acc)
 	}
 
 	return accounts, nil
 }
 
 // transformToJSON transforms config structure to json structures for saving.
 func transformAccountsToJSON(accounts config.Accounts) jsonAccounts {
-	jsonAccounts := jsonAccounts{}
+	var jsonAccounts jsonAccounts
 
 	for _, a := range accounts {
 		if a.Key.IsDefault() {
-			jsonAccounts[a.Name] = transformSimpleAccountToJSON(a)
+			jsonAccounts.Set(a.Name, transformSimpleAccountToJSON(a))
 		} else {
-			jsonAccounts[a.Name] = transformAdvancedAccountToJSON(a)
+			jsonAccounts.Set(a.Name, transformAdvancedAccountToJSON(a))
 		}
 	}
 

config/json/account_test.go

@@ -119,8 +119,10 @@ func Test_ConfigAccountKeysAdvancedFile(t *testing.T) {
 	assert.Equal(t, "", key.ResourceID)
 
 	jsonAccs := transformAccountsToJSON(accounts)
-	assert.Equal(t, "./test.pkey", jsonAccs["test"].Advanced.Key.Location)
-	assert.Equal(t, "", jsonAccs["test"].Advanced.Key.PrivateKey)
+	testAcc, ok := jsonAccs.Get("test")
+	assert.True(t, ok)
+	assert.Equal(t, "./test.pkey", testAcc.Advanced.Key.Location)
+	assert.Equal(t, "", testAcc.Advanced.Key.PrivateKey)
 }
 
 func Test_ConfigAccountKeysAdvancedKMS(t *testing.T) {

config/json/config.go

@@ -26,13 +26,19 @@ import (
 )
 
 // jsonConfig implements JSON format for persisting and parsing configuration.
+//
+// Sections use ordered maps to preserve user-chosen key ordering across
+// load/save cycles. Both `omitempty` and `omitzero` are set: `omitzero` is what
+// actually skips empty sections during marshaling (since `omitempty` is a no-op
+// for struct values), while `omitempty` is read by the invopop jsonschema
+// reflector to mark these fields as optional in the generated schema.
 type jsonConfig struct {
-	Emulators    jsonEmulators    `json:"emulators,omitempty"`
-	Contracts    jsonContracts    `json:"contracts,omitempty"`
-	Dependencies jsonDependencies `json:"dependencies,omitempty"`
-	Networks     jsonNetworks     `json:"networks,omitempty"`
-	Accounts     jsonAccounts     `json:"accounts,omitempty"`
-	Deployments  jsonDeployments  `json:"deployments,omitempty"`
+	Emulators    jsonEmulators    `json:"emulators,omitempty,omitzero"`
+	Contracts    jsonContracts    `json:"contracts,omitempty,omitzero"`
+	Dependencies jsonDependencies `json:"dependencies,omitempty,omitzero"`
+	Networks     jsonNetworks     `json:"networks,omitempty,omitzero"`
+	Accounts     jsonAccounts     `json:"accounts,omitempty,omitzero"`
+	Deployments  jsonDeployments  `json:"deployments,omitempty,omitzero"`
 }
 
 func (j *jsonConfig) transformToConfig() (*config.Config, error) {

config/json/config_test.go

@@ -18,6 +18,7 @@
 package json
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/onflow/flow-go-sdk"
@@ -255,3 +256,136 @@ func Test_SerializeConfigToJsonEmulatorNotDefault(t *testing.T) {
 	assert.JSONEq(t, string(configJson), string(conf))
 
 }
+
+// Test_RoundTripPreservesKeyOrder verifies that loading a flow.json and then
+// re-serializing it preserves the user's chosen ordering of keys within each
+// section, including nested deployments. This was the motivating bug for the
+// orderedMap data structure.
+func Test_RoundTripPreservesKeyOrder(t *testing.T) {
+	input := []byte(`{
+	"contracts": {
+		"ZetaContract": "./z.cdc",
+		"AlphaContract": "./a.cdc",
+		"MuContract": "./m.cdc"
+	},
+	"dependencies": {
+		"Burner": "testnet://9a0766d93b6608b7.Burner",
+		"AAA": "testnet://9a0766d93b6608b7.AAA"
+	},
+	"networks": {
+		"mainnet": "access.mainnet.nodes.onflow.org:9000",
+		"testnet": "access.testnet.nodes.onflow.org:9000",
+		"emulator": "127.0.0.1:3569"
+	},
+	"accounts": {
+		"zoo-account": {
+			"address": "f8d6e0586b0a20c7",
+			"key": "11c5dfdeb0ff03a7a73ef39788563b62c89adea67bbb21ab95e5f710bd1d40b7"
+		},
+		"alpha-account": {
+			"address": "f8d6e0586b0a20c7",
+			"key": "11c5dfdeb0ff03a7a73ef39788563b62c89adea67bbb21ab95e5f710bd1d40b7"
+		}
+	},
+	"deployments": {
+		"testnet": {
+			"zoo-account": ["ZetaContract"]
+		},
+		"emulator": {
+			"alpha-account": ["MuContract", "AlphaContract"]
+		}
+	}
+}`)
+
+	parser := NewParser()
+	conf, err := parser.Deserialize(input)
+	assert.NoError(t, err)
+
+	out, err := parser.Serialize(conf)
+	assert.NoError(t, err)
+
+	// Re-parse into the JSON layer directly to compare structural ordering
+	// without depending on whitespace.
+	var original, roundTripped jsonConfig
+	assert.NoError(t, json.Unmarshal(input, &original))
+	assert.NoError(t, json.Unmarshal(out, &roundTripped))
+
+	assert.Equal(t,
+		sectionKeys(original.Contracts.orderedMap),
+		sectionKeys(roundTripped.Contracts.orderedMap),
+		"contracts order changed")
+	assert.Equal(t,
+		sectionKeys(original.Dependencies.orderedMap),
+		sectionKeys(roundTripped.Dependencies.orderedMap),
+		"dependencies order changed")
+	assert.Equal(t,
+		sectionKeys(original.Networks.orderedMap),
+		sectionKeys(roundTripped.Networks.orderedMap),
+		"networks order changed")
+	assert.Equal(t,
+		sectionKeys(original.Accounts.orderedMap),
+		sectionKeys(roundTripped.Accounts.orderedMap),
+		"accounts order changed")
+	assert.Equal(t,
+		sectionKeys(original.Deployments.orderedMap),
+		sectionKeys(roundTripped.Deployments.orderedMap),
+		"deployments order changed")
+
+	// Verify inner deployment account ordering is also preserved (this is the
+	// nested orderedMap[[]deployment] case).
+	originalEmu, _ := original.Deployments.Get("emulator")
+	roundTrippedEmu, _ := roundTripped.Deployments.Get("emulator")
+	assert.Equal(t,
+		sectionKeys(originalEmu.orderedMap),
+		sectionKeys(roundTrippedEmu.orderedMap),
+		"deployments.emulator account order changed")
+}
+
+// Test_NewKeysAppendAtEnd verifies that when a new entry is added programmatically
+// (e.g. a new dependency is fetched), it is appended at the end rather than
+// inserted alphabetically.
+func Test_NewKeysAppendAtEnd(t *testing.T) {
+	input := []byte(`{
+	"contracts": {
+		"ZetaContract": "./z.cdc",
+		"AlphaContract": "./a.cdc"
+	},
+	"networks": {
+		"emulator": "127.0.0.1:3569"
+	},
+	"accounts": {
+		"emulator-account": {
+			"address": "f8d6e0586b0a20c7",
+			"key": "11c5dfdeb0ff03a7a73ef39788563b62c89adea67bbb21ab95e5f710bd1d40b7"
+		}
+	}
+}`)
+
+	parser := NewParser()
+	conf, err := parser.Deserialize(input)
+	assert.NoError(t, err)
+
+	conf.Contracts.AddOrUpdate(config.Contract{
+		Name:     "MuContract",
+		Location: "./m.cdc",
+	})
+
+	out, err := parser.Serialize(conf)
+	assert.NoError(t, err)
+
+	var result jsonConfig
+	assert.NoError(t, json.Unmarshal(out, &result))
+
+	assert.Equal(t,
+		[]string{"ZetaContract", "AlphaContract", "MuContract"},
+		sectionKeys(result.Contracts.orderedMap),
+		"new contract should be appended at the end, not inserted alphabetically")
+}
+
+func sectionKeys[V any](m orderedMap[V]) []string {
+	keys := make([]string, 0, m.Len())
+	for k := range m.All {
+		keys = append(keys, k)
+	}
+	return keys
+}

config/json/contract.go

@@ -29,44 +29,45 @@ import (
 	"github.com/onflow/flowkit/v2/config"
 )
 
-type jsonContracts map[string]jsonContract
+type jsonContracts struct {
+	orderedMap[jsonContract]
+}
 
 // transformToConfig transforms json structures to config structure.
 func (j jsonContracts) transformToConfig() (config.Contracts, error) {
 	contracts := make(config.Contracts, 0)
 
-	for contractName, c := range j {
+	for contractName, c := range j.All {
 		if c.Simple != "" {
-			contract := config.Contract{
+			contracts = append(contracts, config.Contract{
 				Name:     contractName,
 				Location: c.Simple,
-			}
+			})
+			continue
+		}
 
-			contracts = append(contracts, contract)
-		} else {
-			contract := config.Contract{
-				Name:      contractName,
-				Location:  c.Advanced.Source,
-				Canonical: c.Advanced.Canonical,
+		contract := config.Contract{
+			Name:      contractName,
+			Location:  c.Advanced.Source,
+			Canonical: c.Advanced.Canonical,
+		}
+		for network, alias := range c.Advanced.Aliases {
+			address := flow.HexToAddress(alias)
+			if address == flow.EmptyAddress {
+				return nil, fmt.Errorf("invalid alias address for a contract")
 			}
-			for network, alias := range c.Advanced.Aliases {
-				address := flow.HexToAddress(alias)
-				if address == flow.EmptyAddress {
-					return nil, fmt.Errorf("invalid alias address for a contract")
-				}
 
-				contract.Aliases.Add(network, address)
-			}
-			contracts = append(contracts, contract)
+			contract.Aliases.Add(network, address)
 		}
+		contracts = append(contracts, contract)
 	}
 
 	return contracts, nil
 }
 
 // transformToJSON transforms config structure to json structures for saving.
 func transformContractsToJSON(contracts config.Contracts) jsonContracts {
-	jsonContracts := jsonContracts{}
+	var jsonContracts jsonContracts
 
 	for _, c := range contracts {
 		// If it's a dependency, skip. These are used under the hood and should not be saved.
@@ -76,23 +77,23 @@ func transformContractsToJSON(contracts config.Contracts) jsonContracts {
 
 		// if simple case (no aliases and no canonical)
 		if !c.IsAliased() && c.Canonical == "" {
-			jsonContracts[c.Name] = jsonContract{
+			jsonContracts.Set(c.Name, jsonContract{
 				Simple: filepath.ToSlash(c.Location),
-			}
+			})
 		} else { // if advanced config
 			// check if we already created for this name then add or create
 			aliases := make(map[string]string)
 			for _, alias := range c.Aliases {
 				aliases[alias.Network] = alias.Address.String()
 			}
 
-			jsonContracts[c.Name] = jsonContract{
+			jsonContracts.Set(c.Name, jsonContract{
 				Advanced: jsonContractAdvanced{
 					Source:    filepath.ToSlash(c.Location),
 					Aliases:   aliases,
 					Canonical: c.Canonical,
 				},
-			}
+			})
 		}
 	}
 

config/json/dependency.go

@@ -30,12 +30,14 @@ import (
 	"github.com/onflow/flowkit/v2/config"
 )
 
-type jsonDependencies map[string]jsonDependency
+type jsonDependencies struct {
+	orderedMap[jsonDependency]
+}
 
 func (j jsonDependencies) transformToConfig() (config.Dependencies, error) {
 	deps := make(config.Dependencies, 0)
 
-	for dependencyName, dependency := range j {
+	for dependencyName, dependency := range j.All {
 		var dep config.Dependency
 
 		if dependency.Simple != "" {
@@ -87,7 +89,7 @@ func (j jsonDependencies) transformToConfig() (config.Dependencies, error) {
 }
 
 func transformDependenciesToJSON(configDependencies config.Dependencies, configContracts config.Contracts) jsonDependencies {
-	jsonDeps := jsonDependencies{}
+	var jsonDeps jsonDependencies
 
 	for _, dep := range configDependencies {
 		aliases := make(map[string]string)
@@ -99,15 +101,15 @@ func transformDependenciesToJSON(configDependencies config.Dependencies, configC
 			}
 		}
 
-		jsonDeps[dep.Name] = jsonDependency{
+		jsonDeps.Set(dep.Name, jsonDependency{
 			Extended: jsonDependencyExtended{
 				Source:      buildSourceString(dep.Source),
 				Hash:        dep.Hash,
 				BlockHeight: dep.BlockHeight,
 				Aliases:     aliases,
 				Canonical:   dep.Canonical,
 			},
-		}
+		})
 	}
 
 	return jsonDeps