Skip to content

Commit

Permalink
refactor(template): Introduce new File and InMemory Templates
Browse files Browse the repository at this point in the history
The updated FileBasedTemplate loads file contents only when required,
instead of at creation, reducing the long term in-use memory footprint
 of configurations.

The new InMemoryTemplate replaces the Download specific template, as
well as some dedicated test implementations of templates.
It currently carries an optional filepath, which is used for writing
converted templates to the desired target file. This may be split off
  • Loading branch information
UnseenWizzard committed Sep 22, 2023
1 parent 4e98312 commit 5a647eb
Show file tree
Hide file tree
Showing 35 changed files with 441 additions and 495 deletions.
74 changes: 27 additions & 47 deletions cmd/monaco/download/download_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ import (
// We want to be pragmatic in comparing them - so we define these options to make it very simple.
var compareOptions = []cmp.Option{
cmp.Comparer(func(a, b template.Template) bool {
return jsonEqual(a.Content(), b.Content())
cA, _ := a.Content()
cB, _ := b.Content()
return jsonEqual(cA, cB)
}),
cmpopts.SortSlices(func(a, b config.Config) bool {
return strings.Compare(a.Coordinate.String(), b.Coordinate.String()) < 0
Expand All @@ -59,28 +61,6 @@ var compareOptions = []cmp.Option{
}),
}

type contentOnlyTemplate struct {
content string
}

func (c contentOnlyTemplate) Id() string {
panic("implement me")
}

func (c contentOnlyTemplate) Name() string {
panic("implement me")
}

func (c contentOnlyTemplate) Content() string {
return c.content
}

func (c contentOnlyTemplate) UpdateContent(_ string) {
panic("implement me")
}

var _ template.Template = (*contentOnlyTemplate)(nil)

func TestDownloadIntegrationSimple(t *testing.T) {
// GIVEN apis, server responses, file system
const projectName = "integration-test-1"
Expand Down Expand Up @@ -142,7 +122,7 @@ func TestDownloadIntegrationSimple(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("template-1", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: fakeApi.ID},
},
},
Expand Down Expand Up @@ -208,7 +188,7 @@ func TestDownloadIntegrationWithReference(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("template-1", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "fake-id"},
},
{
Expand All @@ -220,7 +200,7 @@ func TestDownloadIntegrationWithReference(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}", "reference-to-id1": "{{.fakeid__id1__id}}"}`},
Template: template.NewInMemoryTemplate("template-2", `{"custom-response": true, "name": "{{.name}}", "reference-to-id1": "{{.fakeid__id1__id}}"}`),
Type: config.ClassicApiType{Api: "fake-id"},
},
},
Expand Down Expand Up @@ -295,7 +275,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "fake-id-1"},
},
{
Expand All @@ -307,7 +287,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": false, "name": "{{.name}}", "reference-to-id1": "{{.fakeid1__id1__id}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": false, "name": "{{.name}}", "reference-to-id1": "{{.fakeid1__id1__id}}"}`),
Type: config.ClassicApiType{Api: "fake-id-1"},
},
},
Expand All @@ -321,7 +301,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": "No!", "name": "{{.name}}", "subobject": {"something": "{{.fakeid1__id1__id}}"}}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": "No!", "name": "{{.name}}", "subobject": {"something": "{{.fakeid1__id1__id}}"}}`),
Type: config.ClassicApiType{Api: "fake-id-2"},
},
{
Expand All @@ -333,7 +313,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}", "reference-to-id3": "{{.fakeid2__id3__id}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": true, "name": "{{.name}}", "reference-to-id3": "{{.fakeid2__id3__id}}"}`),
Type: config.ClassicApiType{Api: "fake-id-2"},
},
},
Expand All @@ -348,7 +328,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"name": "{{.name}}", "custom-response": true, "reference-to-id6-of-another-api": ["{{.fakeid2__id4__id}}" ,{"o": "{{.fakeid1__id2__id}}"}]}`},
Template: template.NewInMemoryTemplate("id", `{"name": "{{.name}}", "custom-response": true, "reference-to-id6-of-another-api": ["{{.fakeid2__id4__id}}" ,{"o": "{{.fakeid1__id2__id}}"}]}`),
Type: config.ClassicApiType{Api: "fake-id-3"},
},
},
Expand Down Expand Up @@ -413,7 +393,7 @@ func TestDownloadIntegrationSingletonConfig(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "fake-id"},
},
},
Expand Down Expand Up @@ -479,7 +459,7 @@ func TestDownloadIntegrationSyntheticLocations(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"type": "PRIVATE", "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("id", `{"type": "PRIVATE", "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "synthetic-location"},
},
},
Expand Down Expand Up @@ -549,7 +529,7 @@ func TestDownloadIntegrationDashboards(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}", "owner": "Q"}, "tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}", "owner": "Q"}, "tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -560,7 +540,7 @@ func TestDownloadIntegrationDashboards(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}", "owner": "Admiral Jean-Luc Picard"}, "tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}", "owner": "Admiral Jean-Luc Picard"}, "tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -571,7 +551,7 @@ func TestDownloadIntegrationDashboards(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}","owner": "Not Dynatrace","preset": true},"tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}","owner": "Not Dynatrace","preset": true},"tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
},
Expand Down Expand Up @@ -644,7 +624,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}", "owner": "Q"}, "tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}", "owner": "Q"}, "tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -655,7 +635,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}", "owner": "Admiral Jean-Luc Picard"}, "tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}", "owner": "Admiral Jean-Luc Picard"}, "tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -666,7 +646,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}","owner": "Dynatrace"},"tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}","owner": "Dynatrace"},"tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -677,7 +657,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}","owner": "Not Dynatrace","preset": true},"tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}","owner": "Not Dynatrace","preset": true},"tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -688,7 +668,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}","owner": "Dynatrace","preset": true},"tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}","owner": "Dynatrace","preset": true},"tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
},
Expand Down Expand Up @@ -755,7 +735,7 @@ func TestDownloadIntegrationAnomalyDetectionMetrics(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{}`},
Template: template.NewInMemoryTemplate("id", `{}`),
Type: config.ClassicApiType{Api: "anomaly-detection-metrics"},
},
{
Expand All @@ -766,7 +746,7 @@ func TestDownloadIntegrationAnomalyDetectionMetrics(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{}`},
Template: template.NewInMemoryTemplate("id", `{}`),
Type: config.ClassicApiType{Api: "anomaly-detection-metrics"},
},
},
Expand All @@ -791,7 +771,7 @@ func TestDownloadIntegrationHostAutoUpdate(t *testing.T) {
},
Group: "default",
Environment: "valid",
Template: contentOnlyTemplate{`{"updateWindows":{"windows":[{"id":"3","name":"Daily maintenance window"}]}}`},
Template: template.NewInMemoryTemplate("id", `{"updateWindows":{"windows":[{"id":"3","name":"Daily maintenance window"}]}}`),
Type: config.ClassicApiType{Api: "hosts-auto-update"},
},
},
Expand All @@ -808,7 +788,7 @@ func TestDownloadIntegrationHostAutoUpdate(t *testing.T) {
},
Group: "default",
Environment: "updateWindows-empty",
Template: contentOnlyTemplate{`{}`},
Template: template.NewInMemoryTemplate("id", `{}`),
Type: config.ClassicApiType{Api: "hosts-auto-update"},
},
},
Expand All @@ -830,7 +810,7 @@ func TestDownloadIntegrationHostAutoUpdate(t *testing.T) {
},
Group: "default",
Environment: "windows-missing",
Template: contentOnlyTemplate{`{"updateWindows":{}}`},
Template: template.NewInMemoryTemplate("id", `{"updateWindows":{}}`),
Type: config.ClassicApiType{Api: "hosts-auto-update"},
},
},
Expand Down Expand Up @@ -982,7 +962,7 @@ func TestDownloadIntegrationOverwritesFolderAndManifestIfForced(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "fake-id"},
},
},
Expand Down
4 changes: 2 additions & 2 deletions cmd/monaco/download/download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func Test_checkForCircularDependencies(t *testing.T) {
"test_project": {
"dashboard": []config.Config{
{
Template: template.CreateTemplateFromString("some/path", "{}"),
Template: template.NewInMemoryTemplate("some/path", "{}"),
Parameters: map[string]parameter.Parameter{
"name": &valueParam.ValueParameter{Value: "name A"},
"ref": parameter.NewDummy(coordinate.Coordinate{Project: "test", Type: "dashboard", ConfigId: "b"}),
Expand All @@ -150,7 +150,7 @@ func Test_checkForCircularDependencies(t *testing.T) {
},
},
{
Template: template.CreateTemplateFromString("some/path", "{}"),
Template: template.NewInMemoryTemplate("some/path", "{}"),
Parameters: map[string]parameter.Parameter{
"name": &valueParam.ValueParameter{Value: "name A"},
"ref": parameter.NewDummy(coordinate.Coordinate{Project: "test", Type: "dashboard", ConfigId: "a"}),
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/integrationtest/v2/references_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func findSetting(t *testing.T, confsPerType project.ConfigsPerType, api, name, p

for _, c := range confs {

content := c.Template.Content()
content, _ := c.Template.Content() // TODO error handling
// convert content to json
var jsonContent map[string]interface{}
err := json.Unmarshal([]byte(content), &jsonContent)
Expand Down
3 changes: 2 additions & 1 deletion cmd/monaco/integrationtest/v2/settings_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,13 @@ func TestOldExternalIDGetsUpdated(t *testing.T) {
id, _ := idutils.GenerateExternalID(input)
return id, nil
}))
content, _ := configToDeploy.Template.Content() //TODO error handling
_, err := c.UpsertSettings(context.TODO(), dtclient.SettingsObject{
Coordinate: configToDeploy.Coordinate,
SchemaId: configToDeploy.Type.(config.SettingsType).SchemaId,
SchemaVersion: configToDeploy.Type.(config.SettingsType).SchemaVersion,
Scope: "environment",
Content: []byte(configToDeploy.Template.Content()),
Content: []byte(content),
OriginObjectId: configToDeploy.OriginObjectId,
})
assert.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ func (c *Config) Render(properties map[string]interface{}) (string, error) {
return "", nil
}

templatePath := c.Template.Name()
if t, ok := c.Template.(template.FileBasedTemplate); ok {
templatePath := c.Template.Id()
if t, ok := c.Template.(*template.FileBasedTemplate); ok {
templatePath = t.FilePath()
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func toParameterMap(params []parameter.NamedParameter) map[string]parameter.Para
func generateDummyTemplate(t *testing.T) template.Template {
newUUID, err := uuid.NewUUID()
assert.NoError(t, err)
templ := template.CreateTemplateFromString("deploy_test-"+newUUID.String(), "{}")
templ := template.NewInMemoryTemplate("deploy_test-"+newUUID.String(), "{}")
return templ
}

Expand Down
Loading

0 comments on commit 5a647eb

Please sign in to comment.