Clean up the code

README.md

@@ -0,0 +1,16 @@
+# Petstore Swagger Demo
+
+Tovi Jaeschke-Rogers | 2022/02/02
+
+
+## Possible improvements
+
+Below is a list of improvements I would add if this was code for a production environment
+
+- Remove orphaned files after they are deleted
+- Use UUID's for the ID's, to prevent any conflicts
+- Authentication to be added
+
+
+
+

api/petApiHandlers.go

@@ -8,10 +8,7 @@ import (
 	"log"
 	"mime/multipart"
 	"net/http"
-	"os"
-	"path/filepath"
 	"strconv"
-	"strings"
 
 	"git.tovijaeschke.xyz/tovi/JumboPetstore/database"
 	"git.tovijaeschke.xyz/tovi/JumboPetstore/models"
@@ -20,6 +17,13 @@ import (
 	"github.com/gorilla/mux"
 )
 
+func verifyStatus(status string) error {
+	if status != "available" && status != "pending" && status != "sold" {
+		return errors.New("Invalid status")
+	}
+	return nil
+}
+
 func deserialsePetJson(data []byte) (models.Pet, error) {
 	var (
 		petData                  models.Pet             = models.Pet{}
@@ -42,17 +46,28 @@ func deserialsePetJson(data []byte) (models.Pet, error) {
 	}
 
 	if len(jsonStructureTestResults.MismatchedFields) > 0 {
-		fmt.Println(jsonStructureTestResults.MismatchedFields)
-		return petData, errors.New("MismatchedFields found when deserializing data")
+		return petData, errors.New(fmt.Sprintf(
+			"MismatchedFields found when deserializing data, %s",
+			jsonStructureTestResults.Errors().Error(),
+		))
 	}
 
 	if len(jsonStructureTestResults.MissingFields) > 0 {
-		fmt.Println(jsonStructureTestResults.MissingFields)
-		return petData, errors.New("MissingFields found when deserializing data")
+		return petData, errors.New(fmt.Sprintf(
+			"MissingFields found when deserializing data, %s",
+			jsonStructureTestResults.Errors().Error(),
+		))
+
 	}
 
 	// Deserialize the JSON into the struct
 	err = json.Unmarshal(data, &petData)
+	if err != nil {
+		return petData, err
+	}
+
+	err = verifyStatus(petData.Status)
+
 	return petData, err
 }
 
@@ -102,24 +117,22 @@ func PetHandlerCreateUpdate(w http.ResponseWriter, r *http.Request) {
 
 	switch r.Method {
 	case "POST":
-		petData, err = database.CreatePet(petData)
+		err = database.CreatePet(&petData)
 		if err != nil {
-			panic(err)
+			genericJsonReturn(w, 405, "Invalid data", "unknown")
 		}
 		break
 	case "PUT":
-		petData, err = database.UpdatePet(petData)
+		err = database.UpdatePet(&petData)
 		if err != nil {
-			panic(err)
+			genericJsonReturn(w, 405, "Invalid data", "unknown")
 		}
 		break
 	}
 
 	returnJson, err = json.MarshalIndent(petData, "", "  ")
 	if err != nil {
-		log.Printf("Error encountered when creating pet record: %s\n", err.Error())
-		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte("500 - An error has occured\n"))
+		genericJsonReturn(w, 500, "An error occured", "unknown")
 		return
 	}
 
@@ -128,103 +141,122 @@ func PetHandlerCreateUpdate(w http.ResponseWriter, r *http.Request) {
 	w.Write(returnJson)
 }
 
-func PetHandlerId(w http.ResponseWriter, r *http.Request) {
+func getPetDataById(w http.ResponseWriter, r *http.Request) (models.Pet, bool) {
 	var (
-		petData      models.Pet
-		urlVars      map[string]string
-		returnJson   []byte
-		notFoundJson map[string]interface{} = make(map[string]interface{})
-		id_str       string
-		id           int
-		ok           bool
-		err          error
+		petData models.Pet
+		urlVars map[string]string
+		idStr   string
+		id      int
+		ok      bool
+		err     error
 	)
 
 	urlVars = mux.Vars(r)
-	id_str, ok = urlVars["petId"]
+	idStr, ok = urlVars["petId"]
 	if !ok {
 		log.Printf("Error encountered getting id\n")
-		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte("500 - An error has occured\n"))
-		return
+		genericJsonReturn(w, 500, "An error occured", "unknown")
+		return petData, false
 	}
 
-	id, err = strconv.Atoi(id_str)
+	id, err = strconv.Atoi(idStr)
 	if err != nil {
 		log.Printf("Error encountered converting id to string: %s\n", err.Error())
-		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte("500 - An error has occured\n"))
-		return
+		genericJsonReturn(w, 500, "An error occured", "unknown")
+		return petData, false
 	}
 
 	petData = database.GetPetById(id)
 	if petData.Id == 0 {
 		log.Printf("Could not find pet with id %d\n", id)
-		w.WriteHeader(http.StatusNotFound)
-		notFoundJson["code"] = 404
-		notFoundJson["type"] = "unknown"
-		notFoundJson["message"] = "not found"
-		returnJson, err = json.MarshalIndent(notFoundJson, "", "  ")
-		w.Write(returnJson)
+		genericJsonReturn(w, 404, "Not found", "unknown")
+		return petData, false
+	}
+
+	return petData, true
+}
+
+func GetPetHandler(w http.ResponseWriter, r *http.Request) {
+	var (
+		petData    models.Pet
+		returnJson []byte
+		ok         bool
+		err        error
+	)
+
+	petData, ok = getPetDataById(w, r)
+	if !ok {
 		return
 	}
 
-	switch r.Method {
-	case "GET":
-		returnJson, err = json.MarshalIndent(petData, "", "  ")
-		if err != nil {
-			log.Printf("Error encountered when creating pet record: %s\n", err.Error())
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write([]byte("500 - An error has occured\n"))
-			return
-		}
+	returnJson, err = json.MarshalIndent(petData, "", "  ")
+	if err != nil {
+		log.Printf("Error encountered when creating pet record: %s\n", err.Error())
+		genericJsonReturn(w, 500, "An error occured", "unknown")
+		return
+	}
 
-		// Return updated json
-		w.WriteHeader(http.StatusOK)
-		w.Write(returnJson)
-		break
-	case "POST":
-		err = r.ParseForm()
-		if err != nil {
-			log.Printf("Error encountered parsing form: %s\n", err.Error())
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write([]byte("500 - An error has occured\n"))
-			return
-		}
+	// Return updated json
+	w.WriteHeader(http.StatusOK)
+	w.Write(returnJson)
+}
 
-		if r.FormValue("name") != "" {
-			petData.Name = r.FormValue("name")
-		}
-		if r.FormValue("status") != "" {
-			petData.Status = r.FormValue("status")
-		}
+func UpdatePetHandler(w http.ResponseWriter, r *http.Request) {
+	var (
+		petData models.Pet
+		ok      bool
+		err     error
+	)
 
-		_, err = database.UpdatePet(petData)
-		if err != nil {
-			log.Printf("Error updating pet: %s\n", err.Error())
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write([]byte("500 - An error has occured\n"))
-			return
-		}
+	petData, ok = getPetDataById(w, r)
+	if !ok {
+		return
+	}
 
-		break
-	case "DELETE":
-		log.Printf("Marking pet %d as deleted\n", id)
+	err = r.ParseForm()
+	if err != nil {
+		log.Printf("Error encountered parsing form: %s\n", err.Error())
+		genericJsonReturn(w, 500, "An error occured", "unknown")
+		return
+	}
+
+	if r.FormValue("name") != "" {
+		petData.Name = r.FormValue("name")
+	}
+	if r.FormValue("status") != "" {
+		petData.Status = r.FormValue("status")
+	}
+
+	err = database.UpdatePet(&petData)
+	if err != nil {
+		log.Printf("Error updating pet: %s\n", err.Error())
+		genericJsonReturn(w, 500, "An error occured", "unknown")
+		return
+	}
 
-		database.DeletePet(petData)
+}
 
-		w.WriteHeader(http.StatusOK)
-		notFoundJson["code"] = 200
-		notFoundJson["type"] = "unknown"
-		notFoundJson["message"] = id_str
-		returnJson, err = json.MarshalIndent(notFoundJson, "", "  ")
-		w.Write(returnJson)
+func DeletePetHandler(w http.ResponseWriter, r *http.Request) {
+	var (
+		petData models.Pet
+		id_str  string
+		ok      bool
+	)
 
-		break
+	petData, ok = getPetDataById(w, r)
+	if !ok {
+		return
 	}
+
+	log.Printf("Marking pet %d as deleted\n", petData.Id)
+
+	database.DeletePet(petData)
+
+	id_str = strconv.Itoa(petData.Id)
+	genericJsonReturn(w, 200, id_str, "unknown")
 }
 
-func PetHandlerFindByStatus(w http.ResponseWriter, r *http.Request) {
+func FindByStatusPetHandler(w http.ResponseWriter, r *http.Request) {
 	var (
 		status     string
 		petData    []models.Pet
@@ -233,26 +265,24 @@ func PetHandlerFindByStatus(w http.ResponseWriter, r *http.Request) {
 	)
 
 	status = r.URL.Query().Get("status")
-	if status != "available" && status != "pending" && status != "sold" {
+	err = verifyStatus(status)
+	if err != nil {
 		log.Printf("Invalid status provided to /pet/findByStatus")
-		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte("500 - An error has occured\n"))
+		genericJsonReturn(w, 422, "Invalid status", "unknown")
 		return
 	}
 
 	petData, err = database.GetPetsByStatus(status)
 	if err != nil {
 		log.Printf("An error occured in GetPetsByStatus")
-		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte("500 - An error has occured\n"))
+		genericJsonReturn(w, 500, "An error occured", "unknown")
 		return
 	}
 
 	returnJson, err = json.MarshalIndent(petData, "", "  ")
 	if err != nil {
 		log.Printf("An error occured while serializing pet data to JSON")
-		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte("500 - An error has occured\n"))
+		genericJsonReturn(w, 500, "An error occured", "unknown")
 		return
 	}
 
@@ -260,72 +290,61 @@ func PetHandlerFindByStatus(w http.ResponseWriter, r *http.Request) {
 
 }
 
-func PetHandlerUploadImage(w http.ResponseWriter, r *http.Request) {
+func UploadImagePetHandler(w http.ResponseWriter, r *http.Request) {
 	var (
-		urlVars   map[string]string
-		file      multipart.File
-		handler   *multipart.FileHeader
-		tempFile  *os.File
-		nameSplit []string
-		fileBytes []byte
-		id_str    string
-		id        int
-		ok        bool
-		err       error
+		petData            models.Pet
+		additionalMetadata string
+		file               multipart.File
+		handler            *multipart.FileHeader
+		fileBytes          []byte
+		fileName           string
+		ok                 bool
+		err                error
 	)
 
-	urlVars = mux.Vars(r)
-	id_str, ok = urlVars["petId"]
+	petData, ok = getPetDataById(w, r)
 	if !ok {
-		log.Printf("Error encountered getting id\n")
-		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte("500 - An error has occured\n"))
 		return
 	}
 
-	id, err = strconv.Atoi(id_str)
-	if err != nil {
-		log.Printf("Error encountered converting id to string: %s\n", err.Error())
-		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte("500 - An error has occured\n"))
-		return
-	}
-
-	fmt.Println(id)
-
 	// Upload 10Mb files
 	r.ParseMultipartForm(10 << 20)
 	file, handler, err = r.FormFile("file")
 	if err != nil {
 		log.Printf("Error retrieving file: %s\n", err.Error())
-		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte("500 - An error has occured\n"))
+		genericJsonReturn(w, 500, "An error occured", "unknown")
 		return
 	}
 	defer file.Close()
 
-	nameSplit = strings.Split(handler.Filename, ".")
-
-	tempFile, err = ioutil.TempFile("./uploads", "upload-*."+nameSplit[len(nameSplit)-1])
-	if err != nil {
-		fmt.Println(err)
-		return
+	additionalMetadata = r.FormValue("additionalMetadata")
+	if additionalMetadata == "" {
+		additionalMetadata = "null"
 	}
-	defer tempFile.Close()
 
 	fileBytes, err = ioutil.ReadAll(file)
 	if err != nil {
-		fmt.Println(err)
+		log.Printf("An error occured reading POST file: %s\n", err.Error())
+		genericJsonReturn(w, 500, "An error occured", "unknown")
 		return
 	}
 
-	tempFile.Write(fileBytes)
-
-	err = database.AddPhotoToPet(id, filepath.Base(tempFile.Name()))
+	fileName, err = database.AddPhotoToPet(&petData, handler.Filename, additionalMetadata, fileBytes)
 	if err != nil {
-		fmt.Println(err)
+		log.Printf("An error occured adding file to database: %s\n", err.Error())
+		genericJsonReturn(w, 500, "An error occured", "unknown")
 		return
 	}
 
-	fmt.Fprintf(w, "Successfully Uploaded File\n")
+	genericJsonReturn(
+		w,
+		200,
+		fmt.Sprintf(
+			"additionalMetadata: %s\nFile uploaded to %s, %d bytes",
+			additionalMetadata,
+			fileName,
+			handler.Size,
+		),
+		"unknown",
+	)
 }

api/routes.go

@@ -15,11 +15,13 @@ func InitApiEndpoints() *mux.Router {
 
 	// Define routes for pet api
 	router.HandleFunc("/pet", PetHandlerCreateUpdate).Methods("POST", "PUT")
-	router.HandleFunc("/pet/findByStatus", PetHandlerFindByStatus).Methods("GET")
-	router.HandleFunc("/pet/{petId}", PetHandlerId).Methods("GET", "POST", "DELETE")
-	router.HandleFunc("/pet/{petId}/uploadImage", PetHandlerUploadImage).Methods("POST")
+	router.HandleFunc("/pet/findByStatus", FindByStatusPetHandler).Methods("GET")
+	router.HandleFunc("/pet/{petId}", GetPetHandler).Methods("GET")
+	router.HandleFunc("/pet/{petId}", UpdatePetHandler).Methods("POST")
+	router.HandleFunc("/pet/{petId}", DeletePetHandler).Methods("DELETE")
+	router.HandleFunc("/pet/{petId}/uploadImage", UploadImagePetHandler).Methods("POST")
 
-	router.PathPrefix("/images/").Handler(http.FileServer(http.Dir("./uploads/")))
+	router.PathPrefix("/").Handler(http.StripPrefix("/images/", http.FileServer(http.Dir("./uploads"))))
 
 	return router
 }

database/pet.go

@@ -5,76 +5,89 @@ import (
 
 	"git.tovijaeschke.xyz/tovi/JumboPetstore/models"
 	"git.tovijaeschke.xyz/tovi/JumboPetstore/util"
+
+	"gorm.io/gorm"
 	"gorm.io/gorm/clause"
 )
 
-func CreatePet(petData models.Pet) (models.Pet, error) {
+func CreatePet(petData *models.Pet) error {
 	var (
-		photoUrl string
-		fileName string
-		err      error
+		photoUrls []string
+		photoUrl  string
+		fileName  string
+		err       error
 	)
 
-	err = DB.Omit("PhotoUrlJson").Create(&petData).Error
-	if err != nil {
-		return petData, err
-	}
-
 	for _, photoUrl = range petData.PhotoUrlJson {
 		fileName, err = util.DownloadFile(photoUrl)
 		if err != nil {
-			return petData, err
+			return err
 		}
 
 		petData.PhotoUrls = append(petData.PhotoUrls, models.PetPhoto{
 			PetId:    petData.Id,
 			FileName: fileName,
 		})
+
+		photoUrls = append(photoUrls, fmt.Sprintf(
+			"/images/%s",
+			fileName,
+		))
 	}
 
-	petData, err = UpdatePet(petData)
+	petData.PhotoUrlJson = photoUrls
+
+	err = DB.Session(&gorm.Session{FullSaveAssociations: true}).
+		Omit("PhotoUrlJson").
+		Create(petData).
+		Error
 	if err != nil {
-		return petData, err
+		return err
 	}
 
-	return petData, err
+	return err
 }
 
-func UpdatePet(petData models.Pet) (models.Pet, error) {
+func UpdatePet(petData *models.Pet) error {
 	var (
-		photoUrl string
-		err      error
+		photoUrls []string
+		photoUrl  string
+		fileName  string
+		err       error
 	)
 
-	err = DB.Model(&models.Pet{}).
-		Where("id = ?", petData.Id).
-		Omit("PhotoUrlJson").
-		Updates(&petData).
-		Error
-	if err != nil {
-		return petData, err
+	for _, photoUrl = range petData.PhotoUrlJson {
+		fileName, err = util.DownloadFile(photoUrl)
+		if err != nil {
+			return err
+		}
+
+		petData.PhotoUrls = append(petData.PhotoUrls, models.PetPhoto{
+			PetId:    petData.Id,
+			FileName: fileName,
+		})
+
+		photoUrls = append(photoUrls, fmt.Sprintf(
+			"/images/%s",
+			fileName,
+		))
 	}
 
-	// Delete existing pet photos
-	err = DB.Where("pet_id = ?", petData.ID).
-		Delete(&models.PetPhoto{}).
+	petData.PhotoUrlJson = photoUrls
+
+	// This leaves some orphaned files in ./uploads
+	DB.Model(petData).Association("PhotoUrls").Replace(petData.PhotoUrls)
+	DB.Model(petData).Association("Tags").Replace(petData.Tags)
+
+	err = DB.Session(&gorm.Session{FullSaveAssociations: true}).
+		Omit("PhotoUrlJson").
+		Updates(petData).
 		Error
 	if err != nil {
-		return petData, err
+		return err
 	}
 
-	// Update pet photos from PUT data
-	for _, photoUrl = range petData.PhotoUrlJson {
-		err = DB.Create(&models.PetPhoto{
-			PetId: petData.Id,
-			Url:   photoUrl,
-		}).Error
-		if err != nil {
-			return petData, err
-		}
-	}
-
-	return petData, err
+	return err
 }
 
 func GetPetById(id int) models.Pet {
@@ -92,43 +105,32 @@ func GetPetById(id int) models.Pet {
 
 	petData.PhotoUrlJson = photoUrls
 
-	fmt.Println(petData)
-
 	return petData
 }
 
 func DeletePet(petData models.Pet) {
-	DB.Where("pet_id = ?", petData.Id).Delete(&petData.Categories)
-	DB.Where("pet_id = ?", petData.Id).Delete(&petData.Tags)
-	DB.Where("pet_id = ?", petData.Id).Delete(&petData.PhotoUrls)
-	DB.Delete(&petData)
+	DB.Preload(clause.Associations).Delete(&petData)
 }
 
 func GetPetsByStatus(status string) ([]models.Pet, error) {
 	var (
 		petDatas  []models.Pet
-		petData   models.Pet
 		petPhoto  models.PetPhoto
 		photoUrls []string
 		i         int
 		err       error
 	)
 
-	err = DB.Where("status = ?", status).Find(&petDatas).Error
+	err = DB.Preload(clause.Associations).
+		Where("status = ?", status).
+		Find(&petDatas).
+		Error
 	if err != nil {
 		return petDatas, err
 	}
 
-	for i, petData = range petDatas {
-		err = DB.Where("pet_id = ?", petData.Id).Find(&petDatas[i].Tags).Error
-		if err != nil {
-			return petDatas, err
-		}
-
-		err = DB.Where("pet_id = ?", petData.Id).Find(&petDatas[i].PhotoUrls).Error
-		if err != nil {
-			return petDatas, err
-		}
+	for i = range petDatas {
+		photoUrls = []string{}
 
 		for _, petPhoto = range petDatas[i].PhotoUrls {
 			photoUrls = append(photoUrls, "/images/"+petPhoto.FileName)
@@ -140,19 +142,23 @@ func GetPetsByStatus(status string) ([]models.Pet, error) {
 	return petDatas, err
 }
 
-func AddPhotoToPet(id int, fileName string) error {
+func AddPhotoToPet(petData *models.Pet, fileName string, additionalMetadata string, fileBytes []byte) (string, error) {
 	var (
-		petData models.Pet
-		err     error
+		err error
 	)
 
-	petData = GetPetById(id)
+	fileName, err = util.WriteFile(fileName, fileBytes)
 
 	petData.PhotoUrls = append(petData.PhotoUrls, models.PetPhoto{
-		PetId:    id,
-		FileName: fileName,
+		PetId:              petData.Id,
+		FileName:           fileName,
+		AdditionalMetadata: additionalMetadata,
 	})
-	_, err = UpdatePet(petData)
 
-	return err
+	err = DB.Session(&gorm.Session{FullSaveAssociations: true}).
+		Omit("PhotoUrlJson").
+		Updates(&petData).
+		Error
+
+	return fileName, err
 }

models/pet.go

@@ -17,11 +17,12 @@ type PetTag struct {
 }
 
 type PetPhoto struct {
-	gorm.Model `json:"-"`
-	Id         int    `gorm:"primary_key"`
-	PetId      int    `json:"-"`
-	Url        string `gorm:"-"`
-	FileName   string `json:"-"`
+	gorm.Model         `json:"-"`
+	Id                 int    `gorm:"primary_key"`
+	PetId              int    `json:"-"`
+	Url                string `gorm:"-"`
+	FileName           string `json:"-"`
+	AdditionalMetadata string `json:"-"`
 }
 
 type Pet struct {

util/downloadFile.go

@@ -9,30 +9,42 @@ import (
 	"strings"
 )
 
-func DownloadFile(url string) (string, error) {
+func WriteFile(fileName string, fileBytes []byte) (string, error) {
 	var (
 		nameSplit []string
-		resp      *http.Response
 		tempFile  *os.File
 		err       error
 	)
 
 	// Used to get the file extension
 	// This could be improved with a mime-type lookup
-	nameSplit = strings.Split(url, ".")
+	nameSplit = strings.Split(fileName, ".")
 
-	resp, err = http.Get(url)
+	tempFile, err = ioutil.TempFile("./uploads", "upload-*."+nameSplit[len(nameSplit)-1])
 	if err != nil {
 		return "", err
 	}
-	defer resp.Body.Close()
+	defer tempFile.Close()
 
-	tempFile, err = ioutil.TempFile("./uploads", "upload-*."+nameSplit[len(nameSplit)-1])
+	_, err = tempFile.Write(fileBytes)
+
+	return filepath.Base(tempFile.Name()), err
+}
+
+func DownloadFile(url string) (string, error) {
+	var (
+		resp      *http.Response
+		fileBytes []byte
+		err       error
+	)
+
+	resp, err = http.Get(url)
 	if err != nil {
 		return "", err
 	}
-	defer tempFile.Close()
+	defer resp.Body.Close()
 
-	_, err = io.Copy(tempFile, resp.Body)
-	return filepath.Base(tempFile.Name()), err
+	fileBytes, err = io.ReadAll(resp.Body)
+
+	return WriteFile(url, fileBytes)
 }