diff --git a/datastores/mysql.go b/datastores/mysql.go index d3e3921..e556f22 100644 --- a/datastores/mysql.go +++ b/datastores/mysql.go @@ -110,7 +110,7 @@ func (ds Mysql) FindProfileByKeycloakID(keycloakId string) (*proto.Profile, erro profiles, err := ds.processProfiles([]milpacs.Profile{profile}) if err != nil { - return nil, fmt.Errorf("error generating profile") + return nil, fmt.Errorf("error generating profile: %w", err) } return profiles[profile.RelationId], nil @@ -134,7 +134,7 @@ func (ds Mysql) FindProfileByDiscordID(discordId string) (*proto.Profile, error) profiles, err := ds.processProfiles([]milpacs.Profile{profile}) if err != nil { - return nil, fmt.Errorf("error generating profile") + return nil, fmt.Errorf("error generating profile: %w", err) } return profiles[profile.RelationId], nil @@ -356,7 +356,7 @@ func (ds Mysql) FindS1UniformsRosterByType(rosterType proto.RosterType) (*proto. milpac, err := ds.generateS1UniformsProtoProfile(profile) if err != nil { - return nil, fmt.Errorf("error generating profile") + return nil, fmt.Errorf("error generating profile: %w", err) } profiles[profile.RelationId] = milpac } diff --git a/servers/grpc/grpc.go b/servers/grpc/grpc.go index fd59b10..4b9f6f0 100644 --- a/servers/grpc/grpc.go +++ b/servers/grpc/grpc.go @@ -80,7 +80,7 @@ func (server *MilpacsService) GetRoster(ctx context.Context, request *proto.Rost return nil, err } if request.Roster == proto.RosterType_ROSTER_TYPE_UNSPECIFIED { - return nil, errors.New("cannot request null roster type") + return nil, status.Errorf(codes.InvalidArgument, "cannot request null roster type") } roster, err := server.Datastore.FindRosterByType(request.Roster) @@ -98,7 +98,7 @@ func (server *MilpacsService) GetUserViaKeycloakId(ctx context.Context, request } if request.GetKeycloakId() == "" { - Warn.Println("Empty Keycloak ID provided, cannot return profile") + return nil, status.Errorf(codes.InvalidArgument, "keycloak id cannot be empty") } profile, err := server.Datastore.FindProfileByKeycloakID(request.GetKeycloakId()) @@ -119,7 +119,7 @@ func (server *MilpacsService) GetUserViaDiscordId(ctx context.Context, request * } if request.GetDiscordId() == "" { - Warn.Println("Empty Discord ID provided, cannot return profile") + return nil, status.Errorf(codes.InvalidArgument, "discord id cannot be empty") } profile, err := server.Datastore.FindProfileByDiscordID(request.GetDiscordId()) @@ -138,7 +138,7 @@ func (server *MilpacsService) GetLiteRoster(ctx context.Context, request *proto. return nil, err } if request.Roster == proto.RosterType_ROSTER_TYPE_UNSPECIFIED { - return nil, errors.New("cannot request null roster type") + return nil, status.Errorf(codes.InvalidArgument, "cannot request null roster type") } roster, err := server.Datastore.FindLiteRosterByType(request.Roster) @@ -170,7 +170,7 @@ func (server *MilpacsService) GetS1UniformsRoster(ctx context.Context, request * return nil, err } if request.Roster == proto.RosterType_ROSTER_TYPE_UNSPECIFIED { - return nil, errors.New("cannot request null roster type") + return nil, status.Errorf(codes.InvalidArgument, "cannot request null roster type") } roster, err := server.Datastore.FindS1UniformsRosterByType(request.Roster) diff --git a/servers/grpc/milpacs_test.go b/servers/grpc/milpacs_test.go index 2f64718..6c1c489 100644 --- a/servers/grpc/milpacs_test.go +++ b/servers/grpc/milpacs_test.go @@ -142,6 +142,62 @@ func TestGetGamertagProfile_DatastoreError(t *testing.T) { assert.Equal(t, codes.Internal, st.Code()) } +func TestGetUserViaKeycloakId_EmptyID_InvalidArgument(t *testing.T) { + // findProfileByKeycloakID left unset: a fall-through to the datastore + // nil-derefs and panics, proving the handler early-returns instead. + svc := &MilpacsService{Datastore: &fakeDatastore{}} + _, err := svc.GetUserViaKeycloakId(withMilpacsKey("read"), &proto.KeycloakIdRequest{KeycloakId: ""}) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) +} + +func TestGetUserViaDiscordId_EmptyID_InvalidArgument(t *testing.T) { + svc := &MilpacsService{Datastore: &fakeDatastore{}} + _, err := svc.GetUserViaDiscordId(withMilpacsKey("read"), &proto.DiscordIdRequest{DiscordId: ""}) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) +} + +func TestGetGamertagProfile_Empty_InvalidArgument(t *testing.T) { + svc := &MilpacsService{Datastore: &fakeDatastore{}} + _, err := svc.GetGamertagProfile(withMilpacsKey("read"), &proto.GamertagRequest{Gamertag: ""}) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) +} + +func TestGetRoster_NullType_InvalidArgument(t *testing.T) { + svc := &MilpacsService{Datastore: &fakeDatastore{}} + _, err := svc.GetRoster(withMilpacsKey("read"), &proto.RosterRequest{Roster: proto.RosterType_ROSTER_TYPE_UNSPECIFIED}) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) +} + +func TestGetLiteRoster_NullType_InvalidArgument(t *testing.T) { + svc := &MilpacsService{Datastore: &fakeDatastore{}} + _, err := svc.GetLiteRoster(withMilpacsKey("read"), &proto.RosterRequest{Roster: proto.RosterType_ROSTER_TYPE_UNSPECIFIED}) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) +} + +func TestGetS1UniformsRoster_NullType_InvalidArgument(t *testing.T) { + svc := &MilpacsService{Datastore: &fakeDatastore{}} + _, err := svc.GetS1UniformsRoster(withMilpacsKey("read"), &proto.RosterRequest{Roster: proto.RosterType_ROSTER_TYPE_UNSPECIFIED}) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) +} + func TestGetRoster_DatastoreError(t *testing.T) { svc := &MilpacsService{Datastore: &fakeDatastore{ findRosterByType: func(proto.RosterType) (*proto.Roster, error) {