Skip to content
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

Sc 12283/db validation #86

Merged
merged 13 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 10 additions & 3 deletions pkg/tenant/db/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@ package db
import "errors"

var (
ErrNotConnected = errors.New("not connected to trtl database")
ErrNotFound = errors.New("object not found for the specified key")
ErrMissingID = errors.New("object requires id for serialization")
ErrNotConnected = errors.New("not connected to trtl database")
ErrNotFound = errors.New("object not found for the specified key")
ErrMissingID = errors.New("object requires id for serialization")
ErrMissingTenantID = errors.New("object requires tenant id for serialization")
ErrMissingProjectID = errors.New("object requires project id for serialization")
ErrNumberFirstCharacter = errors.New("name cannot begin with a number")
ErrSpecialCharacters = errors.New("name cannot include special characters")
ErrMissingMemberName = errors.New("member name is required")
ErrMissingProjectName = errors.New("project name is required")
ErrMissingTopicName = errors.New("topic name is required")
)
54 changes: 53 additions & 1 deletion pkg/tenant/db/members.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package db
import (
"context"
"fmt"
"strings"
"time"

"github.com/oklog/ulid/v2"
Expand Down Expand Up @@ -52,7 +53,52 @@ func (m *Member) UnmarshalValue(data []byte) error {
return msgpack.Unmarshal(data, m)
}

// CreateMember adds a new Member to the database.
func (m *Member) Validate() error {
if m.TenantID.Compare(ulid.ULID{}) == 0 {
return ErrMissingTenantID
}

memberName := m.Name

if memberName == "" {
return ErrMissingMemberName
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Let's add a test case for this and to the other models so that we protect ourselves against regressions when we make future changes to the validation code.


if strings.ContainsAny(string(memberName[0]), "0123456789") {
return ErrNumberFirstCharacter
}

if strings.ContainsAny(memberName, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") {
return ErrSpecialCharacters
}

return nil
}

// CreateTenantMember adds a new Member to a tenant in the database.
// Note: If a memberID is not passed in by the User, a new member id will be generated.
func CreateTenantMember(ctx context.Context, member *Member) (err error) {
// TODO: Use crypto rand and monotonic entropy with ulid.New

if member.ID.Compare(ulid.ULID{}) == 0 {
member.ID = ulid.Make()
}
pdeziel marked this conversation as resolved.
Show resolved Hide resolved

// Validate tenant member data.
if err = member.Validate(); err != nil {
return err
}

member.Created = time.Now()
member.Modified = member.Created

if err = Put(ctx, member); err != nil {
return err
}
return nil
}

// CreateMember adds a new Member to an organization in the database.
// Note: If a memberID is not passed in by the User, a new member id will be generated.
func CreateMember(ctx context.Context, member *Member) (err error) {
// TODO: Use crypto rand and monotonic entropy with ulid.New
Expand Down Expand Up @@ -112,6 +158,7 @@ func ListMembers(ctx context.Context, tenantID ulid.ULID) (members []*Member, er

// UpdateMember updates the record of a member by its id.
func UpdateMember(ctx context.Context, member *Member) (err error) {

// TODO: Use crypto rand and monotonic entropy with ulid.New

// Check if memberID exists and return a missing
Expand All @@ -120,6 +167,11 @@ func UpdateMember(ctx context.Context, member *Member) (err error) {
return ErrMissingID
}

// Validate member data.
if err = member.Validate(); err != nil {
return err
}

member.Modified = time.Now()

if err = Put(ctx, member); err != nil {
Expand Down
59 changes: 50 additions & 9 deletions pkg/tenant/db/members_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,15 @@ func TestMemberModel(t *testing.T) {
member := &db.Member{
TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"),
ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"),
Name: "member-example",
Name: "member001",
Role: "role-example",
Created: time.Unix(1670424445, 0).In(time.UTC),
Modified: time.Unix(1670424445, 0).In(time.UTC),
}

err := member.Validate()
require.NoError(t, err, "could not validate member data")

key, err := member.Key()
require.NoError(t, err, "could not marshal the key")
require.Equal(t, member.TenantID[:], key[0:16], "unexpected marshaling of the tenant id half of the key")
Expand All @@ -43,10 +46,45 @@ func TestMemberModel(t *testing.T) {
MembersEqual(t, member, other)
}

func (s *dbTestSuite) TestCreateTenantMember() {
require := s.Require()
ctx := context.Background()
member := &db.Member{
TenantID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"),
Name: "member001",
Role: "role-example",
}

err := member.Validate()
require.NoError(err, "could not validate member data")

// Call OnPut method from mock trtl database
s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) {
if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.MembersNamespace {
return nil, status.Error(codes.FailedPrecondition, "bad Put request")
}

return &pb.PutReply{
Success: true,
}, nil
}

err = db.CreateTenantMember(ctx, member)
require.NoError(err, "could not create member")

require.NotEmpty(member.ID, "expected non-zero ulid to be populated")
require.NotZero(member.Created, "expected member to have a created timestamp")
require.Equal(member.Created, member.Modified, "expected the same created and modified timestamp")
}

func (s *dbTestSuite) TestCreateMember() {
require := s.Require()
ctx := context.Background()
member := &db.Member{Name: "member-example"}
member := &db.Member{
TenantID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"),
Name: "member001",
Role: "role-example",
}

// Call OnPut method from mock trtl database
s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) {
Expand All @@ -62,7 +100,7 @@ func (s *dbTestSuite) TestCreateMember() {
err := db.CreateMember(ctx, member)
require.NoError(err, "could not create member")

require.NotEqual("", member.ID, "expected non-zero ulid to be populated")
require.NotEmpty(member.ID, "expected non-zero ulid to be populated")
require.NotZero(member.Created, "expected member to have a created timestamp")
require.Equal(member.Created, member.Modified, "expected the same created and modified timestamp")
}
Expand All @@ -73,7 +111,7 @@ func (s *dbTestSuite) TestRetrieveMember() {
member := &db.Member{
TenantID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"),
ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"),
Name: "member-example",
Name: "member001",
Role: "role-example",
}

Expand Down Expand Up @@ -106,7 +144,7 @@ func (s *dbTestSuite) TestRetrieveMember() {
require.NoError(err, "could not retrieve member")

require.Equal(ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), member.ID, "expected member id to match")
require.Equal("member-example", member.Name, "expected member name to match")
require.Equal("member001", member.Name, "expected member name to match")
require.Equal("role-example", member.Role, "expected member role to match")

// TODO: Use crypto rand and monotonic entropy with ulid.New
Expand All @@ -121,7 +159,7 @@ func (s *dbTestSuite) TestListMembers() {
member := &db.Member{
TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"),
ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"),
Name: "member-example",
Name: "member001",
Role: "role-example",
Created: time.Unix(1670424445, 0),
Modified: time.Unix(1670424445, 0),
Expand Down Expand Up @@ -164,12 +202,15 @@ func (s *dbTestSuite) TestUpdateMember() {
member := &db.Member{
TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"),
ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"),
Name: "member-example",
Name: "member001",
Role: "role-example",
Created: time.Unix(1670424445, 0),
Modified: time.Unix(1670424467, 0),
}

err := member.Validate()
require.NoError(err, "could not validate member data")

// Call OnPut method from mock trtl database
s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) {
if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.MembersNamespace {
Expand All @@ -189,7 +230,7 @@ func (s *dbTestSuite) TestUpdateMember() {
}, nil
}

err := db.UpdateMember(ctx, member)
err = db.UpdateMember(ctx, member)
require.NoError(err, "could not update member")

require.Equal(ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), member.ID, "member ID should not have changed")
Expand All @@ -198,7 +239,7 @@ func (s *dbTestSuite) TestUpdateMember() {

// Test NotFound path
// TODO: Use crypto rand and monotonic entropy with ulid.New
err = db.UpdateMember(ctx, &db.Member{ID: ulid.Make()})
err = db.UpdateMember(ctx, &db.Member{TenantID: ulid.Make(), ID: ulid.Make(), Name: "member002"})
require.ErrorIs(err, db.ErrNotFound)
}

Expand Down
54 changes: 53 additions & 1 deletion pkg/tenant/db/projects.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package db

import (
"strings"
"time"

"github.com/oklog/ulid/v2"
Expand Down Expand Up @@ -50,9 +51,54 @@ func (p *Project) UnmarshalValue(data []byte) error {
return msgpack.Unmarshal(data, p)
}

// CreateProject adds a new project to the database.
func (p *Project) Validate() error {
if p.TenantID.Compare(ulid.ULID{}) == 0 {
return ErrMissingTenantID
}

projectName := p.Name

if projectName == "" {
return ErrMissingProjectName
}

if strings.ContainsAny(string(projectName[0]), "0123456789") {
return ErrNumberFirstCharacter
}

if strings.ContainsAny(projectName, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") {
return ErrSpecialCharacters
}

return nil
}

// CreateTenantProject adds a new project to a tenant in the database.
// Note: If a project id is not passed in by the User, a new project id will be generated.
func CreateTenantProject(ctx context.Context, project *Project) (err error) {
// TODO: Use crypto rand and monotonic entropy with ulid.New
if project.ID.Compare(ulid.ULID{}) == 0 {
project.ID = ulid.Make()
}

// Validate project data.
if err = project.Validate(); err != nil {
return err
}

project.Created = time.Now()
project.Modified = project.Created

if err = Put(ctx, project); err != nil {
return err
}
return nil
}

// CreateProject adds a new project to an organization in the database.
// Note: If a project id is not passed in by the User, a new project id will be generated.
func CreateProject(ctx context.Context, project *Project) (err error) {
// TODO: Use crypto rand and monotonic entropy with ulid.New
if project.ID.Compare(ulid.ULID{}) == 0 {
project.ID = ulid.Make()
}
Expand Down Expand Up @@ -108,10 +154,16 @@ func ListProjects(ctx context.Context, tenantID ulid.ULID) (projects []*Project,

// UpdateProject updates the record of a project by its id.
func UpdateProject(ctx context.Context, project *Project) (err error) {
// TODO: Use crypto rand and monotonic entropy with ulid.New
if project.ID.Compare(ulid.ULID{}) == 0 {
return ErrMissingID
}

// Validate project data.
if err = project.Validate(); err != nil {
return err
}

project.Modified = time.Now()

if err = Put(ctx, project); err != nil {
Expand Down