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

Cохранение картинки вызывает reload #86

Open
bibendi opened this issue Apr 2, 2018 · 8 comments
Open

Cохранение картинки вызывает reload #86

bibendi opened this issue Apr 2, 2018 · 8 comments

Comments

@bibendi
Copy link
Contributor

bibendi commented Apr 2, 2018

https://github.com/abak-press/apress-images/blame/105c760d99c957b826e3cbd19d9cea6216bd2704/app/models/apress/images/extensions/background_processing.rb#L67

Зачем здесь сделан reload, кто-нибудь знает? Мне кажется, что у меня какая-то проблема в том, что ассоциация каждый раз перезагружается после сохранения модели:

product = create(:product, :valid)
product_image = create(:product_image, product: product)

product_image.association(:product).loaded?
=> false

product.object_id == product_image.product.object_id
=> false
@bibendi bibendi changed the title Каждое сохранение картинки вызывает reload ассоциации product Каждое сохранение картинки вызывает reload Apr 2, 2018
@bibendi bibendi changed the title Каждое сохранение картинки вызывает reload Cохранение картинки вызывает reload Apr 2, 2018
@Napolskih
Copy link
Contributor

Похоже на какой-то кастыль для защиты от повторной постановки, после второго коммита ? защита от гонок.

Надо понять, когда может возникнуть вторая постановка, может заменит на уникальность image_id?

Может Андрей что-то помнит. @isqad

7547b56

@isqad
Copy link
Contributor

isqad commented Apr 3, 2018

Это, похоже, просто ошибка. Перестраховка от случая когда кто-то параллельно изменил флаг например, какой-нибудь таск. Я думаю безболезненно можно убрать релоад.

@bibendi
Copy link
Contributor Author

bibendi commented Apr 3, 2018

Я уберу тогда, потому что у меня все тесты валятся в apress-products. Я туда ProductImage выношу

@bibendi
Copy link
Contributor Author

bibendi commented Apr 3, 2018

Не получается убрать. Этот костыль сделан тут не просто так.
Это реально костылище!!!

Вот этот prepare_enqueuing

def prepare_enqueuing
self.processing = true
nil
end

вызывается вот из этого перекрытого метода

alias_method_chain :save, :prepare_enqueuing

# Public: пометить модель для отложенной обработки
#
# Returns nothing
def save_with_prepare_enqueuing
was_dirty = @dirty
save_without_prepare_enqueuing.tap do
instance.prepare_enqueuing if delay_processing? && was_dirty
end
end

По факту, колонка присваивается уже после реальной вставки в базу (видимо на это и расчет). То есть в базу вставляется processing: false, в модели выставляется зачем-то processing: true. Так как все это происходит в перекрытом save, то у AR сносит башку и он не понимает, что в базе записалось одно, а выставили другое, changes выводит пустой хеш {}. И поэтому тут только reload и помогает.

Я сильно боюсь трогать этот код, так как обязательно что-то сломается. Тут надо конкретно сесть и подумать что происходит, что за чем должно идти, какие флаги когда выставлять, зачем используется колонка processing в качестве временного флага.

@isqad
Copy link
Contributor

isqad commented Apr 3, 2018

Ну это точно ошибка,
флаг был расчитан на то, что он поднимается после взятия джобом в обработку этой картинки.
никак не до.

@bibendi
Copy link
Contributor Author

bibendi commented Apr 3, 2018

Если что, то вот так пытался сделать master...bibendi:remove-reload

@bibendi
Copy link
Contributor Author

bibendi commented Apr 3, 2018

Андрей, раз ты в теме, то может как-нибудь возьмете себе в план посмотреть?

@isqad
Copy link
Contributor

isqad commented Apr 3, 2018

Да, https://jira.railsc.ru/browse/BPC-12195

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

No branches or pull requests

3 participants