From 309a3dda605dbda6b9f6f769ea386764671ea5d3 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Wed, 18 Jul 2018 18:32:33 -0400 Subject: Support `server`, deprecate `backend` in plugin manifest (#9127) * Support `server`, deprecate `backend` in plugin manifest This lets us converge on the use of the term `server` everywhere instead of sometimes `backend` and sometimes `server`. We're still using `webapp` and will eventually support `mobile` as well. The plan is actually to rip out these deprecations as part of releasing 5.2, but I want to coordinate the extra additional breakage at the same time, so for now this is a backwards compatible change. * fix failing tests --- model/manifest.go | 40 ++++++--- model/manifest_test.go | 180 ++++++++++++++++++++++++++++++++++++----- model/plugins_response_test.go | 2 +- 3 files changed, 192 insertions(+), 30 deletions(-) (limited to 'model') diff --git a/model/manifest.go b/model/manifest.go index 6a3b0bfad..e4ed6e4ef 100644 --- a/model/manifest.go +++ b/model/manifest.go @@ -84,7 +84,7 @@ type PluginSettingsSchema struct { // id: com.mycompany.myplugin // name: My Plugin // description: This is my plugin. It does stuff. -// backend: +// server: // executable: myplugin // settings_schema: // settings: @@ -108,8 +108,11 @@ type Manifest struct { // A version number for your plugin. Semantic versioning is recommended: http://semver.org Version string `json:"version" yaml:"version"` - // If your plugin extends the server, you'll need define backend. - Backend *ManifestBackend `json:"backend,omitempty" yaml:"backend,omitempty"` + // Server defines the server-side portion of your plugin. + Server *ManifestServer `json:"server,omitempty" yaml:"server,omitempty"` + + // Backend is a deprecated flag for defining the server-side portion of your plugin. Going forward, use Server instead. + Backend *ManifestServer `json:"backend,omitempty" yaml:"backend,omitempty"` // If your plugin extends the web app, you'll need to define webapp. Webapp *ManifestWebapp `json:"webapp,omitempty" yaml:"webapp,omitempty"` @@ -119,7 +122,7 @@ type Manifest struct { SettingsSchema *PluginSettingsSchema `json:"settings_schema,omitempty" yaml:"settings_schema,omitempty"` } -type ManifestBackend struct { +type ManifestServer struct { // Executables are the paths to your executable binaries, specifying multiple entry points // for different platforms when bundled together in a single plugin. Executables *ManifestExecutables `json:"executables,omitempty" yaml:"executables,omitempty"` @@ -181,7 +184,7 @@ func (m *Manifest) ClientManifest() *Manifest { *cm = *m cm.Name = "" cm.Description = "" - cm.Backend = nil + cm.Server = nil if cm.Webapp != nil { cm.Webapp = new(ManifestWebapp) *cm.Webapp = *m.Webapp @@ -196,28 +199,43 @@ func (m *Manifest) ClientManifest() *Manifest { // is defined, the Executable field will be returned. This method does not guarantee that the // resulting binary can actually execute on the given platform. func (m *Manifest) GetExecutableForRuntime(goOs, goArch string) string { - if m.Backend == nil { + server := m.Server + + // Support the deprecated backend parameter. + if server == nil { + server = m.Backend + } + + if server == nil { return "" } var executable string - if m.Backend.Executables != nil { + if server.Executables != nil { if goOs == "linux" && goArch == "amd64" { - executable = m.Backend.Executables.LinuxAmd64 + executable = server.Executables.LinuxAmd64 } else if goOs == "darwin" && goArch == "amd64" { - executable = m.Backend.Executables.DarwinAmd64 + executable = server.Executables.DarwinAmd64 } else if goOs == "windows" && goArch == "amd64" { - executable = m.Backend.Executables.WindowsAmd64 + executable = server.Executables.WindowsAmd64 } } if executable == "" { - executable = m.Backend.Executable + executable = server.Executable } return executable } +func (m *Manifest) HasServer() bool { + return m.Server != nil || m.Backend != nil +} + +func (m *Manifest) HasWebapp() bool { + return m.Webapp != nil +} + // FindManifest will find and parse the manifest in a given directory. // // In all cases other than a does-not-exist error, path is set to the path of the manifest file that was diff --git a/model/manifest_test.go b/model/manifest_test.go index e264a73c8..c6b31e5df 100644 --- a/model/manifest_test.go +++ b/model/manifest_test.go @@ -64,7 +64,7 @@ func TestFindManifest(t *testing.T) { func TestManifestUnmarshal(t *testing.T) { expected := Manifest{ Id: "theid", - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executable: "theexecutable", Executables: &ManifestExecutables{ LinuxAmd64: "theexecutable-linux-amd64", @@ -101,7 +101,7 @@ func TestManifestUnmarshal(t *testing.T) { var yamlResult Manifest require.NoError(t, yaml.Unmarshal([]byte(` id: theid -backend: +server: executable: theexecutable executables: linux-amd64: theexecutable-linux-amd64 @@ -129,7 +129,7 @@ settings_schema: var jsonResult Manifest require.NoError(t, json.Unmarshal([]byte(`{ "id": "theid", - "backend": { + "server": { "executable": "theexecutable", "executables": { "linux-amd64": "theexecutable-linux-amd64", @@ -185,7 +185,7 @@ func TestFindManifest_FileErrors(t *testing.T) { func TestManifestJson(t *testing.T) { manifest := &Manifest{ Id: "theid", - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executable: "theexecutable", }, Webapp: &ManifestWebapp{ @@ -230,7 +230,7 @@ func TestManifestJson(t *testing.T) { func TestManifestHasClient(t *testing.T) { manifest := &Manifest{ Id: "theid", - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executable: "theexecutable", }, Webapp: &ManifestWebapp{ @@ -250,7 +250,7 @@ func TestManifestClientManifest(t *testing.T) { Name: "thename", Description: "thedescription", Version: "0.0.1", - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executable: "theexecutable", }, Webapp: &ManifestWebapp{ @@ -287,14 +287,14 @@ func TestManifestClientManifest(t *testing.T) { assert.NotEmpty(t, sanitized.SettingsSchema) assert.Empty(t, sanitized.Name) assert.Empty(t, sanitized.Description) - assert.Empty(t, sanitized.Backend) + assert.Empty(t, sanitized.Server) assert.NotEmpty(t, manifest.Id) assert.NotEmpty(t, manifest.Version) assert.NotEmpty(t, manifest.Webapp) assert.NotEmpty(t, manifest.Name) assert.NotEmpty(t, manifest.Description) - assert.NotEmpty(t, manifest.Backend) + assert.NotEmpty(t, manifest.Server) assert.NotEmpty(t, manifest.SettingsSchema) } @@ -307,7 +307,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { ExpectedExecutable string }{ { - "no backend", + "no server", &Manifest{}, "linux", "amd64", @@ -316,7 +316,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { { "no executable", &Manifest{ - Backend: &ManifestBackend{}, + Server: &ManifestServer{}, }, "linux", "amd64", @@ -325,7 +325,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { { "single executable", &Manifest{ - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executable: "path/to/executable", }, }, @@ -336,7 +336,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { { "single executable, different runtime", &Manifest{ - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executable: "path/to/executable", }, }, @@ -347,7 +347,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { { "multiple executables, no match", &Manifest{ - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executables: &ManifestExecutables{ LinuxAmd64: "linux-amd64/path/to/executable", DarwinAmd64: "darwin-amd64/path/to/executable", @@ -362,7 +362,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { { "multiple executables, linux-amd64 match", &Manifest{ - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executables: &ManifestExecutables{ LinuxAmd64: "linux-amd64/path/to/executable", DarwinAmd64: "darwin-amd64/path/to/executable", @@ -377,7 +377,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { { "multiple executables, linux-amd64 match, single executable ignored", &Manifest{ - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executables: &ManifestExecutables{ LinuxAmd64: "linux-amd64/path/to/executable", DarwinAmd64: "darwin-amd64/path/to/executable", @@ -393,7 +393,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { { "multiple executables, darwin-amd64 match", &Manifest{ - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executables: &ManifestExecutables{ LinuxAmd64: "linux-amd64/path/to/executable", DarwinAmd64: "darwin-amd64/path/to/executable", @@ -408,7 +408,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { { "multiple executables, windows-amd64 match", &Manifest{ - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executables: &ManifestExecutables{ LinuxAmd64: "linux-amd64/path/to/executable", DarwinAmd64: "darwin-amd64/path/to/executable", @@ -423,7 +423,7 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { { "multiple executables, no match, single executable fallback", &Manifest{ - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executables: &ManifestExecutables{ LinuxAmd64: "linux-amd64/path/to/executable", DarwinAmd64: "darwin-amd64/path/to/executable", @@ -436,6 +436,43 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { "amd64", "path/to/executable", }, + { + "deprecated backend field, ignored since server present", + &Manifest{ + Server: &ManifestServer{ + Executables: &ManifestExecutables{ + LinuxAmd64: "linux-amd64/path/to/executable", + DarwinAmd64: "darwin-amd64/path/to/executable", + WindowsAmd64: "windows-amd64/path/to/executable", + }, + }, + Backend: &ManifestServer{ + Executables: &ManifestExecutables{ + LinuxAmd64: "linux-amd64/path/to/executable/backend", + DarwinAmd64: "darwin-amd64/path/to/executable/backend", + WindowsAmd64: "windows-amd64/path/to/executable/backend", + }, + }, + }, + "linux", + "amd64", + "linux-amd64/path/to/executable", + }, + { + "deprecated backend field used, since no server present", + &Manifest{ + Backend: &ManifestServer{ + Executables: &ManifestExecutables{ + LinuxAmd64: "linux-amd64/path/to/executable/backend", + DarwinAmd64: "darwin-amd64/path/to/executable/backend", + WindowsAmd64: "windows-amd64/path/to/executable/backend", + }, + }, + }, + "linux", + "amd64", + "linux-amd64/path/to/executable/backend", + }, } for _, testCase := range testCases { @@ -448,3 +485,110 @@ func TestManifestGetExecutableForRuntime(t *testing.T) { }) } } + +func TestManifestHasServer(t *testing.T) { + testCases := []struct { + Description string + Manifest *Manifest + Expected bool + }{ + { + "no server", + &Manifest{}, + false, + }, + { + "no executable, but server still considered present", + &Manifest{ + Server: &ManifestServer{}, + }, + true, + }, + { + "single executable", + &Manifest{ + Server: &ManifestServer{ + Executable: "path/to/executable", + }, + }, + true, + }, + { + "multiple executables", + &Manifest{ + Server: &ManifestServer{ + Executables: &ManifestExecutables{ + LinuxAmd64: "linux-amd64/path/to/executable", + DarwinAmd64: "darwin-amd64/path/to/executable", + WindowsAmd64: "windows-amd64/path/to/executable", + }, + }, + }, + true, + }, + { + "single executable defined via deprecated backend", + &Manifest{ + Backend: &ManifestServer{ + Executable: "path/to/executable", + }, + }, + true, + }, + { + "multiple executables defined via deprecated backend", + &Manifest{ + Backend: &ManifestServer{ + Executables: &ManifestExecutables{ + LinuxAmd64: "linux-amd64/path/to/executable", + DarwinAmd64: "darwin-amd64/path/to/executable", + WindowsAmd64: "windows-amd64/path/to/executable", + }, + }, + }, + true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Description, func(t *testing.T) { + assert.Equal(t, testCase.Expected, testCase.Manifest.HasServer()) + }) + } +} + +func TestManifestHasWebapp(t *testing.T) { + testCases := []struct { + Description string + Manifest *Manifest + Expected bool + }{ + { + "no webapp", + &Manifest{}, + false, + }, + { + "no bundle path, but webapp still considered present", + &Manifest{ + Webapp: &ManifestWebapp{}, + }, + true, + }, + { + "bundle path defined", + &Manifest{ + Webapp: &ManifestWebapp{ + BundlePath: "path/to/bundle", + }, + }, + true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Description, func(t *testing.T) { + assert.Equal(t, testCase.Expected, testCase.Manifest.HasWebapp()) + }) + } +} diff --git a/model/plugins_response_test.go b/model/plugins_response_test.go index 1aa3cbcd0..4b889ea1c 100644 --- a/model/plugins_response_test.go +++ b/model/plugins_response_test.go @@ -10,7 +10,7 @@ import ( func TestPluginsResponseJson(t *testing.T) { manifest := &Manifest{ Id: "theid", - Backend: &ManifestBackend{ + Server: &ManifestServer{ Executable: "theexecutable", }, Webapp: &ManifestWebapp{ -- cgit v1.2.3-1-g7c22