topology: memory: allow encode/decode cycle (jaypipes#283)

After we fixed jaypipes#275 (allow marshal/unmarshal cycle for block.Info),
we discovered few more occourrences which also prevented
full JSON encode/decode/encode cycles (aka roundtrips).

All of the new findings still see unnecessary limitations,
so this patch adds the missing functions.

Fixes: jaypipes#283
Signed-off-by: Francesco Romani <fromani@redhat.com>

pkg/memory/memory_cache.go

@@ -7,6 +7,7 @@
 package memory
 
 import (
+	"encoding/json"
 	"fmt"
 	"strconv"
 	"strings"
@@ -28,6 +29,17 @@ var (
 		CACHE_TYPE_INSTRUCTION: "Instruction",
 		CACHE_TYPE_DATA:        "Data",
 	}
+
+	// NOTE(fromani): the keys are all lowercase and do not match
+	// the keys in the opposite table `memoryCacheTypeString`.
+	// This is done because of the choice we made in
+	// CacheType:MarshalJSON.
+	// We use this table only in UnmarshalJSON, so it should be OK.
+	stringMemoryCacheType = map[string]CacheType{
+		"unified":     CACHE_TYPE_UNIFIED,
+		"instruction": CACHE_TYPE_INSTRUCTION,
+		"data":        CACHE_TYPE_DATA,
+	}
 )
 
 func (a CacheType) String() string {
@@ -38,7 +50,21 @@ func (a CacheType) String() string {
 // get, let's lowercase the string output when serializing, in order to
 // "normalize" the expected serialized output
 func (a CacheType) MarshalJSON() ([]byte, error) {
-	return []byte("\"" + strings.ToLower(a.String()) + "\""), nil
+	return []byte(strconv.Quote(strings.ToLower(a.String()))), nil
+}
+
+func (a *CacheType) UnmarshalJSON(b []byte) error {
+	var s string
+	if err := json.Unmarshal(b, &s); err != nil {
+		return err
+	}
+	key := strings.ToLower(s)
+	val, ok := stringMemoryCacheType[key]
+	if !ok {
+		return fmt.Errorf("unknown memory cache type: %q", key)
+	}
+	*a = val
+	return nil
 }
 
 type SortByCacheLevelTypeFirstProcessor []*Cache

pkg/memory/memory_test.go

@@ -7,10 +7,12 @@
 package memory_test
 
 import (
+	"encoding/json"
 	"os"
 	"testing"
 
 	"github.com/jaypipes/ghw/pkg/memory"
+	"github.com/jaypipes/ghw/pkg/option"
 )
 
 // nolint: gocyclo
@@ -41,3 +43,22 @@ func TestMemory(t *testing.T) {
 		t.Fatalf("Expected >0 supported page sizes, but got 0.")
 	}
 }
+
+func TestMemoryMarshalUnmarshal(t *testing.T) {
+	data, err := memory.New(option.WithNullAlerter())
+	if err != nil {
+		t.Fatalf("Expected no error creating memory.Info, but got %v", err)
+	}
+
+	jdata, err := json.Marshal(data)
+	if err != nil {
+		t.Fatalf("Expected no error marshaling memory.Info, but got %v", err)
+	}
+
+	var topo *memory.Info
+
+	err = json.Unmarshal(jdata, &topo)
+	if err != nil {
+		t.Fatalf("Expected no error unmarshaling memory.Info, but got %v", err)
+	}
+}

pkg/pci/pci_linux_test.go

@@ -7,6 +7,7 @@
 package pci_test
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -189,3 +190,24 @@ func pciTestSetup(t *testing.T) *pci.Info {
 	}
 	return info
 }
+
+// we have this test in pci_linux_test.go (and not in pci_test.go) because `pciFillInfo` is implemented
+// only on linux; so having it in the platform-independent tests would lead to false negatives.
+func TestPCIMarshalUnmarshal(t *testing.T) {
+	data, err := pci.New(option.WithNullAlerter())
+	if err != nil {
+		t.Fatalf("Expected no error creating pci.Info, but got %v", err)
+	}
+
+	jdata, err := json.Marshal(data)
+	if err != nil {
+		t.Fatalf("Expected no error marshaling pci.Info, but got %v", err)
+	}
+
+	var topo *pci.Info
+
+	err = json.Unmarshal(jdata, &topo)
+	if err != nil {
+		t.Fatalf("Expected no error unmarshaling pci.Info, but got %v", err)
+	}
+}

pkg/topology/topology.go

@@ -7,8 +7,10 @@
 package topology
 
 import (
+	"encoding/json"
 	"fmt"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/jaypipes/ghw/pkg/context"
@@ -34,6 +36,16 @@ var (
 		ARCHITECTURE_SMP:  "SMP",
 		ARCHITECTURE_NUMA: "NUMA",
 	}
+
+	// NOTE(fromani): the keys are all lowercase and do not match
+	// the keys in the opposite table `architectureString`.
+	// This is done because of the choice we made in
+	// Architecture:MarshalJSON.
+	// We use this table only in UnmarshalJSON, so it should be OK.
+	stringArchitecture = map[string]Architecture{
+		"smp":  ARCHITECTURE_SMP,
+		"numa": ARCHITECTURE_NUMA,
+	}
 )
 
 func (a Architecture) String() string {
@@ -44,7 +56,21 @@ func (a Architecture) String() string {
 // get, let's lowercase the string output when serializing, in order to
 // "normalize" the expected serialized output
 func (a Architecture) MarshalJSON() ([]byte, error) {
-	return []byte("\"" + strings.ToLower(a.String()) + "\""), nil
+	return []byte(strconv.Quote(strings.ToLower(a.String()))), nil
+}
+
+func (a *Architecture) UnmarshalJSON(b []byte) error {
+	var s string
+	if err := json.Unmarshal(b, &s); err != nil {
+		return err
+	}
+	key := strings.ToLower(s)
+	val, ok := stringArchitecture[key]
+	if !ok {
+		return fmt.Errorf("unknown architecture: %q", key)
+	}
+	*a = val
+	return nil
 }
 
 // Node is an abstract construct representing a collection of processors and

pkg/topology/topology_linux_test.go

@@ -7,6 +7,7 @@
 package topology_test
 
 import (
+	"encoding/json"
 	"path/filepath"
 	"testing"
 
@@ -58,3 +59,24 @@ func TestTopologyNUMADistances(t *testing.T) {
 		t.Fatalf("Expected symmetric distance to the other node, got %v and %v", info.Nodes[0].Distances, info.Nodes[1].Distances)
 	}
 }
+
+// we have this test in topology_linux_test.go (and not in topology_test.go) because `topologyFillInfo`
+// is not implemented on darwin; so having it in the platform-independent tests would lead to false negatives.
+func TestTopologyMarshalUnmarshal(t *testing.T) {
+	data, err := topology.New(option.WithNullAlerter())
+	if err != nil {
+		t.Fatalf("Expected no error creating topology.Info, but got %v", err)
+	}
+
+	jdata, err := json.Marshal(data)
+	if err != nil {
+		t.Fatalf("Expected no error marshaling topology.Info, but got %v", err)
+	}
+
+	var topo *topology.Info
+
+	err = json.Unmarshal(jdata, &topo)
+	if err != nil {
+		t.Fatalf("Expected no error unmarshaling topology.Info, but got %v", err)
+	}
+}