Ошибка загрузки в чате

Привет! Я хочу сообщить о проблеме, которую обнаружил при загрузке файлов или изображений в личных сообщениях (чат DMs).\n\n* Когда Пользователь A загружает Файл A в посте или чате, всё работает нормально, и файл регистрируется корректно.\n\n* Однако, когда Пользователь B скачивает Файл A и пытается загрузить его снова в другом чате:\n\n * Если Пользователь B отправляет файл без текста, появляется ошибка:\n\n \n Сообщение слишком короткое, должно содержать минимум 1 символ.\n \n \n\n * Если Пользователь B отправляет файл с текстом, отправляется только текст — вложение отсутствует.\n\nОднако, если Пользователь A снова загружает тот же файл, всё работает нормально, и файл включается в сообщение.\n\nМой вопрос:\nЯвляется ли это ожидаемым поведением, при котором только оригинальный загрузчик может повторно использовать загруженный файл? Или другие пользователи также должны иметь возможность загружать и отправлять тот же файл в чате?\n\n

1 лайк

Да, это похоже на небольшой сбой. Кто-нибудь посмотрит в ближайшие несколько недель, а пока я с радостью добавлю пометку «PR приветствуется».

1 лайк

Мне потребовалось некоторое время, чтобы разобраться в этом, но я, кажется, нашёл причину. Проблема, думаю, связана с этим кодом:

def fetch_uploads(params:, guardian:)
  return [] if !SiteSetting.chat_allow_uploads
  guardian.user.uploads.where(id: params.upload_ids) # В частности, здесь
end

При создании нового сообщения чата этот метод вызывается для получения вложений, которые нужно прикрепить к сообщению. Вероятно, чтобы гарантировать право собственности, метод обращается к пользователю через Guardian, чтобы получить только те вложения, которые принадлежат этому пользователю.

Проблема в том, что вложения дедуплицируются, как видно здесь:

# есть ли уже такое вложение?
@upload = Upload.find_by(sha1: sha1)

# ...

# если есть, возвращаем предыдущее вложение
if @upload
  add_metadata!
  UserUpload.find_or_create_by!(user_id: user_id, upload_id: @upload.id) if user_id
  return @upload
end

Я думаю, что возможным решением было бы работать с UserUpload, а не просто рассматривать вложения, принадлежащие пользователю. UserUpload связывает вложения с несколькими пользователями, что, кажется, именно то, что нам нужно. Пока я не на 100% уверен, как это правильно реализовать; уже поздно, поэтому я ложусь спать, но если никто другой не исправит это, я вернусь позже и попробую подготовить PR. :slightly_smiling_face:

3 лайка

Это отличный и точный замечание. :hugs:

Загрузки имеют user_id, но это просто оригинальный пользователь, который создал загрузку.

Преобразование этого кода в:

Upload.where(id: params.upload_ids).joins(:user_uploads).where(user_uploads: { user_id: guardian.user.id })

решит проблему.

Можешь отправить PR!

Спасибо, я не был уверен, как лучше построить этот запрос (я здесь ещё новичок :slightly_smiling_face: ). Я займусь созданием PR.

3 лайка

Отлично, я наконец-то завершил PR. Извините за задержку, вмешались жизненные обстоятельства.

ИСПРАВЛЕНИЕ: в сообщениях чата фильтровать загрузки по UserUpload, а не по Upload.user от clechasseur · Pull Request #34596 · discourse/discourse

Я вижу, что некоторые тесты не прошли, но я не трогал эти тесты. У меня тоже возникли проблемы с запуском тестов в этих файлах локально — я получил те же ошибки дублирования ключа, что и здесь… Не уверен, что сломал что-то я или же тесты иногда нестабильны…

Хорошее практическое правило — перезапустить упавшие тесты как минимум три раза и начинать паниковать только если они упадут в третий раз :winking_face_with_tongue:

Тем не менее, @j.jaffeux, может быть, сможет помочь?

1 лайк

Да, это просто нестабильные и другие случайные ошибки, ваши изменения в порядке. Перезапускаю тесты для PR: если они пройдут или снова упадут с той же случайной ошибкой, я одобрю и сделаю слияние.

1 лайк

Я слишком большой фанат ужасов, чтобы не знать, что происходит, когда что-то делаешь трижды :sweat_smile:

2 лайка

Этот PR был слит, еще раз спасибо за вклад @clechasseur :rocket:

6 лайков