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

[2.1] Storage feature #588

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@rsdevigo
Copy link
Contributor

commented Jun 6, 2019

Alteração do modo de upload do i-Educar, utilizando a abstração FileSystem do Laravel.

Descrição

Alterei os arquivos que realizava o upload de imagem, pdf e arquivos diversos para o S3, removendo a parte do uso do s3_config.php e mudando a lógica para usar o Illuminate\Support\Facades\Storage e o Illuminate\Http\File do Laravel.

  • O File cria um arquivo dando um nome hash único para ele.
  • O Storage salva esse arquivo, usando o sistema de arquivos padrão (S3, Local ou Min.io).
  • O Storage retorna a url para o arquivo, assim voltando o caminho para o objeto que disparou o upload.

O bom é, que essa implementação possui uma configuração simples do tipo de upload que o i-Educar pode usar, criei um tutorial de como configurar no arquivo: upload_tutorial.MD na raiz do projeto.

Contexto e motivação

Em conversa no grupo do Telegram após algumas dúvidas sobre upload do i-Educar, o Éber Freitas deu a ideia de usar a abstração FileSystem do Laravel para tal tarefa, pensei então em realizar a atividade.

Vale ressaltar que não pensei em alterar muita coisa do jeito que o i-Educar está estruturado, apenas troquei a classe chamada nos arquivos image_check, file_check e file_check_just_pdf.

Tipos de alterações

  • New feature (Não quebra outras funcionalidades e adiciona funcionalidades)

Checklist:

  • Eu li o documento CONTRIBUTING. [REQUIRED]
  • Meu código segue a PSR2. [REQUIRED]
  • Todos os testes novos e existentes estão passando. [REQUIRED]
  • Minhas alterações necessitam de alteração na documentação e já foram feitas.

rodrigodevigo added some commits Jun 6, 2019

Criado o tutorial, adicionado no .env um exemplo de configuração e no…
… composer.json um comando de criação de link simbolico da pasta public do storage
@edersoares

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

Excelente @rsdevigo, obrigado pela contribuição.

Logo estaremos fazendo o code review e testes para podermos fazer o merge.

Abraço.

@edersoares edersoares changed the title Storage feature [2.1] Storage feature Jun 8, 2019

@edersoares
Copy link
Member

left a comment

@rsdevigo simplesmente uma excelente contribuição!

Já estava para desenvolvermos a algum tempo, mas a contribuição partir da comunidade é algo simplesmente sensacional.

Em resumo fiz alguns comentários pontuais visando manter um pouco mais o padrão Laravel para nós aproveitarmos a documentação do mesmo.

Eu prefiro a abordagem que utiliza local storage, pois assim não é necessário outros usuários entenderem e configurar mais uma tecnologia, estamos tendo muitas dificuldades com isso. Assim quero saber o teu ponto de visto quando a removermos o MinIO Server e criarmos um tutorial de como fazer a instalação, o que acha?

No mais, ficou excelente, já fiz testes locais e funcionou certinho. Assim que fizer as alterações, iremos colocar no nosso servidor de homologação.

Obrigado, abração!

Show resolved Hide resolved .env.example Outdated
Show resolved Hide resolved .env.example Outdated
Show resolved Hide resolved .env.example Outdated
Show resolved Hide resolved .env.example Outdated
Show resolved Hide resolved config/filesystems.php Outdated
Show resolved Hide resolved ieducar/intranet/image_check.php Outdated
@@ -0,0 +1,63 @@
# Sistema de upload do i-Educar

This comment has been minimized.

Copy link
@edersoares

edersoares Jun 10, 2019

Member

Este arquivo vamos mover para a documentação do i-Educar e criar uma página na Wiki do GitHub.

Show resolved Hide resolved ieducar/intranet/file_check.php
Show resolved Hide resolved ieducar/intranet/file_check_just_pdf.php
Show resolved Hide resolved docker-compose.yml Outdated
@rsdevigo

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2019

@rsdevigo simplesmente uma excelente contribuição!

