From a09a0df536ae151b47e5877d66b7b5de6aebdab8 Mon Sep 17 00:00:00 2001 From: Tovi Jaeschke-Rogers Date: Wed, 2 Feb 2022 12:57:21 +1030 Subject: [PATCH] Clean up the code --- README.md | 16 +++ api/petApiHandlers.go | 281 ++++++++++++++++++++++-------------------- api/routes.go | 10 +- database/pet.go | 136 ++++++++++---------- models/pet.go | 11 +- util/downloadFile.go | 30 +++-- 6 files changed, 270 insertions(+), 214 deletions(-) create mode 100644 README.md diff --git a/README.md b/README.md new file mode 100644 index 0000000..bbd098d --- /dev/null +++ b/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 + + + + diff --git a/api/petApiHandlers.go b/api/petApiHandlers.go index cf852bf..4eb308f 100644 --- a/api/petApiHandlers.go +++ b/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", + ) } diff --git a/api/routes.go b/api/routes.go index 8721c27..82b7d70 100644 --- a/api/routes.go +++ b/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 } diff --git a/database/pet.go b/database/pet.go index e027e43..75e640c 100644 --- a/database/pet.go +++ b/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 } diff --git a/models/pet.go b/models/pet.go index f6e8740..bc4e94f 100644 --- a/models/pet.go +++ b/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 { diff --git a/util/downloadFile.go b/util/downloadFile.go index 1554c36..2abffad 100644 --- a/util/downloadFile.go +++ b/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) }