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

Works fine. Evstropov Evgenii #6

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

EJohnF
Copy link

@EJohnF EJohnF commented Oct 16, 2016

No description provided.

@EJohnF EJohnF changed the title Only middleware worked now. I think it works. But test fail. Evstropov. E. Oct 17, 2016
@EJohnF EJohnF changed the title I think it works. But test fail. Evstropov. E. Works fine. Evstropov Evgenii Oct 17, 2016

const PORT = process.env.PORT || 4000;
const app = express();
const PORT = 4000 || process.env.PORT;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

бессмысленная строка, всегда будет возвращаться 4000

@@ -1,13 +1,277 @@
'use strict';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Разнеси по разным файлам, чтобы было легче читать-понимать

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Если если структура типа ./temp/../ - значит нужно просто удалить из пути temp и отсыл наверх.
Опять же тут не полная продуманность, и структура /../../ - просто удалится и всё, а не откинется на 2 уравня на верх
*/
path = path.replace(new RegExp('.+/\.\./', 'gi'), '');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Молодец, что сам подумал над резолвингом путей. Однако, если использовать встроенный модуль path, то всё гораздо проще

fs.readdir(path, (err, items) => {
console.log(items);
// не люблю я это место... но и не знаю как ещё эти штуки присобачить, а они нужна
let json_items = ['.', '..'] + items;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У тебя же items - то массив, просто положи туда через push, или можешь сделать ['.', '..'].concat(items)
Обычные операции над массивом

trns.on('end', () => {
res.setHeader('Transfer-Encoding', 'chunked');
res.setHeader('Content-Type', 'application/json');
res.end(JSON.stringify({content: str}));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ты конечно здорово придумал, сначала весь поток собрал в переменную, а потом просто её вернул. А зачем тогда потоки, почему нельзя просто прочитать сначала весь файл, засунуть его в транслятор и вернуть. Тебе нужно сделать всё полностью на потоках, примерно stream.pipe(trns).pipe(res) чтобы у тебя сразу результат стал отдаваться, а не собирался в переменную. Потому что если вайл несколько гигабайт, то ты будешь всё собирать в переменную и будет переполнение

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вот я сначала и клал просто в рез через pipe. Но так не проходило, потому что нужна ведь json-ка в результирующее поле. Поэтому придумал такой обходной, не очень хороший вариант

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ты же в итоге кладешь не JSON, а строку, а строку ты и через поток можешь успешно положить

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

т.е. сначала положить заголовки.
Потом положить строку '{content: '
потом прокинуть стрим в результат
и потом ещё завершить, отправив '}' ?

const files = (req, res) => {
let path = '';
//MAGIC
if (req.params[0] != undefined) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Откуда списал эту магию?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нашёл таким путём:
задал путь /v1/temp
и в req.params оказалось два элемента
req.params[0]
и
req.params[arr]
и вот для меня вообще не понятно почему именно такие поля и т.д. но раз так, я просто их потом сконкатенировал чтобы получить полный путь, который мне и нужен.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants