Как тут не вспомнить анекдот про секретаршу, которая печатает те самые 10 тыщ знаков в секунду. Из того что заметил при беглом просмотре - Гек Финн явно сын негра Джима. А сам Джим превратился в карлика.
Из того, за что взгляд зацепился при беглом просмотре
функция query() - очень хорошо.
функция fetch_array() - очень так себе, пришита к классу $connect белыми нитками. И по сути это такая страусиная политика, поскольку объект mysqli_result всё равно протекает из вашей абстракции. Надо или не изображать умное лицо, а писать по-простому $result->fetch_array(), или - если делать по уму - то сделать свой класс $dbResult, который инкапсулирует работу с mysqli_result и будет логично вызываться, как $row = $dbResult->fetchArray();. Смена кейса нужна, чтобы не путать этот объект с оригинальным mysqli_result
empty($_COOKIE['tmpsid'])) OR (!isset($_COOKIE['tmpsid'])) - это масло масляное, вместе их писать никогда нет смысла. Прочтите хотя бы один раз описание empty в мануале. И в целом там какая-то дичь, почему-то условия дублируются. По уму нужно только два - isset и регулярка.
$hash = md5(ip().time()); подделывается влёт. Не надо экономить на спичках, надо сгенерить нормальное случайное значение через random_bytes() и записать его в базу
в целом лучше разделить класс Admin на два - модель и контроллер, чтобы второй в своей работе использовал методы первого, а не работал с базой напрямую.
"Все наименование полей и таблиц собираются из формы, а так же: тип данных, поле с уникальным значеним, обязательно поле к заполнению" - такой GraphQL на коленке, наверняка состоит из инъекций чуть более, чем весь. В итоге размазали работу БД между моделью и клиентом. Нормальный такой MVC.
В целом, как я и писал, это материал для Тостера, а не для Хабра. Если в с первой статьёй вас решили поддержать, то эта вряд ли будет в плюсах. С другой стороны, фидбека вы соберёте полную панамку.
Первая транзакция прочитала сумму, и убедилась, что списание не выходит из лимита. Вторая транзакция по вашему сценарию прочитала сумму, и убедилась, что списание не выходит из лимита. Первая транзакция добавила строку и закоммитилась. Вторая транзакция добавила строку и закоммитилась. Сумма ушла в минус.
Денис, вы меня извините, но подход "отбраковка руками пользователей" для меня остался в прошлом веке, вместе с компанией "Рога и Копыта" с Большой Якиманки.
В этом веке я ожидаю, что ляпы будут единичными, а не мейнстримом. А от новостного редактора я ожидаю, что он хотя бы один раз прочтёт своё творение и попытается понять его смысл. А не будет ждать, когда его натыкают носом читатели.
Причём речь идёт не об отдельных ляпах типа опечаток или несогласованных предложений, а о целых абзацах или даже постах, таких как например ваше предыдущее творение, в котором 150 лет лёгким движением руки превращаются в 300.
При этом, сама запись остается открытой для чтения, и в случае параллельного чтения другой, не пишущей транзакцией, в случае отката нашей, та вернет актуальные данные!
Ну так об этом и речь. А в случае успешного завершения транзакции прочитанное значение станет невалидным, и позволит выйти из лимита.
А предотвратить это можно только блокировкой на чтение. Причём у нас только два варианта - либо лочить только записи пользователя, что, по вашему "не нужно" - и тогда остаётся лочить всю таблицу. Уровнем ли изоляции, явной ли блокировкой - но суть одна.
Нам нужна очередь из транзакций с insert.
Опять у вас общие фразы, не имеющие ничего общего с реальностью. У нас даже близко нет ничего похожего на "просто очередь транзакций с insert". У нас очередь запросов insert с условием. И состояние гонки. Напоминаю - это не просто "хронологически связанные данные", которые льются потоком. А данные, которые добавляются или не добавляются в зависимости от текущего состояния БД.
Опять же, в порядке конструктивного диалога с самим собой: в теории, наверное можно делать и по вашему принципу: валить всё, что пришло, а после вставки считать результат, и если меньше лимита, то откатывать вставку. Но этот вариант мне нравится меньше всех остальных. Да, вероятность получить пополнение между снятиями практически нулевая, но в жизни всякое бывает. И, главное, такой подход исходно выглядит недетерминированным, то есть могут быть и другие проблемы, которых я сейчас не могу сообразить.
Это верно, но как тут как раз те ошибки, на которых очень полезно учиться. Как говорит пословица, за одного битого двух небитых дают. То, что понял на собственном опыте, в сто раз ценнее чем то, что ты делаешь потому что дядя сказал.
Мне не очень нравится, что ваша аргументация сводится к общим словам уровня RTFM и ссылке на статью, в которой запрос insert не упоминается ни разу. Если будете трудиться отвечать на этот комментарий, то большая просьба писать только конкретные схемы.
В итоге мне приходится самому от себя получать адекватный фидбек. Получается что да, мы можем, наверное, залочить на чтение не всю таблицу, а только записи данного пользователя, прочитать верхнюю строчку, посмотреть в РНР, не превысит ли новое списание лимит, и если нет - то добавить новую запись и разблокировать остальные. Параллельная вставка не сможет прочитать максимальный баланс, и будет ждать вставки, после которой прочтёт в РНР новый баланс и увидит, что выходит за рамки лимита. То есть опять же, ваше лишнее поле не нужно, кстати. "Необходимость" которого вы, в своей обычной манере, постулировали, но не аргументировали.
При этом "отсутствующая величина" в виде атомарного апдейта позволяет перенести контроль выхода за пределы лимита целиком на уровень базы данных, причём вообще без блокировок. И заодно даёт ваш любимый дополнительный карман.
В целом же мне кажется, что вы путаете транзакции с блокировками. И полагаете, что транзакция - это такая волшебная палочка, которая как-то там сама по себе решает все проблемы - достаточно просто сказать волшебное слово "транзакция". А меня интересует именно конкретика.
Хороший вариант, но это будет уже следующий этап, до которого автору надо дорасти. Он только-только ООП начал осваивать. Пусть велосипедит текущую версию - и в процессе как раз дозреет до использования фреймворков. А если сейчас его посадить на Лару, то получится просто очередная обезьяна за пишмашинкой.
Насколько я понимаю, под "обычной транзакцией" вы имеете в виду полную блокировку таблицы. Поскольку другого варианта залочить под вставку нет. И если сравнивать её с хранением баланса в отдельной таблице (и блокировкой только одной строки в этой таблице), то эта величина хоть и "отсутствующая" но куда более приемлемая в реальности.
@moderator ну действительно, неужели так сложно автоматизировать "информационную службу хабра", если она всё равно гонит машиный шлак? Сделайте кастомный фид, из которого робот будет по набору правил брать новости, переводить, и постить на Хабр.
Вы так ничего и не поняли. про старый говнокод писать вообще совсем не нужно. Если писать - то про свой нынешний. В котором, судя по всему, у вас там в полный ад. И вы бы получили в сто раз больше фидбека, показав реальный код, а не картинки.
Но вообще, судя по всему, вам не на хабр, а на Тостер (https://qna.habr.com/). Описывать свои идеи по улучшению и сразу получать фидбек на тему почему так делать не надо.
Это верно. Но это не единственное достоинство пхп. Внешняя многопоточность делает его гораздо более устойчивым к проблемам с переполнением памяти например. Элементарный деплой, который стал уже притчей во языцех. У меня здесь на Хабре был длинный спор с одним явистом, в результате которого выяснилось, что ему проще поправить хранимую процедуру в БД, чем редактировать запрос в коде и потом это всё деплоить. Мы очень долго не могли друг друга понять, это просто разные миры!
Погодите, а откуда там хоть один юнион, не говоря уже о "простыне"? Ведь ваш список тупо строится по таблице транзакций, которая и так есть у автора? Я могу неправильно понимать структуру таблиц автора, но пока я вижу только одну, из которой всё берётся простым запросом, без всяких юнионов
Извините, но это просто чушь какая-то. Судя по всему, в части инъекций там остался весь ламерский код из предыдущей версии, и вы мечетесь, пытаясь применить дебильные советы из интернета, как уменьшить урон, и в частности не давать право на вставку. Хотя этого давно никто не делает за полной бессмысленностью.
Бороться надо с инъекциями, а не с их последствиями.
Очень плохое решение. Достаточно в каждом поступлении/списании хранить еще поле остатка.
Очень смешной комментарий. Звучит как "очень плохо класть зажигалку в карман. Достаточно пришить ещё один". Вам не кажется, что "хранить еще и поле остатка" - это дополнительное действие, которое никак не подходит под формулировку "достаточно"? А достаточно как раз хранить только сами транзакции?
Другое дело, что с дополнительным полем можно будет реализовать атомарный апдейт и избежать двойных списаний без блокировки всей таблицы. Но тогда это должно быть поле в отдельной таблице с уникальным user_id. И делать транзакцию, где сначала идёт апдейт этой таблицы с условием, и роллбек если условие не выполнилось.
Во вторых, после подтверждения, прям перед update - простая проверка, записан там кто то или нет.
Непонятно, почему такую проверку нельзя было сделать и для INSERT. То есть вы, кажется, так и не поняли своё же решение, выставляя его так, что ключевым является избавление от инсерта, а не проверка перед записью.
Но главное, такая проверка не гарантирует перезаписи при одновременном сохранении формы. Но это пока не горит, а потом узнаете про борьбу с race condition поподробнее.
Статья вызывает смешанные чувства. С одной стороны, ещё года два назад такое сочинение пятиклассника "как я провёл этим летом" выпинали бы в чулан in no time. С другой - поляна настолько обезлюдела, что надо поддерживать и такие ростки.
Примерно то же самое относится и к оформлению. Вроде бы, статья про РНР, но в качестве иллюстраций... картинки интерфейса! Хотя куда логичнее было бы приводить примеры кода. Просто поставьте себя на моё место: я зашел в хаб РНР прочесть статью а почему-то рассматриваю картинки, которые вообще ничего не говорят о внутреннем устройстве системы.
Чисто для информации:
активная поддержка РНР 8.2 уже завершена. Если переписывать сейчас, то под 8.4. Хотя конечно там для вас разницы, пожалуй нету, но всё равно целевой версией надо ставить текущую.
Переписывать на PDO смысл сомннительный. Разница между PDO и mysqli минимальная. Единственное, принципиальное отличие - в mysqli нет именованных плейсхолдеров, вида :name, а только знаки вопроса. Если это для вас принципиально - то переписывайте конечно. Хотя я бы пока сосредоточился на более важных вещах. А потом уже при следующей итерации сразу перешел на какой-нибудь ОRM
Но если говорить в целом, то главное что вы довели этот проект до конца. Это самое важное. А опыт приложится.
Это совет из серии "Лучше быть здоровым и богатым, чем бедным, но больным". Давайте вы возьмёте на себя миссию объяснить заказчику, что затраты на проект надо помножить на два? А уже потом тот наймёт фронта, который расскажет автору как всё плохо.
Как тут не вспомнить анекдот про секретаршу, которая печатает те самые 10 тыщ знаков в секунду. Из того что заметил при беглом просмотре - Гек Финн явно сын негра Джима. А сам Джим превратился в карлика.
Вcё верно, хорошая реклама. Прямо становление программиста в прямом эфире :)
Со временем дойдёт и до всех этих ваших страшных слов. Перепрыгнуть всё равно не получится.
Из того, за что взгляд зацепился при беглом просмотре
функция query() - очень хорошо.
функция fetch_array() - очень так себе, пришита к классу $connect белыми нитками. И по сути это такая страусиная политика, поскольку объект mysqli_result всё равно протекает из вашей абстракции. Надо или не изображать умное лицо, а писать по-простому $result->fetch_array(), или - если делать по уму - то сделать свой класс $dbResult, который инкапсулирует работу с mysqli_result и будет логично вызываться, как $row = $dbResult->fetchArray();. Смена кейса нужна, чтобы не путать этот объект с оригинальным mysqli_result
empty($_COOKIE['tmpsid'])) OR (!isset($_COOKIE['tmpsid']))- это масло масляное, вместе их писать никогда нет смысла. Прочтите хотя бы один раз описание empty в мануале. И в целом там какая-то дичь, почему-то условия дублируются. По уму нужно только два - isset и регулярка.$hash = md5(ip().time()); подделывается влёт. Не надо экономить на спичках, надо сгенерить нормальное случайное значение через random_bytes() и записать его в базу
в целом лучше разделить класс Admin на два - модель и контроллер, чтобы второй в своей работе использовал методы первого, а не работал с базой напрямую.
"Все наименование полей и таблиц собираются из формы, а так же: тип данных, поле с уникальным значеним, обязательно поле к заполнению" - такой GraphQL на коленке, наверняка состоит из инъекций чуть более, чем весь. В итоге размазали работу БД между моделью и клиентом. Нормальный такой MVC.
В целом, как я и писал, это материал для Тостера, а не для Хабра. Если в с первой статьёй вас решили поддержать, то эта вряд ли будет в плюсах. С другой стороны, фидбека вы соберёте полную панамку.
А зачем здесь параллельное-то?
Первая транзакция прочитала сумму, и убедилась, что списание не выходит из лимита.
Вторая транзакция по вашему сценарию прочитала сумму, и убедилась, что списание не выходит из лимита.
Первая транзакция добавила строку и закоммитилась.
Вторая транзакция добавила строку и закоммитилась.
Сумма ушла в минус.
Всё строго последовательно.
Денис, вы меня извините, но подход "отбраковка руками пользователей" для меня остался в прошлом веке, вместе с компанией "Рога и Копыта" с Большой Якиманки.
В этом веке я ожидаю, что ляпы будут единичными, а не мейнстримом. А от новостного редактора я ожидаю, что он хотя бы один раз прочтёт своё творение и попытается понять его смысл. А не будет ждать, когда его натыкают носом читатели.
Причём речь идёт не об отдельных ляпах типа опечаток или несогласованных предложений, а о целых абзацах или даже постах, таких как например ваше предыдущее творение, в котором 150 лет лёгким движением руки превращаются в 300.
Вот! Теперь есть
к чему придратьсяконкретика.Ну так об этом и речь. А в случае успешного завершения транзакции прочитанное значение станет невалидным, и позволит выйти из лимита.
А предотвратить это можно только блокировкой на чтение. Причём у нас только два варианта - либо лочить только записи пользователя, что, по вашему "не нужно" - и тогда остаётся лочить всю таблицу. Уровнем ли изоляции, явной ли блокировкой - но суть одна.
Опять у вас общие фразы, не имеющие ничего общего с реальностью. У нас даже близко нет ничего похожего на "просто очередь транзакций с insert". У нас очередь запросов insert с условием. И состояние гонки. Напоминаю - это не просто "хронологически связанные данные", которые льются потоком. А данные, которые добавляются или не добавляются в зависимости от текущего состояния БД.
Опять же, в порядке конструктивного диалога с самим собой: в теории, наверное можно делать и по вашему принципу: валить всё, что пришло, а после вставки считать результат, и если меньше лимита, то откатывать вставку. Но этот вариант мне нравится меньше всех остальных. Да, вероятность получить пополнение между снятиями практически нулевая, но в жизни всякое бывает. И, главное, такой подход исходно выглядит недетерминированным, то есть могут быть и другие проблемы, которых я сейчас не могу сообразить.
Это верно, но как тут как раз те ошибки, на которых очень полезно учиться. Как говорит пословица, за одного битого двух небитых дают. То, что понял на собственном опыте, в сто раз ценнее чем то, что ты делаешь потому что дядя сказал.
Мне не очень нравится, что ваша аргументация сводится к общим словам уровня RTFM и ссылке на статью, в которой запрос insert не упоминается ни разу. Если будете трудиться отвечать на этот комментарий, то большая просьба писать только конкретные схемы.
В итоге мне приходится самому от себя получать адекватный фидбек. Получается что да, мы можем, наверное, залочить на чтение не всю таблицу, а только записи данного пользователя, прочитать верхнюю строчку, посмотреть в РНР, не превысит ли новое списание лимит, и если нет - то добавить новую запись и разблокировать остальные. Параллельная вставка не сможет прочитать максимальный баланс, и будет ждать вставки, после которой прочтёт в РНР новый баланс и увидит, что выходит за рамки лимита. То есть опять же, ваше лишнее поле не нужно, кстати. "Необходимость" которого вы, в своей обычной манере, постулировали, но не аргументировали.
При этом "отсутствующая величина" в виде атомарного апдейта позволяет перенести контроль выхода за пределы лимита целиком на уровень базы данных, причём вообще без блокировок. И заодно даёт ваш любимый дополнительный карман.
В целом же мне кажется, что вы путаете транзакции с блокировками. И полагаете, что транзакция - это такая волшебная палочка, которая как-то там сама по себе решает все проблемы - достаточно просто сказать волшебное слово "транзакция". А меня интересует именно конкретика.
Хороший вариант, но это будет уже следующий этап, до которого автору надо дорасти. Он только-только ООП начал осваивать. Пусть велосипедит текущую версию - и в процессе как раз дозреет до использования фреймворков. А если сейчас его посадить на Лару, то получится просто очередная обезьяна за пишмашинкой.
Насколько я понимаю, под "обычной транзакцией" вы имеете в виду полную блокировку таблицы. Поскольку другого варианта залочить под вставку нет. И если сравнивать её с хранением баланса в отдельной таблице (и блокировкой только одной строки в этой таблице), то эта величина хоть и "отсутствующая" но куда более приемлемая в реальности.
@moderator ну действительно, неужели так сложно автоматизировать "информационную службу хабра", если она всё равно гонит машиный шлак? Сделайте кастомный фид, из которого робот будет по набору правил брать новости, переводить, и постить на Хабр.
Вы так ничего и не поняли. про старый говнокод писать вообще совсем не нужно. Если писать - то про свой нынешний. В котором, судя по всему, у вас там в полный ад. И вы бы получили в сто раз больше фидбека, показав реальный код, а не картинки.
Но вообще, судя по всему, вам не на хабр, а на Тостер (https://qna.habr.com/). Описывать свои идеи по улучшению и сразу получать фидбек на тему почему так делать не надо.
Это верно. Но это не единственное достоинство пхп. Внешняя многопоточность делает его гораздо более устойчивым к проблемам с переполнением памяти например. Элементарный деплой, который стал уже притчей во языцех. У меня здесь на Хабре был длинный спор с одним явистом, в результате которого выяснилось, что ему проще поправить хранимую процедуру в БД, чем редактировать запрос в коде и потом это всё деплоить. Мы очень долго не могли друг друга понять, это просто разные миры!
Погодите, а откуда там хоть один юнион, не говоря уже о "простыне"? Ведь ваш список тупо строится по таблице транзакций, которая и так есть у автора? Я могу неправильно понимать структуру таблиц автора, но пока я вижу только одну, из которой всё берётся простым запросом, без всяких юнионов
А, то есть версия РНР так и осталась 5.4? Ну тогда флаг в руки, действительно придётся попотеть даже поднимая до 8.2.
Извините, но это просто чушь какая-то. Судя по всему, в части инъекций там остался весь ламерский код из предыдущей версии, и вы мечетесь, пытаясь применить дебильные советы из интернета, как уменьшить урон, и в частности не давать право на вставку. Хотя этого давно никто не делает за полной бессмысленностью.
Бороться надо с инъекциями, а не с их последствиями.
Очень смешной комментарий. Звучит как "очень плохо класть зажигалку в карман. Достаточно пришить ещё один". Вам не кажется, что "хранить еще и поле остатка" - это дополнительное действие, которое никак не подходит под формулировку "достаточно"? А достаточно как раз хранить только сами транзакции?
Другое дело, что с дополнительным полем можно будет реализовать атомарный апдейт и избежать двойных списаний без блокировки всей таблицы. Но тогда это должно быть поле в отдельной таблице с уникальным user_id. И делать транзакцию, где сначала идёт апдейт этой таблицы с условием, и роллбек если условие не выполнилось.
Непонятно, почему такую проверку нельзя было сделать и для INSERT. То есть вы, кажется, так и не поняли своё же решение, выставляя его так, что ключевым является избавление от инсерта, а не проверка перед записью.
Но главное, такая проверка не гарантирует перезаписи при одновременном сохранении формы. Но это пока не горит, а потом узнаете про борьбу с race condition поподробнее.
Статья вызывает смешанные чувства. С одной стороны, ещё года два назад такое сочинение пятиклассника "как я провёл этим летом" выпинали бы в чулан in no time. С другой - поляна настолько обезлюдела, что надо поддерживать и такие ростки.
Примерно то же самое относится и к оформлению. Вроде бы, статья про РНР, но в качестве иллюстраций... картинки интерфейса! Хотя куда логичнее было бы приводить примеры кода. Просто поставьте себя на моё место: я зашел в хаб РНР прочесть статью а почему-то рассматриваю картинки, которые вообще ничего не говорят о внутреннем устройстве системы.
Чисто для информации:
активная поддержка РНР 8.2 уже завершена. Если переписывать сейчас, то под 8.4. Хотя конечно там для вас разницы, пожалуй нету, но всё равно целевой версией надо ставить текущую.
Переписывать на PDO смысл сомннительный. Разница между PDO и mysqli минимальная. Единственное, принципиальное отличие - в mysqli нет именованных плейсхолдеров, вида :name, а только знаки вопроса. Если это для вас принципиально - то переписывайте конечно. Хотя я бы пока сосредоточился на более важных вещах. А потом уже при следующей итерации сразу перешел на какой-нибудь ОRM
Но если говорить в целом, то главное что вы довели этот проект до конца. Это самое важное. А опыт приложится.
Это совет из серии "Лучше быть здоровым и богатым, чем бедным, но больным". Давайте вы возьмёте на себя миссию объяснить заказчику, что затраты на проект надо помножить на два? А уже потом тот наймёт фронта, который расскажет автору как всё плохо.