Skip to content

Commit 50a7869

Browse files
code-ashercode-asher
committed
Encode platform into version
On the storage level, the platform is appended to the version number whenever writing or reading from the file system. Everywhere else the version is passed around as a struct with the version number and the platform string.
1 parent 37778ad commit 50a7869

17 files changed

+677
-208
lines changed

Diff for: ‎api/api.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -217,17 +217,20 @@ func (api *API) extensionQuery(rw http.ResponseWriter, r *http.Request) {
217217
}
218218

219219
func (api *API) assetRedirect(rw http.ResponseWriter, r *http.Request) {
220-
// TODO: Asset URIs can contain a targetPlatform query variable.
221220
baseURL := httpapi.RequestBaseURL(r, "/")
222221
assetType := storage.AssetType(chi.URLParam(r, "type"))
223222
if assetType == "vspackage" {
224223
assetType = storage.VSIXAssetType
225224
}
225+
version := storage.VersionFromString(chi.URLParam(r, "version"))
226+
if version.TargetPlatform == "" {
227+
version.TargetPlatform = storage.Platform(r.URL.Query().Get("targetPlatform"))
228+
}
226229
url, err := api.Database.GetExtensionAssetPath(r.Context(), &database.Asset{
227230
Extension: chi.URLParam(r, "extension"),
228231
Publisher: chi.URLParam(r, "publisher"),
229232
Type: assetType,
230-
Version: chi.URLParam(r, "version"),
233+
Version: version,
231234
}, baseURL)
232235
if err != nil && os.IsNotExist(err) {
233236
httpapi.Write(rw, http.StatusNotFound, httpapi.ErrorResponse{

Diff for: ‎api/api_test.go

+18
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,24 @@ func TestAPI(t *testing.T) {
198198
Status: http.StatusMovedPermanently,
199199
Response: "/files/publisher/extension/version/foo",
200200
},
201+
{
202+
Name: "AssetOKPlatform",
203+
Path: "/assets/publisher/extension/version@linux-x64/type",
204+
Status: http.StatusMovedPermanently,
205+
Response: "/files/publisher/extension/version@linux-x64/foo",
206+
},
207+
{
208+
Name: "AssetOKPlatformQuery",
209+
Path: "/assets/publisher/extension/version/type?targetPlatform=linux-x64",
210+
Status: http.StatusMovedPermanently,
211+
Response: "/files/publisher/extension/version@linux-x64/foo",
212+
},
213+
{
214+
Name: "AssetOKDuplicatedPlatformQuery",
215+
Path: "/assets/publisher/extension/version@darwin-x64/type?targetPlatform=linux-x64",
216+
Status: http.StatusMovedPermanently,
217+
Response: "/files/publisher/extension/version@darwin-x64/foo",
218+
},
201219
{
202220
Name: "DownloadNotExist",
203221
Path: "/publishers/notexist/vsextensions/extension/version/vspackage",

Diff for: ‎cli/add_test.go

+23-4
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,30 @@ func TestAdd(t *testing.T) {
4141
extensions []testutil.Extension
4242
// name is the name of the test.
4343
name string
44+
// platforms to add for the latest version of each extension.
45+
platforms []storage.Platform
4446
// vsixes contains raw bytes of extensions to add. Use for failure cases.
4547
vsixes [][]byte
4648
}{
4749
{
4850
name: "OK",
4951
extensions: []testutil.Extension{testutil.Extensions[0]},
5052
},
53+
{
54+
name: "OKPlatforms",
55+
extensions: []testutil.Extension{testutil.Extensions[0]},
56+
platforms: []storage.Platform{
57+
storage.PlatformUnknown,
58+
storage.PlatformWin32X64,
59+
storage.PlatformLinuxX64,
60+
storage.PlatformDarwinX64,
61+
storage.PlatformWeb,
62+
},
63+
},
5164
{
5265
name: "InvalidVSIX",
5366
error: "not a valid zip",
54-
vsixes: [][]byte{[]byte{}},
67+
vsixes: [][]byte{{}},
5568
},
5669
{
5770
name: "BulkOK",
@@ -72,7 +85,7 @@ func TestAdd(t *testing.T) {
7285
testutil.Extensions[3],
7386
},
7487
vsixes: [][]byte{
75-
[]byte{},
88+
{},
7689
[]byte("foo"),
7790
},
7891
},
@@ -95,7 +108,13 @@ func TestAdd(t *testing.T) {
95108
create(vsix)
96109
}
97110
for _, ext := range test.extensions {
98-
create(testutil.CreateVSIXFromExtension(t, ext))
111+
if len(test.platforms) > 0 {
112+
for _, platform := range test.platforms {
113+
create(testutil.CreateVSIXFromExtension(t, ext, storage.Version{Version: ext.LatestVersion, TargetPlatform: platform}))
114+
}
115+
} else {
116+
create(testutil.CreateVSIXFromExtension(t, ext, storage.Version{Version: ext.LatestVersion}))
117+
}
99118
}
100119

101120
// With multiple extensions use bulk add by pointing to the directory
@@ -108,7 +127,7 @@ func TestAdd(t *testing.T) {
108127
handler := func(rw http.ResponseWriter, r *http.Request) {
109128
var vsix []byte
110129
if test.vsixes == nil {
111-
vsix = testutil.CreateVSIXFromExtension(t, test.extensions[0])
130+
vsix = testutil.CreateVSIXFromExtension(t, test.extensions[0], storage.Version{Version: test.extensions[0].LatestVersion})
112131
} else {
113132
vsix = test.vsixes[0]
114133
}

Diff for: ‎cli/remove.go

+27-21
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,14 @@ func remove() *cobra.Command {
5656
return err
5757
}
5858

59-
id := args[0]
60-
publisher, name, version, err := storage.ParseExtensionID(id)
59+
targetId := args[0]
60+
publisher, name, versionStr, err := storage.ParseExtensionID(targetId)
6161
if err != nil {
6262
return err
6363
}
6464

65-
if version != "" && all {
65+
version := storage.Version{Version: versionStr}
66+
if version.Version != "" && all {
6667
return xerrors.Errorf("cannot specify both --all and version %s", version)
6768
}
6869

@@ -72,34 +73,39 @@ func remove() *cobra.Command {
7273
}
7374

7475
versionCount := len(allVersions)
75-
if !all && version != "" && !util.Contains(allVersions, version) {
76-
return xerrors.Errorf("%s does not exist", id)
77-
} else if versionCount == 0 {
78-
return xerrors.Errorf("%s.%s has no versions to delete", publisher, name)
79-
} else if version == "" && !all {
76+
if version.Version == "" && !all {
8077
return xerrors.Errorf(
8178
"use %s@<version> to target a specific version or pass --all to delete %s of %s",
82-
id,
79+
targetId,
8380
util.Plural(versionCount, "version", ""),
84-
id,
81+
targetId,
8582
)
8683
}
87-
err = store.RemoveExtension(ctx, publisher, name, version)
88-
if err != nil {
89-
return err
90-
}
9184

92-
summary := []string{}
85+
// TODO: Allow deleting by platform as well?
86+
var toDelete []storage.Version
9387
if all {
94-
removedCount := len(allVersions)
95-
summary = append(summary, fmt.Sprintf("Removed %s", util.Plural(removedCount, "version", "")))
96-
for _, version := range allVersions {
97-
summary = append(summary, fmt.Sprintf(" - %s", version))
98-
}
88+
toDelete = allVersions
9989
} else {
100-
summary = append(summary, fmt.Sprintf("Removed %s", version))
90+
for _, sv := range allVersions {
91+
if version.Version == sv.Version {
92+
toDelete = append(toDelete, sv)
93+
}
94+
}
95+
}
96+
if len(toDelete) == 0 {
97+
return xerrors.Errorf("%s does not exist", targetId)
10198
}
10299

100+
summary := []string{fmt.Sprintf("Removed %s", util.Plural(len(toDelete), "version", ""))}
101+
for _, delete := range toDelete {
102+
err = store.RemoveExtension(ctx, publisher, name, delete)
103+
if err != nil {
104+
summary = append(summary, fmt.Sprintf(" - %s (%s)", delete, err))
105+
} else {
106+
summary = append(summary, fmt.Sprintf(" - %s", delete))
107+
}
108+
}
103109
_, err = fmt.Fprintln(cmd.OutOrStdout(), strings.Join(summary, "\n"))
104110
return err
105111
},

Diff for: ‎cli/remove_test.go

+50-12
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/stretchr/testify/require"
1111

1212
"github.com/coder/code-marketplace/cli"
13+
"github.com/coder/code-marketplace/storage"
1314
"github.com/coder/code-marketplace/testutil"
1415
)
1516

@@ -36,8 +37,11 @@ func TestRemove(t *testing.T) {
3637
all bool
3738
// error is the expected error.
3839
error string
39-
// extension is the extension to remove. testutil.Extensions[0] will be
40-
// added with versions a, b, and c before each test.
40+
// expected contains the versions should have been deleted. It is ignored
41+
// in the case of an expected error.
42+
expected []storage.Version
43+
// extension is the extension to remove. Every version of
44+
// testutil.Extensions[0] will be added before each test.
4145
extension testutil.Extension
4246
// name is the name of the test.
4347
name string
@@ -47,12 +51,41 @@ func TestRemove(t *testing.T) {
4751
{
4852
name: "RemoveOne",
4953
extension: testutil.Extensions[0],
54+
version: "2.0.0",
55+
expected: []storage.Version{
56+
{Version: "2.0.0"},
57+
},
58+
},
59+
{
60+
name: "RemovePlatforms",
61+
extension: testutil.Extensions[0],
5062
version: testutil.Extensions[0].LatestVersion,
63+
expected: []storage.Version{
64+
{Version: "3.0.0"},
65+
{Version: "3.0.0", TargetPlatform: storage.PlatformAlpineX64},
66+
{Version: "3.0.0", TargetPlatform: storage.PlatformDarwinX64},
67+
{Version: "3.0.0", TargetPlatform: storage.PlatformLinuxArm64},
68+
{Version: "3.0.0", TargetPlatform: storage.PlatformLinuxX64},
69+
{Version: "3.0.0", TargetPlatform: storage.PlatformWin32X64},
70+
},
5171
},
5272
{
5373
name: "All",
5474
extension: testutil.Extensions[0],
5575
all: true,
76+
expected: []storage.Version{
77+
{Version: "1.0.0"},
78+
{Version: "1.0.0", TargetPlatform: storage.PlatformWin32X64},
79+
{Version: "1.5.2"},
80+
{Version: "2.0.0"},
81+
{Version: "2.2.2"},
82+
{Version: "3.0.0"},
83+
{Version: "3.0.0", TargetPlatform: storage.PlatformAlpineX64},
84+
{Version: "3.0.0", TargetPlatform: storage.PlatformDarwinX64},
85+
{Version: "3.0.0", TargetPlatform: storage.PlatformLinuxArm64},
86+
{Version: "3.0.0", TargetPlatform: storage.PlatformLinuxX64},
87+
{Version: "3.0.0", TargetPlatform: storage.PlatformWin32X64},
88+
},
5689
},
5790
{
5891
name: "MissingTarget",
@@ -61,7 +94,7 @@ func TestRemove(t *testing.T) {
6194
},
6295
{
6396
name: "MissingTargetNoVersions",
64-
error: "has no versions",
97+
error: "target a specific version or pass --all",
6598
extension: testutil.Extensions[1],
6699
},
67100
{
@@ -85,10 +118,19 @@ func TestRemove(t *testing.T) {
85118
},
86119
{
87120
name: "AllNoVersions",
88-
error: "has no versions",
121+
error: "does not exist",
89122
extension: testutil.Extensions[1],
90123
all: true,
91124
},
125+
{
126+
// Cannot target specific platforms at the moment. If we wanted this
127+
// we would likely need to use a `--platform` flag since we already use @
128+
// to delineate the version.
129+
name: "NoPlatformTarget",
130+
error: "does not exist",
131+
extension: testutil.Extensions[0],
132+
version: "1.0.0@win32-x64",
133+
},
92134
}
93135

94136
for _, test := range tests {
@@ -99,7 +141,7 @@ func TestRemove(t *testing.T) {
99141
extdir := t.TempDir()
100142
ext := testutil.Extensions[0]
101143
for _, version := range ext.Versions {
102-
manifestPath := filepath.Join(extdir, ext.Publisher, ext.Name, version, "extension.vsixmanifest")
144+
manifestPath := filepath.Join(extdir, ext.Publisher, ext.Name, version.String(), "extension.vsixmanifest")
103145
err := os.MkdirAll(filepath.Dir(manifestPath), 0o755)
104146
require.NoError(t, err)
105147
err = os.WriteFile(manifestPath, testutil.ConvertExtensionToManifestBytes(t, ext, version), 0o644)
@@ -128,13 +170,9 @@ func TestRemove(t *testing.T) {
128170
require.Regexp(t, test.error, err.Error())
129171
} else {
130172
require.NoError(t, err)
131-
if test.all {
132-
require.Contains(t, output, fmt.Sprintf("Removed %d versions", len(test.extension.Versions)))
133-
for _, version := range test.extension.Versions {
134-
require.Contains(t, output, fmt.Sprintf(" - %s", version))
135-
}
136-
} else {
137-
require.Contains(t, output, fmt.Sprintf("Removed %s", test.version))
173+
require.Contains(t, output, fmt.Sprintf("Removed %d version", len(test.expected)))
174+
for _, version := range test.expected {
175+
require.Contains(t, output, fmt.Sprintf(" - %s\n", version))
138176
}
139177
}
140178
})

Diff for: ‎database/database.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,12 @@ type ExtPublisher struct {
123123
// ExtVersion implements IRawGalleryExtensionVersion.
124124
// https://github.com/microsoft/vscode/blob/29234f0219bdbf649d6107b18651a1038d6357ac/src/vs/platform/extensionManagement/common/extensionGalleryService.ts#L42-L50
125125
type ExtVersion struct {
126-
Version string `json:"version"`
126+
storage.Version
127127
LastUpdated time.Time `json:"lastUpdated"`
128128
AssetURI string `json:"assetUri"`
129129
FallbackAssetURI string `json:"fallbackAssetUri"`
130130
Files []ExtFile `json:"files"`
131131
Properties []ExtProperty `json:"properties,omitempty"`
132-
TargetPlatform string `json:"targetPlatform,omitempty"`
133132
}
134133

135134
// ExtFile implements IRawGalleryExtensionFile.
@@ -157,7 +156,7 @@ type Asset struct {
157156
Extension string
158157
Publisher string
159158
Type storage.AssetType
160-
Version string
159+
Version storage.Version
161160
}
162161

163162
type Database interface {

0 commit comments

Comments
 (0)