fix(generate)!: embedded optional structs changed to pointers (#170)

* #131. fix(generate): now uses pointers for optional fields. update: all types.

* #131. refactor(generate): extract the pointer string logic into addPointerIfOptional method in generate_utils

* #131. update(generate): Primitives are no longer prefixed as pointers.

* chore(generate): #131. Add tests to ensure no additional json fields are being added when marshalling and unmarshalling using the generated types.

* docs(adr). #174. instantiated adr directory in docs. Add ADR for using pointers for non-primitive types

* docs(readme): update readme w/ pointer example.

.adr-dir

@@ -0,0 +1 @@
+docs/adr

README.md

@@ -45,16 +45,16 @@ go get github.com/defenseunicorns/go-oscal
 oscalTypes_1_1_2 "github.com/defenseunicorns/go-oscal/src/types/oscal-1-1-2"
 
 result := oscalTypes_1_1_2.Result{
-  Findings: []oscalTypes_1_1_2.Finding{
-    {
-      Target: oscalTypes_1_1_2.FindingTarget{
-        TargetId: "ID-1",
-        Status: oscalTypes_1_1_2.ObjectiveStatus{
-          State: "satisfied",
+    Findings: &[]oscalTypes_1_1_2.Finding{
+        {
+            Target: oscalTypes_1_1_2.FindingTarget{
+                TargetId: "ID-1",
+                Status: oscalTypes_1_1_2.ObjectiveStatus{
+                    State: "satisfied",
+                },
+            },
         },
-      },
     },
-  },
 }
 ```
 ## Development

docs/adr/0001-record-architecture-decisions.md

@@ -0,0 +1,19 @@
+# 1. Record architecture decisions
+
+Date: 2024-03-25
+
+## Status
+
+Accepted
+
+## Context
+
+We need to record the architectural decisions made on this project.
+
+## Decision
+
+We will use Architecture Decision Records, as [described by Michael Nygard](http://thinkrelevance.com/blog/2011/11/15/documenting-architecture-decisions).
+
+## Consequences
+
+See Michael Nygard's article, linked above. For a lightweight ADR toolset, see Nat Pryce's [adr-tools](https://github.com/npryce/adr-tools).

docs/adr/0002-use-pointers-for-non-primitive-types.md

@@ -0,0 +1,22 @@
+# 2. Use pointers for non-primitive types
+
+Date: 2024-03-25
+
+## Status
+
+Accepted
+
+## Context
+
+Initially, types were generated without pointers. When using `encoding/json`'s `json.Marshal` and `json.Unmarshal`, all fields, even those tagged with `omitempty`, were instantiated with empty values. This can cause problems when reading, editing, and writing `json` objects, as they no longer reflect the initial and intended state of the original `json`. This issue was not present in `yaml` documents using `gopkg.in/yaml.v3`. More context is available in this [proposal](https://github.com/golang/go/issues/11939).
+
+## Decision
+
+To better support consumers who implement OSCAL and the go-oscal types in `json`, we have decided that embedded `struct` objects and non-primitives in the go-oscal types should be pointers. This is the most conventional solution to prevent the instantiation of unused (`omitempty`) `json` fields. A more in-depth explanation of the `json` `omitempty` tag can be found [here](https://www.sohamkamani.com/golang/omitempty/).
+
+## Consequences
+- **Nil Checks**: Additional `nil` checks are required before accessing methods or fields on embedded structs.
+- **Initialization**: Pointers to embedded structs must be properly initialized before use.
+- **Interface Compatibility**: Interfaces that expect embedded structs may need to be adjusted to work with pointers.
+- **Serialization and Deserialization**: Correct handling of pointers is required to ensure proper serialization and deserialization.
+- **Memory Management**: Using pointers may increase memory usage compared to embedded structs.

src/cmd/generate/generate_e2e_test.go

@@ -1,6 +1,7 @@
 package generate_test
 
 import (
+	"encoding/json"
 	"os"
 	"reflect"
 	"strings"
@@ -17,7 +18,9 @@ var (
 		"include-all": true,
 	}
 	rev4YamlPath  = "../../../testdata/generation/e2e/rev4/yaml/"
+	rev4JsonPath  = "../../../testdata/generation/e2e/rev4/json/"
 	rev5YamlPath  = "../../../testdata/generation/e2e/rev5/yaml/"
+	rev5JsonPath  = "../../../testdata/generation/e2e/rev5/json/"
 	oscal104Types = "../../types/oscal-1-0-4/types.go"
 	oscal111Types = "../../types/oscal-1-1-1/types.go"
 )
@@ -175,6 +178,92 @@ func TestFieldStability(t *testing.T) {
 	})
 }
 
+func TestNoExtraJsonFields(t *testing.T) {
+	t.Parallel()
+
+	t.Run("Rev4", func(t *testing.T) {
+		t.Parallel()
+		dir, err := os.ReadDir(rev4JsonPath)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		for _, file := range dir {
+			bytes, err := os.ReadFile(rev4JsonPath + file.Name())
+			if err != nil {
+				t.Fatal(err)
+			}
+			oscalDoc := oscalTypes_1_0_4.OscalCompleteSchema{}
+			err = json.Unmarshal(bytes, &oscalDoc)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// Marshal the document back to yaml
+			marshaled, err := json.Marshal(oscalDoc)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			actual := map[string]interface{}{}
+			expected := map[string]interface{}{}
+			err = json.Unmarshal(marshaled, &actual)
+			if err != nil {
+				t.Fatal(err)
+			}
+			err = json.Unmarshal(bytes, &expected)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !validateMapKeys(actual, expected) {
+				t.Error("expected marshaled json to be equal to the original json")
+			}
+		}
+	})
+	t.Run("Rev5", func(t *testing.T) {
+		t.Parallel()
+		dir, err := os.ReadDir(rev5JsonPath)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		for _, file := range dir {
+			bytes, err := os.ReadFile(rev5JsonPath + file.Name())
+			if err != nil {
+				t.Fatal(err)
+			}
+			oscalDoc := oscalTypes_1_1_1.OscalCompleteSchema{}
+			err = json.Unmarshal(bytes, &oscalDoc)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// Marshal the document back to yaml
+			marshaled, err := json.Marshal(oscalDoc)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			actual := map[string]interface{}{}
+			expected := map[string]interface{}{}
+			err = json.Unmarshal(marshaled, &actual)
+			if err != nil {
+				t.Fatal(err)
+			}
+			err = json.Unmarshal(bytes, &expected)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !validateMapKeys(actual, expected) {
+				t.Error("expected marshaled json to be equal to the original json")
+			}
+		}
+	})
+
+}
+
 func ValidateKeys(model map[string]interface{}, typeString string, t *testing.T) {
 	for key, value := range model {
 		if !strings.Contains(typeString, "yaml:\""+key) {
@@ -198,3 +287,22 @@ func ValidateKeys(model map[string]interface{}, typeString string, t *testing.T)
 		}
 	}
 }
+
+func collectKeys(mapData map[string]interface{}, keys map[string]struct{}) {
+	for key, value := range mapData {
+		keys[key] = struct{}{}
+		if nestedMap, ok := value.(map[string]interface{}); ok {
+			collectKeys(nestedMap, keys)
+		}
+	}
+}
+
+func validateMapKeys(map1, map2 map[string]interface{}) bool {
+	keys1 := make(map[string]struct{})
+	keys2 := make(map[string]struct{})
+
+	collectKeys(map1, keys1)
+	collectKeys(map2, keys2)
+
+	return reflect.DeepEqual(keys1, keys2)
+}

src/internal/generate/generate.go

@@ -198,6 +198,8 @@ func (c *GeneratorConfig) buildStructString(def jsonschema.Schema) (structString
 		if err != nil {
 			return structString, err
 		}
+
+		propType = addPointerIfOptionalNonPrimitive(required[key], propType)
 		propTags := buildTagString(c.tags, key, required[key])
 		structString += fmt.Sprintf("\t%s %s %s\n", propName, propType, propTags)
 	}

src/internal/generate/generate_utils.go

@@ -12,6 +12,13 @@ import (
 
 var Imports []string = []string{"time"}
 
+var GoPrimitive map[string]bool = map[string]bool{
+	"string":  true,
+	"bool":    true,
+	"float64": true,
+	"int":     true,
+}
+
 var PrimitiveAndCustomTypes map[string]string = map[string]string{
 	"string":    "string",
 	"boolean":   "bool",
@@ -104,6 +111,15 @@ var intToWordMap = []string{
 	"nine",
 }
 
+// addPointerIfOptionalNonPrimitive adds a pointer to the type if the field is optional
+func addPointerIfOptionalNonPrimitive(required bool, t string) string {
+
+	if required || GoPrimitive[t] {
+		return t
+	}
+	return "*" + t
+}
+
 // buildStructs builds the structs for each definition in the schema
 func buildTagString(tags []string, field string, required bool) string {
 	tagStrings := []string{}

src/internal/generate/generate_utils_test.go

@@ -6,6 +6,30 @@ import (
 	"github.com/swaggest/jsonschema-go"
 )
 
+func TestAddPointerIfOptionalNonPrimitive(t *testing.T) {
+	t.Parallel()
+
+	type TestCase struct {
+		required bool
+		in       string
+		out      string
+	}
+
+	testCases := []TestCase{
+		{required: false, in: "", out: "*"},
+		{required: true, in: "", out: ""},
+		{required: false, in: "string", out: "string"},
+	}
+
+	for _, testCase := range testCases {
+		actual := addPointerIfOptionalNonPrimitive(testCase.required, testCase.in)
+		expected := testCase.out
+		if expected != actual {
+			t.Fatalf("error addPointerIfOptional(): expected: %s | got: %s", expected, actual)
+		}
+	}
+}
+
 func TestBuildTagString(t *testing.T) {
 	t.Parallel()