Já estava para desenvolvermos a algum tempo, mas a contribuição partir da comunidade é algo simplesmente sensacional.

Em resumo fiz alguns comentários pontuais visando manter um pouco mais o padrão Laravel para nós aproveitarmos a documentação do mesmo.

Eu prefiro a abordagem que utiliza local storage, pois assim não é necessário outros usuários entenderem e configurar mais uma tecnologia, estamos tendo muitas dificuldades com isso. Assim quero saber o teu ponto de visto quando a removermos o MinIO Server e criarmos um tutorial de como fazer a instalação, o que acha?

No mais, ficou excelente, já fiz testes locais e funcionou certinho. Assim que fizer as alterações, iremos colocar no nosso servidor de homologação.

Obrigado, abração!

Opa, valeu mano, então a ideia do MinIO seria algo como uma alternativa do S3 self host já que ele é compatível com a API do S3. Então a ideia seria remover do arquivo filesystem.php o minio e então criar um tutorial de como adicionar o suporte ao Minio ?

Irei realizar as modificações pedidas.

Att.

@edersoares

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

@rsdevigo pode deixar as configurações do MinIO no filesystem.php, apenas não vamos deixar ele como padrão, deixaremos como opção, assim como o Apache HTTP Server #592.

Ótimo! Estamos ansiosos para liberar esta feature 😄

@rsdevigo

This comment has been minimized.

Copy link
Contributor Author

commented Jun 12, 2019

Feito, deixei o local como default.
Podemos criar esse tutorial do Minio, com a instalação do próprio.

@edersoares

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

@rodrigodevigo testei novamente e ficou excelente!

Já estou colocando para nosso setor de QA validar, pode apenas remover o arquivo de tutorial de upload, por favor, pois já criei uma página na Wiki: https://github.com/portabilis/i-educar/wiki/Sistema-de-upload-do-i-Educar

@edersoares
Copy link
Member

left a comment

@rsdevigo os testes passaram, apenas tem que adicionar o código que comentei para separar os tenants.

Remover o tutorial de upload.

$file = new File($tmp);
if (Storage::put('/', $file)) {
$filePath= Storage::url($file->hashName());

This comment has been minimized.

Copy link
@edersoares

edersoares Jun 21, 2019

Member
Suggested change
$filePath= Storage::url($file->hashName());
$filePath= Storage::url($file->hashName($tenant));
@@ -54,21 +58,14 @@ function __construct($file, $maxSize = NULL,
}
function sendFile(){
$tmp = $this->file["tmp_name"];

This comment has been minimized.

Copy link
@edersoares

edersoares Jun 21, 2019

Member

Adicionar:

$tenant = config('legacy.app.database.dbname');
@@ -53,19 +56,14 @@ function __construct($file, $maxSize = NULL,
}
function sendFile(){
$tmp = $this->file["tmp_name"];

This comment has been minimized.

Copy link
@edersoares

edersoares Jun 21, 2019

Member

Adicionar:

$tenant = config('legacy.app.database.dbname');
$file = new File($tmp);
if (Storage::put('/', $file)) {
$filePath= Storage::url($file->hashName());

This comment has been minimized.

Copy link
@edersoares

edersoares Jun 21, 2019

Member
Suggested change
$filePath= Storage::url($file->hashName());
$filePath= Storage::url($file->hashName($tenant));
else{
$this->imageName = $file->hashName();
if (Storage::put('/', $file)) {
$filePath= Storage::url($file->hashName());

This comment has been minimized.

Copy link
@edersoares

edersoares Jun 21, 2019

Member
Suggested change
$filePath= Storage::url($file->hashName());
$filePath= Storage::url($file->hashName($tenant));
$this->imageName = $imageName;
function sendPicture(){
$tmp = $this->imageFile["tmp_name"];

This comment has been minimized.

Copy link
@edersoares

edersoares Jun 21, 2019

Member

Adicionar:

$tenant = config('legacy.app.database.dbname');
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.