Skip to content

[usage] Define db.Workspace model #10293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions components/usage/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ replace k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.23.5 /
require (
github.com/gitpod-io/gitpod/common-go v0.0.0-00010101000000-000000000000
github.com/go-sql-driver/mysql v1.6.0
github.com/relvacode/iso8601 v1.1.0
github.com/spf13/cobra v1.4.0
github.com/stretchr/testify v1.7.0
gorm.io/datatypes v1.0.6
gorm.io/driver/mysql v1.3.3
gorm.io/gorm v1.23.5
)
Expand Down
143 changes: 143 additions & 0 deletions components/usage/go.sum

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion components/usage/pkg/db/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,5 @@ func ConnectForTests(t *testing.T) *gorm.DB {
require.NoError(t, rawConn.Close(), "must close database connection")
})

return conn
return conn.Debug()
}
59 changes: 59 additions & 0 deletions components/usage/pkg/db/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
// Licensed under the GNU Affero General Public License (AGPL).
// See License-AGPL.txt in the project root for license information.

package db

import (
"database/sql/driver"
"errors"
"fmt"
"github.com/relvacode/iso8601"
"time"
)

func NewVarcharTime(t time.Time) VarcharTime {
return VarcharTime(t.UTC())
}

func NewVarcharTimeFromStr(s string) (VarcharTime, error) {
parsed, err := iso8601.ParseString(string(s))
if err != nil {
return VarcharTime{}, fmt.Errorf("failed to parse as ISO 8601: %w", err)
}
return VarcharTime(parsed), nil
}

// VarcharTime exists for cases where records are inserted into the DB as VARCHAR but actually contain a timestamp which is time.RFC3339
type VarcharTime time.Time

// Scan implements the Scanner interface.
func (n *VarcharTime) Scan(value interface{}) error {
if value == nil {
return fmt.Errorf("nil value")
}

switch s := value.(type) {
case []uint8:
if len(s) == 0 {
return errors.New("failed to parse empty varchar time")
}

parsed, err := iso8601.ParseString(string(s))
if err != nil {
return fmt.Errorf("failed to parse %v into ISO8601: %w", string(s), err)
}
*n = VarcharTime(parsed.UTC())
return nil
}
return fmt.Errorf("unknown scan value for VarcharTime with value: %v", value)
}

// Value implements the driver Valuer interface.
func (n VarcharTime) Value() (driver.Value, error) {
return time.Time(n).UTC().Format(time.RFC3339Nano), nil
}

func (n VarcharTime) String() string {
return time.Time(n).Format(time.RFC3339Nano)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a different format to what we currently store in the DB (2006-01-02T15:04:05.999999999Z07:00 vs. 2006-01-02T15:04:05.999Z) and wouldn't parse using JS new Date(<string>).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String() is only used for printing it out in a human readable form. It's the interface to Stringer() which printers use to serialize a struct. Given that the format used by JS is fairly non-standard, the human readable format here is the standard RFC3339.

There may be gotchas in other places with timestamps, and I'll be adding much more in regards to tests for those. For now, we're focused on reading records only so writes (while important) are not as a big of a deal at this point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A secondary problem with what we store right now is that we abuse VarChar to store timestamps. We rely on somewhat obscure feature of MySQL which allows for VarChart timestamp comparisons to work. The same, for example, is not true in PostgreSQL

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String() is only used for printing it out in a human readable form

@easyCZ Despite what format is standardized where, we should really thrive to have one format everywhere. Everything else will give us headaches when doing cross-service deugging.

Copy link
Member

@geropl geropl May 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A secondary problem with what we store right now is that we abuse VarChar to store timestamps

While I admin that this solution disturbed my personally feeling of "this is not the way it should be done (sic)" I think it worked rather well.

We rely on somewhat obscure feature of MySQL which allows for VarChart timestamp comparisons to work

I think it's regular string ordering, and not that far fetched. 😉

The same, for example, is not true in PostgreSQL

True, but we would not translate the current scheme 1:1 to postgresql, anyway. Even if we were to do it today. 🙃

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That being said, I'm fine with having this method returning the RFC3339 format. But let's please be careful to not confuse things in code, so we end up with reporting to different formats somewhere (so far there only ever has been one). 🙏

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage of ISO 8601 is pretty non standard in Go. I'd strongly recommend that for human consumption, we stick with the standard way.

IMO, this is a non-issue in the bigger picture of this PR. But if it enables approval of this PR and allows us to progress usage based billing, I'll happily change it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As promised, a follow-up to this issue is in a PR now: #10490 (with tests)

}
53 changes: 53 additions & 0 deletions components/usage/pkg/db/types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
// Licensed under the GNU Affero General Public License (AGPL).
// See License-AGPL.txt in the project root for license information.

