I'm afraid your code is incorrect. It is not going to behave the way you think it does.
if (ModelState.IsValid)
{
foreach (var file in Picture1)
{
//This condition is incorrect. If file == null then it will crash
//because file.ContentLength will reference memory
//As such file is never null and likely can be removed but to be safe use this
// if ((file?.ContentLength ?? 0) > 0)
if (file != null || file.ContentLength > 0)
{
string ext = System.IO.Path.GetExtension(file.FileName);
//This code is not going to work because case matters in
//string comparisons, you need to do a case insensitive comparison
// if (String.Equals(ext, ".png", StringComparison.OrdinalIgnoreCase) || ...
//Perhaps a better approach is to move the list of supported extensions into
//a list and then just do a case-insensitive lookup for it in the list
if (ext == ".png" || ext == ".jpg" || ext == ".jpeg")
{
//You aren't saving the filename here...
file.SaveAs(Path.Combine(Server.MapPath("/Content/Images/large"), Guid.NewGuid() + Path.GetExtension(file.FileName)));
var medImg = Images.ResizeImage(Image.FromFile(file.FileName), 250, 300);
medImg.Save(Path.Combine(Server.MapPath("/Content/Images/medium"), Guid.NewGuid() + Path.GetExtension(file.FileName)));
var smImg = Images.ResizeImage(Image.FromFile(file.FileName), 45, 55);
smImg.Save(Path.Combine(Server.MapPath("/Content/Images/small"), Guid.NewGuid() + Path.GetExtension(file.FileName)));
//You don't do anything with the filenames, assuming this is the image URLs
//you want to store then you need to update your Product object with the values
//However your database only seems to support a single image URL so which of
//these URLs is the one you want to store?
}
}
//Remove all this code, this code runs for each iteration of your foreach
//loop, as such you will only add a single product with a single "file"
//and then return, if you only plan for a single file then remove the foreach
//altogether as looping does nothing because of this code
db.Products.Add(prod);
db.SaveChanges();
return RedirectToAction("Index", "Product");
}
//Ensure your prod variable has the image url property set before saving changes
db.Products.Add(prod);
db.SaveChanges();
return RedirectToAction("Index", "Product");
}
The issues I see are several:
- Your foreach only loops once because it saves the product and then immediately returns
- You aren't actually storing the saved image URLs anywhere, store that in your prod (?) if it is in the DB that way
Another issue you're going to run into is security and persistence. From a security point of view your app should not be writing to the Content folder. If you need to persist data to the file system in your web app then create a folder under the standard App_Data
and store the files there. This is important for several reasons. First, this folder is the only legitimate place an app should be able to write too, for security reasons. Second, this folder is dynamically changing at runtime and therefore you should probably ensure this structure is backed up in case of a server failure. Third, when you deploy your app again later it will generally stomp over all the existing folders. This will wipe your product images away and your URLs stored in the DB are now useless. Deployments tend to not change App_Data so that folder can be preserved across deployments.
Another problem I have with your approach is that you're storing an app-specific path into a database that should be app agnostic. If you really need images in the database then either store the images directly in the DB (as VARBINARY) or, preferably, use a file stream (for MS SQL) to have the images stored in the file system by SQL. In either case the images are part of the data and should be with the database.
If you absolutely have to store the images on disk, outside the database, then either use a convention or store just the raw filename, not an entire path. In the future if you need to move this folder (for reasons given earlier) then you will have to update all your rows to point to the new path. Instead just store the filename and have your app take the given filename and append it to the root path where you store product images. This, to me, is an app-level decision. Alternatively don't store the filename at all. If the product has an ID (int, guid, whatever) then just store the image on disk using the product ID (e.g. 1.jpg
). So given a product 1 the app knows the filename (or at least can quickly find it).