Improve comments and code readability

doc/api.yaml

@@ -210,7 +210,7 @@ paths:
                 $ref: "#/components/schemas/generic_response"
               example:
                 status: "Resource not found"
-        '400': # todo: not sure if this is the right error code
+        '400':
           description: Trying to follow a user that does not exist.
           content:
             application/json:

service/api/authorization/auth-anonymous.go

@@ -7,9 +7,11 @@ import (
 	"github.com/notherealmarco/WASAPhoto/service/database"
 )
 
+// AnonymousAuth is the authentication provider for non logged-in users
 type AnonymousAuth struct {
 }
 
+// Returns a newly created AnonymousAuth instance
 func BuildAnonymous() *AnonymousAuth {
 	return &AnonymousAuth{}
 }
@@ -18,14 +20,17 @@ func (u *AnonymousAuth) GetType() string {
 	return "Anonymous"
 }
 
+// Returns UNAUTHORIZED, as anonymous users are logged in
 func (u *AnonymousAuth) Authorized(db database.AppDatabase) (reqcontext.AuthStatus, error) {
 	return reqcontext.UNAUTHORIZED, nil
 }
 
+// Returns UNAUTHORIZED, as anonymous users are not logged in
 func (u *AnonymousAuth) UserAuthorized(db database.AppDatabase, uid string) (reqcontext.AuthStatus, error) {
 	return reqcontext.UNAUTHORIZED, nil
 }
 
+// Returns an empty string, as anonymous users have no user ID
 func (u *AnonymousAuth) GetUserID() string {
 	return ""
 }

service/api/authorization/auth-bearer.go

@@ -8,6 +8,8 @@ import (
 	"github.com/notherealmarco/WASAPhoto/service/database"
 )
 
+// BearerAuth is the authentication provider that authorizes users by Bearer tokens
+// In this case, a token is the unique identifier for a user.
 type BearerAuth struct {
 	token string
 }
@@ -16,6 +18,8 @@ func (b *BearerAuth) GetType() string {
 	return "Bearer"
 }
 
