Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a Project Icon #217

Closed
Brend-Smits opened this issue Aug 17, 2020 · 12 comments · Fixed by #241
Closed

Add a Project Icon #217

Brend-Smits opened this issue Aug 17, 2020 · 12 comments · Fixed by #241
Assignees
Labels
Good first issue Good first issue to get to know the project internally. requires development

Comments

@Brend-Smits
Copy link
Member

Is your feature request related to a problem? Please describe.
No way to add/update icon of a project

Describe the solution you'd like
The backend needs a way to save a project icon so the frontend can add this feature. We could save the icons in base64 or something similar. Need to do some research as to how to save these and future icons and how to transfer them using our API.

Additional context
The solution that you propose and implement should keep in mind future icons like the ones from a user's avatar.

@Brend-Smits Brend-Smits added requires development Good first issue Good first issue to get to know the project internally. labels Aug 17, 2020
@niraymak niraymak self-assigned this Sep 7, 2020
@waltersajtos
Copy link
Member

I'm currently working on the front-end part related to this issue. I did some research online and I can't find a way to send an image through json other than base64.

For the property name I think we just go with 'icon'?

@Maxvanhattum
Copy link
Contributor

Another option would be to just have another endpoint (put multipart/form-data) in the back-end where you process the image uploads, returning an ID. Allows more freedom in handling the image, easier debugging and reduced processing time. Then the ID is sent back to the client which then sends another request with the ID and other data, letting the server reassociate it with the uploaded picture.

@Brend-Smits
Copy link
Member Author

Does @rubenb994 and/or @niraymak have any opinion on this? Let's not reinvent the wheel and just research what other companies/apps/websites use for this exact feature. I'm sure there are some blogs/repo's publicly available

@waltersajtos
Copy link
Member

waltersajtos commented Sep 14, 2020

I think one big factor that we need to consider is how big will the files be? If we just plan on storing project/ profile icons we can get away with just storing 32x32/ 64x64/ 128x128? But say we want to implement bigger images (profile banners for example) the files would get a lot bigger. In that case, it probably makes more sense to store the files, whether it would be on the server or some CDN (I.E AWS, FireBase, or Cloudflare).

So the options are: using a Content Delivery Network, storing the files in the database, or storing them on the server.
According to a research paper from Microsoft, it would make more sense to store the images in the file system.

The study indicates that if objects are larger than one megabyte on average, NTFS has a clear advantage over SQL Server.

If the objects are under 256 kilobytes, the database has a clear advantage.

The thing is that saving only an id/ file-path to the image in the database would require an extra backup method for the images.

If there are no plans for saving bigger images than the profile/ project icons then we might get away with the database approach as an average 256x256 image is around 256kb.

Using a CDN could make sense but that I don't know how expensive that would be and if we can just make those decisions.

@niraymak
Copy link
Member

I agree on what Walter says.

Also please note: I tried encoding an image using https://www.base64-image.de/ . A .png 256x256 is encoded already 16336 characters. I have no experience with this but I doubt if we should store strings this big in the database.. (Also: base64 encoded can be up to 133% the original file size)

@Maxvanhattum
Copy link
Contributor

Maxvanhattum commented Sep 14, 2020

Maybe we should also consider if in some point of the future we are to support image/file uploads in other places too e.g. for in a project description. If so then a separate endpoint for image handling will be necessary anyways.

Quick edit: the research paper from microsoft is quite old, but I also can't find other 'legit' sources on the topic.
However when searching for best practices on image/file handling in combination with the REST architecture, separate file handling (ultimately storing locally or preferably on a cdn) comes out on top.

@rubenb994
Copy link
Contributor

The way to go is without a doubt storing the images on the filesystem and NOT in the database. See the following StackOverflow post for more elaboration: https://softwareengineering.stackexchange.com/questions/150669/is-it-a-bad-practice-to-store-large-files-10-mb-in-a-database.

A separate endpoint should be implemented for uploading images. I would recommend keeping track of all images/files in the db, containing a unique ID for each image and rename each uploaded file to match the unique id.

I think the following article should provide a good starting point for how this should be implemented: https://code-maze.com/upload-files-dot-net-core-angular/

@StijnGroenen
Copy link
Member

I strongly agree with @rubenb994 that files should not be stored in the database (unless it's a file specific type of database like MongoDB's GridFS or something like S3 or Minio).
I recommend the approach that @Maxvanhattum and @rubenb994 suggested. So uploading the images (in binary form), storing them and than returning the ID. This ID can be stored in the database for each project.
There should be an endpoint for the project image files like /image/{ID} or something that returns the image in binary form using the right content headers. This endpoint can be used as the src attribute of the image. This way the browser can decide when to load the image. This makes it asynchronous and allows for browser caching.

@waltersajtos
Copy link
Member

I agree with @rubenb994 too,

The only challenge this will bring is backing up the images.

@rubenb994
Copy link
Contributor

I strongly agree with @rubenb994 that files should not be stored in the database (unless it's a file specific type of database like MongoDB's GridFS or something like S3 or Minio).
I recommend the approach that @Maxvanhattum and @rubenb994 suggested. So uploading the images (in binary form), storing them and than returning the ID. This ID can be stored in the database for each project.
There should be an endpoint for the project image files like /image/{ID} or something that returns the image in binary form using the right content headers. This endpoint can be used as the src attribute of the image. This way the browser can decide when to load the image. This makes it asynchronous and allows for browser caching.

I think the solution can be even more generic, why focus on images when it can be about processing all types of files. If I remember correctly eventually we also want to store .pdf and .doc files anyway.

@Brend-Smits
Copy link
Member Author

Thanks all for the discussion and contributions. Let's go with the approach of a single endpoint to upload files and storing a unique path for each file in our database.

Since files are stored on disk, we should be able to easily backup the files using our existing backup solutions. In the end, we're just going to have to back up an additional volume.
Once we scale up, we should be able to easily hook up to a CDN to make sure our files are accessible across the world with good performance.

@StijnGroenen
Copy link
Member

I strongly agree with @rubenb994 that files should not be stored in the database (unless it's a file specific type of database like MongoDB's GridFS or something like S3 or Minio).
I recommend the approach that @Maxvanhattum and @rubenb994 suggested. So uploading the images (in binary form), storing them and than returning the ID. This ID can be stored in the database for each project.
There should be an endpoint for the project image files like /image/{ID} or something that returns the image in binary form using the right content headers. This endpoint can be used as the src attribute of the image. This way the browser can decide when to load the image. This makes it asynchronous and allows for browser caching.

I think the solution can be even more generic, why focus on images when it can be about processing all types of files. If I remember correctly eventually we also want to store .pdf and .doc files anyway.

The solution should indeed be usable for all file types.

I agree with @rubenb994 too,

The only challenge this will bring is backing up the images.

An object storage like S3 or Minio could be the solution for this. Most cloud provider already have an object storage product. (I believe Digital Excellence is hosted at OVH. They have an object storage product as well.) Most existing solutions have built-in backup options.

@Brend-Smits Brend-Smits added this to To do in Sprint 5 - Backend via automation Sep 14, 2020
@Brend-Smits Brend-Smits moved this from To do to In progress in Sprint 5 - Backend Sep 15, 2020
@niraymak niraymak mentioned this issue Sep 21, 2020
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Good first issue to get to know the project internally. requires development
Projects
No open projects
Sprint 5 - Backend
  
In progress
Development

Successfully merging a pull request may close this issue.

6 participants