package db

import (
"github.com/stretchr/testify/require"
"testing"
"time"
)

func TestVarcharTime_Scan(t *testing.T) {
for _, scenario := range []struct {
Name string

Input interface{}
Expected time.Time
Error bool
}{
{
Name: "nil value errors",
Input: nil,
Error: true,
},
{
Name: "empty uint8 slice errors",
Input: []uint8{},
Error: true,
},
{
Name: "fails with string",
Input: "2019-05-10T09:54:28.185Z",
Error: true,
},
{
Name: "parses valid ISO 8601 from TypeScript",
Input: []uint8("2019-05-10T09:54:28.185Z"),
Expected: time.Date(2019, 05, 10, 9, 54, 28, 185000000, time.UTC),
},
} {
t.Run(scenario.Name, func(t *testing.T) {
var vt VarcharTime
err := vt.Scan(scenario.Input)
if scenario.Error {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, scenario.Expected, time.Time(vt))
}
})
}
}
48 changes: 48 additions & 0 deletions components/usage/pkg/db/workspace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
// Licensed under the GNU Affero General Public License (AGPL).
// See License-AGPL.txt in the project root for license information.

package db

import (
"database/sql"
"gorm.io/datatypes"
"time"
)

// Workspace represents the underlying DB object
type Workspace struct {
ID string `gorm:"primary_key;column:id;type:char;size:36;" json:"id"`
OwnerID string `gorm:"column:ownerId;type:char;size:36;" json:"ownerId"`
ProjectID sql.NullString `gorm:"column:projectId;type:char;size:36;" json:"projectId"`
Description string `gorm:"column:description;type:varchar;size:255;" json:"description"`
Type string `gorm:"column:type;type:char;size:16;default:regular;" json:"type"`
CloneURL string `gorm:"column:cloneURL;type:varchar;size:255;" json:"cloneURL"`

ContextURL string `gorm:"column:contextURL;type:text;size:65535;" json:"contextURL"`
Context datatypes.JSON `gorm:"column:context;type:text;size:65535;" json:"context"`
Config datatypes.JSON `gorm:"column:config;type:text;size:65535;" json:"config"`
BasedOnPrebuildID sql.NullString `gorm:"column:basedOnPrebuildId;type:char;size:36;" json:"basedOnPrebuildId"`
BasedOnSnapshotID sql.NullString `gorm:"column:basedOnSnapshotId;type:char;size:36;" json:"basedOnSnapshotId"`
ImageSource datatypes.JSON `gorm:"column:imageSource;type:text;size:65535;" json:"imageSource"`
ImageNameResolved string `gorm:"column:imageNameResolved;type:varchar;size:255;" json:"imageNameResolved"`
BaseImageNameResolved string `gorm:"column:baseImageNameResolved;type:varchar;size:255;" json:"baseImageNameResolved"`

CreationTime VarcharTime `gorm:"column:creationTime;type:varchar;size:255;" json:"creationTime"`
LastModified time.Time `gorm:"column:_lastModified;type:timestamp;default:CURRENT_TIMESTAMP(6);" json:"_lastModified"`
SoftDeletedTime VarcharTime `gorm:"column:softDeletedTime;type:varchar;size:255;" json:"softDeletedTime"`
ContentDeletedTime VarcharTime `gorm:"column:contentDeletedTime;type:varchar;size:255;" json:"contentDeletedTime"`

Archived int32 `gorm:"column:archived;type:tinyint;default:0;" json:"archived"`
Shareable int32 `gorm:"column:shareable;type:tinyint;default:0;" json:"shareable"`

SoftDeleted sql.NullString `gorm:"column:softDeleted;type:char;size:4;" json:"softDeleted"`
Pinned int32 `gorm:"column:pinned;type:tinyint;default:0;" json:"pinned"`

// deleted is reserved for use by db-sync
_ int32 `gorm:"column:deleted;type:tinyint;default:0;" json:"deleted"`
}

