diff --git a/Featurecreep.md b/Featurecreep.md index c4c394d99..853170f1d 100644 --- a/Featurecreep.md +++ b/Featurecreep.md @@ -208,7 +208,7 @@ Teams sind global, d.h. Ein Team kann mehrere Namespaces verwalten. * [x] Cacher konfigurierbar * [ ] Validation der ankommenden structs, am besten mit https://github.com/go-validator/validator -* [ ] Wenn die ID bei irgendeiner GetByID... Methode < 1 ist soll ein error not exist geworfen werden +* [x] Wenn die ID bei irgendeiner GetByID... Methode < 1 ist soll ein error not exist geworfen werden ### Later/Nice to have diff --git a/models/list.go b/models/list.go index 2e4216409..0572d407e 100644 --- a/models/list.go +++ b/models/list.go @@ -30,6 +30,10 @@ func (l *List) AfterLoad() { // GetListByID returns a list by its ID func GetListByID(id int64) (list List, err error) { + if id < 1 { + return list, ErrListDoesNotExist{ID: id} + } + exists, err := x.ID(id).Get(&list) if err != nil { return list, err diff --git a/models/list_items.go b/models/list_items.go index 82ce86ada..699febd3f 100644 --- a/models/list_items.go +++ b/models/list_items.go @@ -75,6 +75,10 @@ func GetTasksByListID(listID int64) (tasks []*ListTask, err error) { // GetListTaskByID returns all tasks a list has func GetListTaskByID(listTaskID int64) (listTask ListTask, err error) { + if listTaskID < 1 { + return ListTask{}, ErrListTaskDoesNotExist{listTaskID} + } + exists, err := x.ID(listTaskID).Get(&listTask) if err != nil { return ListTask{}, err diff --git a/models/namespace_test.go b/models/namespace_test.go index 757028450..c324e4beb 100644 --- a/models/namespace_test.go +++ b/models/namespace_test.go @@ -40,7 +40,8 @@ func TestNamespace_Create(t *testing.T) { // Try inserting one with a nonexistant user nUser := &User{ID: 9482385} - err = dummynamespace.Create(nUser) + dnsp2 := dummynamespace + err = dnsp2.Create(nUser) assert.Error(t, err) assert.True(t, IsErrUserDoesNotExist(err)) @@ -88,5 +89,5 @@ func TestNamespace_Create(t *testing.T) { assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(nsps).Kind(), reflect.Slice) s := reflect.ValueOf(nsps) - assert.Equal(t, s.Len(), 2) + assert.Equal(t, 1, s.Len()) } diff --git a/models/namespaces.go b/models/namespaces.go index f9a7e315b..9355fb972 100644 --- a/models/namespaces.go +++ b/models/namespaces.go @@ -28,6 +28,10 @@ func (n *Namespace) AfterLoad() { // GetNamespaceByID returns a namespace object by its ID func GetNamespaceByID(id int64) (namespace Namespace, err error) { + if id < 1 { + return namespace, ErrNamespaceDoesNotExist{ID: id} + } + namespace.ID = id exists, err := x.Get(&namespace) if err != nil { diff --git a/models/team_list_test.go b/models/team_list_test.go index faaecfd74..37effb454 100644 --- a/models/team_list_test.go +++ b/models/team_list_test.go @@ -4,7 +4,6 @@ import ( "github.com/stretchr/testify/assert" "reflect" "testing" - "fmt" ) func TestTeamList(t *testing.T) { @@ -72,7 +71,6 @@ func TestTeamList(t *testing.T) { tl6 := tl tl6.ListID = 3 _, err = tl6.ReadAll(&user) - fmt.Println(tl6) assert.Error(t, err) assert.True(t, IsErrNeedToHaveListReadAccess(err)) diff --git a/models/teams.go b/models/teams.go index ad4b0f521..d28f62404 100644 --- a/models/teams.go +++ b/models/teams.go @@ -62,6 +62,10 @@ type TeamUser struct { // GetTeamByID gets a team by its ID func GetTeamByID(id int64) (team Team, err error) { + if id < 1 { + return team, ErrTeamDoesNotExist{id} + } + exists, err := x.Where("id = ?", id).Get(&team) if err != nil { return diff --git a/models/user.go b/models/user.go index a38ab8ede..f37c1bc09 100644 --- a/models/user.go +++ b/models/user.go @@ -48,8 +48,8 @@ func (apiUser *APIUserPassword) APIFormat() User { // GetUserByID gets informations about a user by its ID func GetUserByID(id int64) (user User, err error) { // Apparently xorm does otherwise look for all users but return only one, which leads to returing one even if the ID is 0 - if id == 0 { - return User{}, nil + if id < 1 { + return User{}, ErrUserDoesNotExist{} } return GetUser(User{ID: id}) diff --git a/models/user_test.go b/models/user_test.go index 253801fb4..904af48c0 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -67,9 +67,10 @@ func TestCreateUser(t *testing.T) { _, err = GetUserByID(theuser.ID) assert.NoError(t, err) - // Passing 0 as ID should return an empty user + // Passing 0 as ID should return an error _, err = GetUserByID(0) - assert.NoError(t, err) + assert.Error(t, err) + assert.True(t, IsErrUserDoesNotExist(err)) // Check the user credentials user, err := CheckUserCredentials(&UserLogin{"testuu", "1234"})