Skip to content

Commit

Permalink
chore:minor improvements (#222)
Browse files Browse the repository at this point in the history 
* cleaned up more improper logging calls

* made sure that all db operations are in the same place

* renamed a few things to be closer to what they actually are

* extracted the db-intalisation to a separate function

* migrated some of the if-error statements to this syntax

* minor whitespace refactorings

* unnessesary `status.Errorf` instead of  `status.Error`
  • Loading branch information
@CommanderStorm
CommanderStorm authored Sep 14, 2023
1 parent 43d4014 commit facf354
Show file tree
Hide file tree
Showing 13 changed files with 111 additions and 157 deletions.
8 changes: 3 additions & 5 deletions client/localServer/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func canteenHeadCount(c pb.CampusClient, ctx context.Context) {
})

if err != nil {
log.Error(err)
log.WithError(err).Error("Canteen HeadCount data request failed.")
} else {
log.WithField("res", res).Info("Canteen HeadCount data request successful.")
}
Expand Down Expand Up @@ -245,8 +245,7 @@ func getImageToBytes(path string) []byte {
}

defer func(file *os.File) {
err := file.Close()
if err != nil {
if err := file.Close(); err != nil {
log.WithError(err).Error("could not close file")
}
}(file)
Expand Down Expand Up @@ -291,8 +290,7 @@ func storeImage(path string, i []byte) (string, error) {
log.WithError(errFile).Error("Unable to create the new testfile")
}
defer func(out *os.File) {
err := out.Close()
if err != nil {
if err := out.Close(); err != nil {
log.WithError(err).Error("File was not closed successfully")
}
}(out)
Expand Down
5 changes: 2 additions & 3 deletions client/publicServer/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ func main() {

conn, err := grpc.Dial(address, grpc.WithTransportCredentials(creds))
if err != nil {
log.WithError(err).Fatalf("did not connect")
log.WithError(err).Fatal("did not connect")
}
defer func(conn *grpc.ClientConn) {
err := conn.Close()
if err != nil {
if err := conn.Close(); err != nil {
log.WithError(err).Error("did not close connection")
}
}(conn)
Expand Down
6 changes: 2 additions & 4 deletions server/backend/cafeteriaRatingDBInitializer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,7 @@ func generateNameTagListFromFile(path string) multiLanguageNameTags {
log.WithError(errjson).Error("Error while reading the file.")
}
defer func(jsonFile *os.File) {
err := jsonFile.Close()
if err != nil {
if err := jsonFile.Close(); err != nil {
log.WithError(err).Error("Error in parsing json.")
}
}(file)
Expand All @@ -224,8 +223,7 @@ func generateRatingTagListFromFile(path string) multiLanguageTags {
log.WithError(errjson).Error("Error while reading or parsing the file.")
}
defer func(jsonFile *os.File) {
err := jsonFile.Close()
if err != nil {
if err := jsonFile.Close(); err != nil {
log.WithError(err).Error("Error in parsing json.")
}
}(file)
Expand Down
64 changes: 28 additions & 36 deletions server/backend/cafeteriaService.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (s *CampusServer) GetCafeteriaRatings(_ context.Context, input *pb.Cafeteri

if res.Error != nil {
log.WithError(res.Error).Error("Error while querying the cafeteria with Id ", cafeteriaId)
return nil, status.Errorf(codes.Internal, "This cafeteria has not yet been rated.")
return nil, status.Error(codes.Internal, "This cafeteria has not yet been rated.")
}

if res.RowsAffected > 0 {
Expand Down Expand Up @@ -157,7 +157,7 @@ func (s *CampusServer) GetDishRatings(_ context.Context, input *pb.DishRatingReq
if err.Error != nil {
fields := log.Fields{"dishID": dishID, "cafeteriaID": cafeteriaID}
log.WithError(err.Error).WithFields(fields).Error("Error while querying the average ratings")
return nil, status.Errorf(codes.Internal, "This dish has not yet been rated.")
return nil, status.Error(codes.Internal, "This dish has not yet been rated.")
}

if err.RowsAffected > 0 {
Expand Down Expand Up @@ -249,15 +249,12 @@ func getImageToBytes(path string) []byte {
return make([]byte, 0)
}
file, err := os.Open(path)

if err != nil {
log.WithError(err).Error("while opening image file with path: ", path)
return nil
}

defer func(file *os.File) {
err := file.Close()
if err != nil {
if err := file.Close(); err != nil {
log.WithError(err).Error("Unable to close the file for storing the image.")
}
}(file)
Expand All @@ -267,8 +264,7 @@ func getImageToBytes(path string) []byte {
imageAsBytes := make([]byte, size)

buffer := bufio.NewReader(file)
_, err = buffer.Read(imageAsBytes)
if err != nil {
if _, err = buffer.Read(imageAsBytes); err != nil {
log.WithError(err).Error("while trying to read image as bytes")
return nil
}
Expand Down Expand Up @@ -376,10 +372,9 @@ func (s *CampusServer) NewCafeteriaRating(_ context.Context, input *pb.NewCafete
Image: resPath,
}

err := s.db.Model(&model.CafeteriaRating{}).Create(&rating).Error
if err != nil {
if err := s.db.Model(&model.CafeteriaRating{}).Create(&rating).Error; err != nil {
log.WithError(err).Error("Error occurred while creating the new cafeteria rating.")
return nil, status.Errorf(codes.InvalidArgument, "Error while creating new cafeteria rating. Rating has not been saved.")
return nil, status.Error(codes.InvalidArgument, "Error while creating new cafeteria rating. Rating has not been saved.")

}
return storeRatingTags(s, rating.CafeteriaRating, input.RatingTags, CAFETERIA)
Expand All @@ -400,12 +395,11 @@ func imageWrapper(image []byte, path string, id int32) string {
}

// storeImage
// stores an image and returns teh path to this image.
// stores an image and returns the path to this image.
// if needed, a new directory will be created and the path is extended until it is unique
func storeImage(path string, i []byte) (string, error) {
if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) {
err := os.MkdirAll(path, os.ModePerm)
if err != nil {
if err := os.MkdirAll(path, os.ModePerm); err != nil {
log.WithError(err).WithField("path", path).Error("Directory could not be created successfully")
return "", nil
}
Expand All @@ -430,8 +424,7 @@ func storeImage(path string, i []byte) (string, error) {
return imgPath, errFile
}
defer func(out *os.File) {
err := out.Close()
if err != nil {
if err := out.Close(); err != nil {
log.WithError(err).Error("while closing the file.")
}
}(out)
Expand Down Expand Up @@ -459,7 +452,7 @@ func (s *CampusServer) NewDishRating(_ context.Context, input *pb.NewDishRatingR
First(&dish).Error
if errDish != nil || dish == nil {
log.WithError(errDish).Error("Error while creating a new dish rating.")
return nil, status.Errorf(codes.InvalidArgument, "Dish is not offered in this week in this canteen. Rating has not been saved.")
return nil, status.Error(codes.InvalidArgument, "Dish is not offered in this week in this canteen. Rating has not been saved.")
}

resPath := imageWrapper(input.Image, "dishes", dish.Dish)
Expand All @@ -473,10 +466,9 @@ func (s *CampusServer) NewDishRating(_ context.Context, input *pb.NewDishRatingR
Image: resPath,
}

err := s.db.Model(&model.DishRating{}).Create(&rating).Error
if err != nil {
if err := s.db.Model(&model.DishRating{}).Create(&rating).Error; err != nil {
log.WithError(err).Error("while creating a new dish rating.")
return nil, status.Errorf(codes.Internal, "Error while creating the new rating in the database. Rating has not been saved.")
return nil, status.Error(codes.Internal, "Error while creating the new rating in the database. Rating has not been saved.")
}

assignDishNameTag(s, rating, dish.Dish)
Expand Down Expand Up @@ -511,24 +503,24 @@ func assignDishNameTag(s *CampusServer, rating model.DishRating, dishID int32) {
// Additionally, queries the cafeteria ID, since it checks whether the cafeteria actually exists.
func inputSanitizationForNewRatingElements(rating int32, comment string, cafeteriaName string, s *CampusServer) (int32, error) {
if rating > 5 || rating < 0 {
return -1, status.Errorf(codes.InvalidArgument, "Rating must be a positive number not larger than 10. Rating has not been saved.")
return -1, status.Error(codes.InvalidArgument, "Rating must be a positive number not larger than 10. Rating has not been saved.")
}

if len(comment) > 256 {
return -1, status.Errorf(codes.InvalidArgument, "Ratings can only contain up to 256 characters, this is too long. Rating has not been saved.")
return -1, status.Error(codes.InvalidArgument, "Ratings can only contain up to 256 characters, this is too long. Rating has not been saved.")
}

if strings.Contains(comment, "@") {
return -1, status.Errorf(codes.InvalidArgument, "Comments must not contain @ symbols in order to prevent misuse. Rating has not been saved.")
return -1, status.Error(codes.InvalidArgument, "Comments must not contain @ symbols in order to prevent misuse. Rating has not been saved.")
}

var result *model.Cafeteria
res := s.db.Model(&model.Cafeteria{}).
Where("name LIKE ?", cafeteriaName).
First(&result)
if res.Error == gorm.ErrRecordNotFound || res.RowsAffected == 0 {
if errors.Is(res.Error, gorm.ErrRecordNotFound) || res.RowsAffected == 0 {
log.WithError(res.Error).Error("Error while querying the cafeteria id by name: ", cafeteriaName)
return -1, status.Errorf(codes.InvalidArgument, "Cafeteria does not exist. Rating has not been saved.")
return -1, status.Error(codes.InvalidArgument, "Cafeteria does not exist. Rating has not been saved.")
}

return result.Cafeteria, nil
Expand Down Expand Up @@ -587,11 +579,11 @@ func storeRatingTags(s *CampusServer, parentRatingID int32, tags []*pb.RatingTag
}

if len(errorOccurred) > 0 && len(warningOccurred) > 0 {
return &emptypb.Empty{}, status.Errorf(codes.InvalidArgument, "The tag(s) "+errorOccurred+" does not exist. Remaining rating was saved without this rating tag. The tag(s) "+warningOccurred+" occurred more than once in this rating.")
return &emptypb.Empty{}, status.Error(codes.InvalidArgument, fmt.Sprintf("The tag(s) %s does not exist. Remaining rating was saved without this rating tag. The tag(s) %s occurred more than once in this rating.", errorOccurred, warningOccurred))
} else if len(errorOccurred) > 0 {
return &emptypb.Empty{}, status.Errorf(codes.InvalidArgument, "The tag(s) "+errorOccurred+" does not exist. Remaining rating was saved without this rating tag.")
return &emptypb.Empty{}, status.Error(codes.InvalidArgument, fmt.Sprintf("The tag(s) %s does not exist. Remaining rating was saved without this rating tag.", errorOccurred))
} else if len(warningOccurred) > 0 {
return &emptypb.Empty{}, status.Errorf(codes.InvalidArgument, "The tag(s) "+warningOccurred+" occurred more than once in this rating.")
return &emptypb.Empty{}, status.Error(codes.InvalidArgument, fmt.Sprintf("The tag(s) %s occurred more than once in this rating.", warningOccurred))
} else {
return &emptypb.Empty{}, nil
}
Expand Down Expand Up @@ -641,7 +633,7 @@ func (s *CampusServer) GetAvailableDishTags(_ context.Context, _ *emptypb.Empty)
err := s.db.Model(&model.DishRatingTagOption{}).Select("DE as de, EN as en, dishRatingTagOption as TagId").Find(&result).Error
if err != nil {
log.WithError(err).Error("while loading Cafeterias from database.")
requestStatus = status.Errorf(codes.Internal, "Available dish tags could not be loaded from the database.")
requestStatus = status.Error(codes.Internal, "Available dish tags could not be loaded from the database.")
}

return &pb.GetTagsReply{
Expand All @@ -657,7 +649,7 @@ func (s *CampusServer) GetNameTags(_ context.Context, _ *emptypb.Empty) (*pb.Get
err := s.db.Model(&model.DishNameTagOption{}).Select("DE as de, EN as en, dishNameTagOption as TagId").Find(&result).Error
if err != nil {
log.WithError(err).Error("while loading available Name Tags from database.")
requestStatus = status.Errorf(codes.Internal, "Available dish tags could not be loaded from the database.")
requestStatus = status.Error(codes.Internal, "Available dish tags could not be loaded from the database.")
}

return &pb.GetTagsReply{
Expand All @@ -673,7 +665,7 @@ func (s *CampusServer) GetAvailableCafeteriaTags(_ context.Context, _ *emptypb.E
err := s.db.Model(&model.CafeteriaRatingTagOption{}).Select("DE as de, EN as en, cafeteriaRatingsTagOption as TagId").Find(&result).Error
if err != nil {
log.WithError(err).Error("while loading Cafeterias from database.")
requestStatus = status.Errorf(codes.Internal, "Available cafeteria tags could not be loaded from the database.")
requestStatus = status.Error(codes.Internal, "Available cafeteria tags could not be loaded from the database.")
}

return &pb.GetTagsReply{
Expand All @@ -689,7 +681,7 @@ func (s *CampusServer) GetCafeterias(_ context.Context, _ *emptypb.Empty) (*pb.G
err := s.db.Model(&model.Cafeteria{}).Select("cafeteria as id,address,latitude,longitude").Scan(&result).Error
if err != nil {
log.WithError(err).Error("while loading Cafeterias from database.")
requestStatus = status.Errorf(codes.Internal, "Cafeterias could not be loaded from the database.")
requestStatus = status.Error(codes.Internal, "Cafeterias could not be loaded from the database.")
}

return &pb.GetCafeteriaReply{
Expand All @@ -699,13 +691,13 @@ func (s *CampusServer) GetCafeterias(_ context.Context, _ *emptypb.Empty) (*pb.G

func (s *CampusServer) GetDishes(_ context.Context, request *pb.GetDishesRequest) (*pb.GetDishesReply, error) {
if request.Year < 2022 {
return &pb.GetDishesReply{}, status.Errorf(codes.Internal, "Years must be larger or equal to 2022 ") // currently, no previous values have been added
return &pb.GetDishesReply{}, status.Error(codes.Internal, "Years must be larger or equal to 2022 ") // currently, no previous values have been added
}
if request.Week < 1 || request.Week > 53 {
return &pb.GetDishesReply{}, status.Errorf(codes.Internal, "Weeks must be in the range 1 - 53")
return &pb.GetDishesReply{}, status.Error(codes.Internal, "Weeks must be in the range 1 - 53")
}
if request.Day < 0 || request.Day > 4 {
return &pb.GetDishesReply{}, status.Errorf(codes.Internal, "Days must be in the range 1 (Monday) - 4 (Friday)")
return &pb.GetDishesReply{}, status.Error(codes.Internal, "Days must be in the range 1 (Monday) - 4 (Friday)")
}

var requestStatus error = nil
Expand All @@ -721,7 +713,7 @@ func (s *CampusServer) GetDishes(_ context.Context, request *pb.GetDishesRequest

if err != nil {
log.WithError(err).Error("while loading Cafeterias from database.")
requestStatus = status.Errorf(codes.Internal, "Cafeterias could not be loaded from the database.")
requestStatus = status.Error(codes.Internal, "Cafeterias could not be loaded from the database.")
}

return &pb.GetDishesReply{
Expand Down
9 changes: 2 additions & 7 deletions server/backend/campus_api/campusApi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,18 @@ func FetchGrades(token string) (*model.IOSGrades, error) {
req.URL.RawQuery = q.Encode()

resp, err := http.DefaultClient.Do(req)

if err != nil {
log.WithError(err).Error("failed to fetch grades")
return nil, ErrWhileFetchingGrades
}

defer func(Body io.ReadCloser) {
err := Body.Close()
if err != nil {
if err := Body.Close(); err != nil {
log.WithError(err).Error("Could not close body")
}
}(resp.Body)

var grades model.IOSGrades
err = xml.NewDecoder(resp.Body).Decode(&grades)

if err != nil {
if err = xml.NewDecoder(resp.Body).Decode(&grades); err != nil {
log.WithError(err).Error("could not unmarshall grades")
return nil, ErrorWhileUnmarshalling
}
Expand Down
13 changes: 5 additions & 8 deletions server/backend/cron/canteenHeadCount.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,17 +189,16 @@ func updateDb(canteen *CanteenApInformation, count uint32, db *gorm.DB) error {
}

if res.RowsAffected == 0 {
err := db.Create(&entry).Error
if err != nil {
if err := db.Create(&entry).Error; err != nil {
fields := log.Fields{
"CanteenId": entry.CanteenId,
"Count": entry.Count,
"MaxCount": entry.MaxCount,
"Percent": entry.Percent,
"Timestamp": entry.Timestamp}
log.WithError(res.Error).WithFields(fields).Error("could not create headcount entry")
return err
}
return err
}
return nil
}
Expand All @@ -216,17 +215,15 @@ func (canteen CanteenApInformation) requestApData() []AccessPoint {
// Ensure we close the body once we leave this function
if resp.Body != nil {
defer func(Body io.ReadCloser) {
err := Body.Close()
if err != nil {
if err := Body.Close(); err != nil {
log.WithError(err).Error("Could not close body")
}
}(resp.Body)
}

// Parse as JSON
aps := []AccessPoint{}
err = json.NewDecoder(resp.Body).Decode(&aps)
if err != nil {
var aps []AccessPoint
if err = json.NewDecoder(resp.Body).Decode(&aps); err != nil {
log.WithError(err).Error("Canteen HeadCount parsing output as JSON failed for: ", canteen.CanteenId)
return []AccessPoint{}
}
Expand Down
14 changes: 5 additions & 9 deletions server/backend/ios_notifications/ios_apns/iosAPNsRepository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,20 +100,18 @@ func (r *Repository) SendNotification(notification *model.IOSNotificationPayload

resp, err := client.Do(req)
if err != nil {
log.Error(err)
log.WithError(err).Error("Could not send notification")
return nil, ErrCouldNotSendNotification
}

defer func(Body io.ReadCloser) {
err := Body.Close()
if err != nil {
if err := Body.Close(); err != nil {
log.WithError(err).Error("Could not close body")
}
}(resp.Body)

var response model.IOSRemoteNotificationResponse
if err = json.NewDecoder(resp.Body).Decode(&response); err != nil && err != io.EOF {
log.Error(err)
log.WithError(err).Error("Could not decode APNs response")
return nil, ErrCouldNotDecodeAPNsResponse
}

Expand All @@ -137,15 +135,13 @@ func NewRepository(db *gorm.DB, token *ios_apns_jwt.Token) *Repository {

func NewCronRepository(db *gorm.DB) (*Repository, error) {
if err := ValidateRequirementsForIOSNotificationsService(); err != nil {
log.Warn(err)

log.WithError(err).Warn("Failed to validate requirements for ios notifications service")
return nil, err
}

token, err := ios_apns_jwt.NewToken()

if err != nil {
log.Fatal(err)
log.WithError(err).Fatal("Could not create APNs token")
}

return NewRepository(db, token), nil
Expand Down
Loading

0 comments on commit facf354

Please sign in to comment.