func (d *Workspace) TableName() string {
return "d_b_workspace"
}
105 changes: 105 additions & 0 deletions components/usage/pkg/db/workspace_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
// Licensed under the GNU Affero General Public License (AGPL).
// See License-AGPL.txt in the project root for license information.

package db_test

import (
"fmt"
"github.com/gitpod-io/gitpod/usage/pkg/db"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"strings"
"testing"
)

var testObject = map[string]interface{}{
"id": "a0089911-5efc-4e2d-8714-0d3f67d82769",
"creationTime": "2019-05-10T09:54:28.185Z",
"ownerId": "39051644-7a8f-4ddb-8bd3-fbede229ad4e",
"contextURL": "https://github.com/gitpod-io/gitpod",
"description": "gitpod-io/gitpod - master",
"context": "{\"isFile\":false,\"path\":\"\",\"" +
"title\":\"gitpod-io/gitpod - master\",\"" +
"ref\":\"master\",\"" +
"revision\":\"6a463917e0718d36ee764a898db2801bc4da580f\",\"" +
"repository\":{\"cloneUrl\":\"https://github.com/gitpod-io/gitpod.git\",\"" +
"host\":\"github.com\",\"" +
"name\":\"gitpod\",\"" +
"owner\":\"gitpod-io\",\"" +
"private\":false}}",
"config": "{\"ports\":[{\"port\":3000}],\"tasks\":[],\"image\":\"eu.gcr.io/gitpod-dev/workspace-full:master.1458\"}",
"archived": "0",
"shareable": "0",
"imageSource": "{\"baseImageResolved\":\"eu.gcr.io/gitpod-dev/workspace-full@sha256:9c0892d1901210f3999c9bd64e371038bfc0505f04858e959e1bb3778dcf19c1\"}",
"imageNameResolved": "eu.gcr.io/gitpod-dev/workspace-full@sha256:9c0892d1901210f3999c9bd64e371038bfc0505f04858e959e1bb3778dcf19c1",
"_lastModified": "2020-04-14T00:31:26.101Z",
"deleted": "0",
"type": "regular",
"baseImageNameResolved": "eu.gcr.io/gitpod-dev/workspace-full@sha256:9c0892d1901210f3999c9bd64e371038bfc0505f04858e959e1bb3778dcf19c1",
"softDeleted": "gc",
"pinned": "0",
"softDeletedTime": "2020-03-24T00:03:03.777Z",
"contentDeletedTime": "2020-04-14T00:31:22.979Z",
//"basedOnPrebuildId": null,
//"basedOnSnapshotId": null,
//"projectId": null,
"cloneURL": "",
}

func TestRead(t *testing.T) {
conn := db.ConnectForTests(t)

insertRawWorkspace(t, conn, testObject)

var ws db.Workspace
tx := conn.First(&ws)
require.NoError(t, tx.Error)

require.Equal(t, stringToVarchar(t, testObject["creationTime"].(string)), ws.CreationTime)
require.Equal(t, stringToVarchar(t, testObject["softDeletedTime"].(string)), ws.SoftDeletedTime)
require.Equal(t, testObject["context"].(string), ws.Context.String())
}

func insertRawWorkspace(t *testing.T, conn *gorm.DB, rawWorkspace map[string]interface{}) string {
columns := []string{
"id", "creationTime",
"ownerId", "contextURL", "description", "context", "config", "imageSource", "imageNameResolved", "archived", "shareable",
"deleted", "type", "baseImageNameResolved", "softDeleted", "pinned", "softDeletedTime", "contentDeletedTime", "basedOnPrebuildId", "basedOnSnapshotId", "projectId", "cloneURL",
}
statement := fmt.Sprintf(`
INSERT INTO d_b_workspace (
%s
) VALUES ?;`, strings.Join(columns, ", "))

id := rawWorkspace["id"].(string)

var values []interface{}
for _, col := range columns {
val, ok := rawWorkspace[col]
if !ok {
values = append(values, "null")
} else {
values = append(values, val)
}

}

tx := conn.Debug().Exec(statement, values)
require.NoError(t, tx.Error)

t.Cleanup(func() {
tx := conn.Delete(&db.Workspace{ID: id})
require.NoError(t, tx.Error)
})

return id
}

func stringToVarchar(t *testing.T, s string) db.VarcharTime {
t.Helper()

converted, err := db.NewVarcharTimeFromStr(s)
require.NoError(t, err)
return converted
}