+// Given the content of the Authorization header, returns a BearerAuth instance for the user
+// Returns an error if the header is not valid
 func BuildBearer(header string) (*BearerAuth, error) {
 	if header == "" {
 		return nil, errors.New("missing authorization header")
@@ -29,10 +33,12 @@ func BuildBearer(header string) (*BearerAuth, error) {
 	return &BearerAuth{token: header[7:]}, nil
 }
 
+// Returns the user ID of the user that is currently logged in
 func (b *BearerAuth) GetUserID() string {
 	return b.token
 }
 
+// Checks if the token is valid
 func (b *BearerAuth) Authorized(db database.AppDatabase) (reqcontext.AuthStatus, error) {
 	// this is the way we manage authorization, the bearer token is the user id
 	state, err := db.UserExists(b.token)
@@ -47,6 +53,7 @@ func (b *BearerAuth) Authorized(db database.AppDatabase) (reqcontext.AuthStatus,
 	return reqcontext.UNAUTHORIZED, nil
 }
 
+// Checks if the given user and the currently logged in user are the same user
 func (b *BearerAuth) UserAuthorized(db database.AppDatabase, uid string) (reqcontext.AuthStatus, error) {
 
 	// If uid is not a valid user, return USER_NOT_FOUND
@@ -60,6 +67,7 @@ func (b *BearerAuth) UserAuthorized(db database.AppDatabase, uid string) (reqcon
 	}
 
 	if b.token == uid {
+		// If the user is the same as the one in the token, check if the user does actually exist in the database
 		auth, err := b.Authorized(db)
 
 		if err != nil {
@@ -68,5 +76,6 @@ func (b *BearerAuth) UserAuthorized(db database.AppDatabase, uid string) (reqcon
 
 		return auth, nil
 	}
+	// If the user is not the same as the one in the token, return FORBIDDEN
 	return reqcontext.FORBIDDEN, nil
 }

service/api/authorization/auth-manager.go

@@ -10,6 +10,7 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// BuildAuth returns an Authorization implementation for the currently logged in user
 func BuildAuth(header string) (reqcontext.Authorization, error) {
 	auth, err := BuildBearer(header)
 	if err != nil {
@@ -21,6 +22,8 @@ func BuildAuth(header string) (reqcontext.Authorization, error) {
 	return auth, nil
 }
 
+// Given a user authorization function, if the function returns some error, it sends the error to the client and return false
+// Otherwise it returns true without sending anything to the client
 func SendAuthorizationError(f func(db database.AppDatabase, uid string) (reqcontext.AuthStatus, error), uid string, db database.AppDatabase, w http.ResponseWriter, l logrus.FieldLogger, notFoundStatus int) bool {
 	auth, err := f(db, uid)
 	if err != nil {
@@ -28,21 +31,25 @@ func SendAuthorizationError(f func(db database.AppDatabase, uid string) (reqcont
 		return false
 	}
 	if auth == reqcontext.UNAUTHORIZED {
+		// The token is not valid
 		helpers.SendStatus(http.StatusUnauthorized, w, "Unauthorized", l)
 		return false
 	}
 	if auth == reqcontext.FORBIDDEN {
+		// The user is not authorized for this action
 		helpers.SendStatus(http.StatusForbidden, w, "Forbidden", l)
 		return false
 	}
-	// requested user is not found -> 404 as the resource is not found
 	if auth == reqcontext.USER_NOT_FOUND {
+		// Attempting to perform an action on a non-existent user
 		helpers.SendStatus(notFoundStatus, w, "User not found", l)
 		return false
 	}
 	return true
 }
 
+// Given a function that validates a token, if the function returns some error, it sends the error to the client and return false
+// Otherwise it returns true without sending anything to the client
 func SendErrorIfNotLoggedIn(f func(db database.AppDatabase) (reqcontext.AuthStatus, error), db database.AppDatabase, w http.ResponseWriter, l logrus.FieldLogger) bool {
 
 	auth, err := f(db)
@@ -53,6 +60,7 @@ func SendErrorIfNotLoggedIn(f func(db database.AppDatabase) (reqcontext.AuthStat
 	}
 
 	if auth == reqcontext.UNAUTHORIZED {
+		// The token is not valid
 		helpers.SendStatus(http.StatusUnauthorized, w, "Unauthorized", l)
 		return false
 	}

service/api/bans.go

@@ -66,6 +66,7 @@ func (rt *_router) PutBan(w http.ResponseWriter, r *http.Request, ps httprouter.
 		return
 	}
 
+	// Execute the query
 	status, err := rt.db.BanUser(uid, banned)
 
 	if err != nil {
@@ -95,6 +96,7 @@ func (rt *_router) DeleteBan(w http.ResponseWriter, r *http.Request, ps httprout
 		return
 	}
 
+	// Execute the query
 	status, err := rt.db.UnbanUser(uid, banned)
 
 	if err != nil {

service/api/comments.go

@@ -55,6 +55,7 @@ func (rt *_router) GetComments(w http.ResponseWriter, r *http.Request, ps httpro
 	}
 
 	// send the response
+	w.Header().Set("Content-Type", "application/json")
 	err = json.NewEncoder(w).Encode(comments)
 
 	if err != nil {

service/api/followers.go

@@ -79,6 +79,7 @@ func (rt *_router) PutFollow(w http.ResponseWriter, r *http.Request, ps httprout
 		return
 	}
 
+	// Execute the query
 	status, err := rt.db.FollowUser(follower, uid)
 
 	if err != nil {
@@ -109,6 +110,7 @@ func (rt *_router) DeleteFollow(w http.ResponseWriter, r *http.Request, ps httpr
 		return
 	}
 
+	// Execute the query
 	status, err := rt.db.UnfollowUser(follower, uid)
 
 	if err != nil {

service/api/helpers/api-helpers.go

@@ -10,6 +10,8 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// Tries to decode a json, if it fails, it returns Bad Request to the client and the function returns false
+// Otherwise it returns true without sending anything to the client
 func DecodeJsonOrBadRequest(r io.Reader, w http.ResponseWriter, v interface{}, l logrus.FieldLogger) bool {
 
 	err := json.NewDecoder(r).Decode(v)
@@ -20,6 +22,8 @@ func DecodeJsonOrBadRequest(r io.Reader, w http.ResponseWriter, v interface{}, l
 	return true
 }
 
+// Verifies if a user exists, if it doesn't, it returns Not Found to the client and the function returns false
+// Otherwise it returns true without sending anything to the client
 func VerifyUserOrNotFound(db database.AppDatabase, uid string, w http.ResponseWriter, l logrus.FieldLogger) bool {
 
 	user_exists, err := db.UserExists(uid)
@@ -36,6 +40,8 @@ func VerifyUserOrNotFound(db database.AppDatabase, uid string, w http.ResponseWr
 	return true
 }
 
+// Sends a generic status response
+// The response is a json object with a "status" field desribing the status of a request
 func SendStatus(httpStatus int, w http.ResponseWriter, description string, l logrus.FieldLogger) {
 	w.WriteHeader(httpStatus)
 	err := json.NewEncoder(w).Encode(structures.GenericResponse{Status: description})
@@ -44,40 +50,29 @@ func SendStatus(httpStatus int, w http.ResponseWriter, description string, l log
 	}
 }
 
+// Sends a Not Found error to the client
 func SendNotFound(w http.ResponseWriter, description string, l logrus.FieldLogger) {
-	w.WriteHeader(http.StatusNotFound)
-	err := json.NewEncoder(w).Encode(structures.GenericResponse{Status: description})
-	if err != nil {
-		l.WithError(err).Error("Error encoding json")
-	}
+	SendStatus(http.StatusNotFound, w, description, l)
 }
 
+// Sends a Bad Request error to the client
 func SendBadRequest(w http.ResponseWriter, description string, l logrus.FieldLogger) {
-	w.WriteHeader(http.StatusBadRequest)
-	err := json.NewEncoder(w).Encode(structures.GenericResponse{Status: description})
-	if err != nil {
-		l.WithError(err).Error("Error encoding json")
-	}
+	SendStatus(http.StatusBadRequest, w, description, l)
 }
 
+// Sends a Bad Request error to the client and logs the given error
 func SendBadRequestError(err error, description string, w http.ResponseWriter, l logrus.FieldLogger) {
-	w.WriteHeader(http.StatusBadRequest)
 	l.WithError(err).Error(description)
-	err = json.NewEncoder(w).Encode(structures.GenericResponse{Status: description})
-	if err != nil {
-		l.WithError(err).Error("Error encoding json")
-	}
+	SendBadRequest(w, description, l)
 }
 
+// Sends an Internal Server Error to the client and logs the given error
 func SendInternalError(err error, description string, w http.ResponseWriter, l logrus.FieldLogger) {
-	w.WriteHeader(http.StatusInternalServerError)
 	l.WithError(err).Error(description)
-	err = json.NewEncoder(w).Encode(structures.GenericResponse{Status: description})
-	if err != nil {
-		l.WithError(err).Error("Error encoding json")
-	}
+	SendStatus(http.StatusInternalServerError, w, description, l)
 }
 
+// Tries to roll back a transaction, if it fails it logs the error
 func RollbackOrLogError(tx database.DBTransaction, l logrus.FieldLogger) {
 	err := tx.Rollback()
 	if err != nil {
@@ -85,6 +80,8 @@ func RollbackOrLogError(tx database.DBTransaction, l logrus.FieldLogger) {
 	}
 }
 
+// Checks if a user is banned by another user, then it returns Not Found to the client and the function returns false
+// Otherwise it returns true whithout sending anything to the client
 func SendNotFoundIfBanned(db database.AppDatabase, uid string, banner string, w http.ResponseWriter, l logrus.FieldLogger) bool {
 	banned, err := db.IsBanned(uid, banner)
 	if err != nil {

service/api/helpers/get-limits.go

@@ -6,10 +6,12 @@ import (
 )
 
 const (
-	DEFAULT_LIMIT  = 15
+	DEFAULT_LIMIT  = 30
 	DEFAULT_OFFSET = 0
 )
 
+// Get the start index and limit from the query.
+// If they are not present, use the default values.
 func GetLimits(query url.Values) (int, int, error) {
 
 	limit := DEFAULT_LIMIT

service/api/helpers/regex-helpers.go

@@ -7,6 +7,8 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// Given a string, a regex and an error description, if the string doesn't match the regex, it sends a bad request error to the client and return false
+// Otherwise it returns true without sending anything to the client
 func MatchRegexOrBadRequest(str string, regex string, error_description string, w http.ResponseWriter, l logrus.FieldLogger) bool {
 
 	stat, err := regexp.Match(regex, []byte(str))
@@ -25,6 +27,7 @@ func MatchRegexOrBadRequest(str string, regex string, error_description string,
 	return true
 }
 
+// Validates a username (must be between 3 and 16 characters long and can only contain letters, numbers and underscores)
 func MatchUsernameOrBadRequest(username string, w http.ResponseWriter, l logrus.FieldLogger) bool {
 	return MatchRegexOrBadRequest(username,
 		`^[a-zA-Z0-9_]{3,16}$`, "Username must be between 3 and 16 characters long and can only contain letters, numbers and underscores",
@@ -32,6 +35,7 @@ func MatchUsernameOrBadRequest(username string, w http.ResponseWriter, l logrus.
 		l)
 }
 
+// Validates a comment (must be between 1 and 255 characters long)
 func MatchCommentOrBadRequest(comment string, w http.ResponseWriter, l logrus.FieldLogger) bool {
 	return MatchRegexOrBadRequest(comment,
 		`^(.){1,255}$`, "Comment must be between 1 and 255 characters long",

service/api/liveness.go

@@ -1,16 +1,18 @@
 package api
 
 import (
-	"github.com/julienschmidt/httprouter"
 	"net/http"
+
+	"github.com/julienschmidt/httprouter"
+	"github.com/notherealmarco/WASAPhoto/service/api/helpers"
 )
 
 // liveness is an HTTP handler that checks the API server status. If the server cannot serve requests (e.g., some
 // resources are not ready), this should reply with HTTP Status 500. Otherwise, with HTTP Status 200
 func (rt *_router) liveness(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
-	/* Example of liveness check:
-	if err := rt.DB.Ping(); err != nil {
+	if err := rt.db.Ping(); err != nil {
 		w.WriteHeader(http.StatusInternalServerError)
 		return
-	}*/
+	}
+	helpers.SendStatus(200, w, "Server is live!", rt.baseLogger)
 }

service/api/photos.go

@@ -33,7 +33,6 @@ func (rt *_router) PostPhoto(w http.ResponseWriter, r *http.Request, ps httprout
 	}
 
 	path := rt.dataPath + "/photos/" + uid + "/" + strconv.FormatInt(photo_id, 10) + ".jpg"
-	// todo: we should check if the body is a valid jpg image
 
 	if err = os.MkdirAll(filepath.Dir(path), os.ModePerm); err != nil { // perms = 511
 		helpers.SendInternalError(err, "Error creating directory", w, rt.baseLogger)
@@ -78,7 +77,6 @@ func (rt *_router) PostPhoto(w http.ResponseWriter, r *http.Request, ps httprout
 
 	if err != nil {
 		helpers.SendInternalError(err, "Error committing transaction", w, rt.baseLogger)
-		//todo: should I roll back?
 		return
 	}
 
@@ -156,7 +154,7 @@ func (rt *_router) DeletePhoto(w http.ResponseWriter, r *http.Request, ps httpro
 	if err != nil {
 		helpers.SendInternalError(err, "Error deleting photo from database", w, rt.baseLogger)
 		return
-	} // todo: maybe let's use a transaction also here
+	}
 
 	if !deleted {
 		helpers.SendNotFound(w, "Photo not found", rt.baseLogger)

service/api/reqcontext/context-auth.go

@@ -11,9 +11,17 @@ const (
 	USER_NOT_FOUND = 3
 )
 
+// Authorization is the interface for an authorization provider
 type Authorization interface {
+	// Returns the type of the authorization provider
 	GetType() string
+
+	// Returns the ID of the currently logged in user
 	GetUserID() string
+
+	// Checks if the token is valid
 	Authorized(db database.AppDatabase) (AuthStatus, error)
+
+	// Checks if the given user and the currently logged in user are the same user
 	UserAuthorized(db database.AppDatabase, uid string) (AuthStatus, error)
 }

service/database/database.go

@@ -79,8 +79,11 @@ type AppDatabase interface {
 	Ping() error
 }
 
+// DBTransaction is the interface for a generic database transaction
 type DBTransaction interface {
+	// Commit commits the transaction
 	Commit() error
+	// Rollback rolls back the transaction
 	Rollback() error
 }
 
@@ -95,16 +98,17 @@ func New(db *sql.DB) (AppDatabase, error) {
 		return nil, errors.New("database is required when building a AppDatabase")
 	}
 
-	// Check if tables exist. If not, the database is empty, and we need to create the structure
+	// Check if some table exists. If not, the database is empty, and we need to create the structure
 	var tableName string
-	//todo: check for all the tables, not just users
+
 	err := db.QueryRow(`SELECT name FROM sqlite_master WHERE type='table' AND name='users';`).Scan(&tableName)
 	if errors.Is(err, sql.ErrNoRows) {
+		// Database is empty, let's create the structure
 		sqlStmt := `CREATE TABLE "users" (
 			"uid"	TEXT NOT NULL,
 			"name"	TEXT NOT NULL UNIQUE,
 			PRIMARY KEY("uid")
-		)` // todo: one query is enough! We are we doing a query per table?
+		)`
 		_, err = db.Exec(sqlStmt)
 		if err != nil {
 			return nil, fmt.Errorf("error creating database structure: %w", err)

service/database/db-likes.go

@@ -96,6 +96,7 @@ func (db *appdbimpl) UnlikePhoto(uid string, photo int64, liker_uid string) (Que
 	// But our DB implementation only requires the photo id.
 	exists, err := db.photoExists(uid, photo)
 	if err != nil || !exists {
+		// The photo does not exist, or the user has been banned
 		return ERR_NOT_FOUND, err
 	}
 
@@ -111,6 +112,7 @@ func (db *appdbimpl) UnlikePhoto(uid string, photo int64, liker_uid string) (Que
 		return ERR_INTERNAL, err
 	}
 
+	// The user was not liking the photo
 	if rows == 0 {
 		return ERR_NOT_FOUND, nil
 	}

service/database/db-photos.go

@@ -18,7 +18,6 @@ func (db *appdbimpl) PostPhoto(uid string) (DBTransaction, int64, error) {
 		err_rb := tx.Rollback()
 		// If rollback fails, we return the original error plus the rollback error
 		if err_rb != nil {
-			// todo: we are losing track of err_rb here
 			err = fmt.Errorf("Rollback error. Rollback cause: %w", err)
 		}
 
@@ -30,7 +29,6 @@ func (db *appdbimpl) PostPhoto(uid string) (DBTransaction, int64, error) {
 		err_rb := tx.Rollback()
 		// If rollback fails, we return the original error plus the rollback error
 		if err_rb != nil {
-			// todo: we are losing track of err_rb here
 			err = fmt.Errorf("Rollback error. Rollback cause: %w", err)
 		}
 
@@ -66,6 +64,7 @@ func (db *appdbimpl) photoExists(uid string, photo int64) (bool, error) {
 	return cnt > 0, nil
 }
 
+// Check if a given photo owned by a given user exists, and the requesting user is not banned by the author
 func (db *appdbimpl) PhotoExists(uid string, photo int64, requesting_uid string) (bool, error) {
 
 	var cnt int64

service/database/db-transactions.go

@@ -2,14 +2,17 @@ package database
 
 import "database/sql"
 
+// dbtransaction is a struct to represent an SQL transaction, it implements the DBTransaction interface
 type dbtransaction struct {
 	c *sql.Tx
 }
 
 func (tx *dbtransaction) Commit() error {
+	// Commit the SQL transaction
 	return tx.c.Commit()
 }
 
 func (tx *dbtransaction) Rollback() error {
+	// Rollback the SQL transaction
 	return tx.c.Rollback()
 }

service/database/db_errors/db-errors.go

@@ -2,7 +2,7 @@ package db_errors
 
 import "strings"
 
-// Returns true if the query result has no rows
+// Returns true if the error is a "no rows in result set" error
 func EmptySet(err error) bool {
 	if err == nil {
 		return false
@@ -10,6 +10,7 @@ func EmptySet(err error) bool {
 	return strings.Contains(err.Error(), "no rows in result set")
 }
 
+// Returns true if the error is a Unique constraint violation error
 func UniqueViolation(err error) bool {
 	if err == nil {
 		return false
@@ -17,6 +18,7 @@ func UniqueViolation(err error) bool {
 	return strings.Contains(err.Error(), "UNIQUE constraint failed")
 }
 
+// Returns true if the error is a Foreign Key constraint violation error
 func ForeignKeyViolation(err error) bool {
 	if err == nil {
 		return false

service/database/set-name.go

@@ -1,7 +0,0 @@
-package database
-
-// SetName is an example that shows you how to execute insert/update
-func (db *appdbimpl) SetName(name string) error {
-	_, err := db.c.Exec("INSERT INTO example_table (id, name) VALUES (1, ?)", name)
-	return err
-}