|
| 1 | +--- |
| 2 | +title: "Глобальное состояние и тестирование через внешнее API" |
| 3 | +date: 2026-02-04T05:25:37+07:00 |
| 4 | +draft: false |
| 5 | +tags: |
| 6 | +- why-fp |
| 7 | +- ergo-testing |
| 8 | +- project-e |
| 9 | +--- |
| 10 | +:source-highlighter: rouge |
| 11 | +:rouge-theme: github |
| 12 | +:icons: font |
| 13 | +:sectlinks: |
| 14 | +:toc: |
| 15 | +:toclevels: 4 |
| 16 | +:toc-title: Содержание |
| 17 | +:imagesdir: /posts/26/02/images |
| 18 | + |
| 19 | +[NOTE] |
| 20 | +-- |
| 21 | +Этот материал является «микропостом» — текстом, написанным в режиме потока сознания без особой редактуры. |
| 22 | + |
| 23 | +Следить за обновлениями блога можно в Telegram-канале: https://t.me/ergonomic_code[Эргономичный код]. |
| 24 | +-- |
| 25 | + |
| 26 | +## Введение |
| 27 | + |
| 28 | +У меня недавно в Проекте Э случилась поучительная и многогранная история. |
| 29 | +Эта история одновременно: |
| 30 | + |
| 31 | +. Иллюстрирует как именно глобальное изменяемое состояние усложняет поддержку кода. |
| 32 | + И соответственно, почему его стоит избегать. |
| 33 | +. Показывает, что зелёные тесты на внешнее API не гарантируют отсутствие тупых багов внутри. |
| 34 | + |
| 35 | +## История |
| 36 | + |
| 37 | +Началась эта история с того, что в админке одного из вспомогательных сервисов Проекта Э мне потребовалось научить PATCH-метод сбрасывать значение пары свойств (выставлять их в null). |
| 38 | +А эта задача в языках с типами является известной болью (https://stackoverflow.com/questions/36907723/how-to-do-patch-properly-in-strongly-typed-languages-based-on-spring-example?utm_source=chatgpt.com[1], https://stackoverflow.com/questions/47772887/spring-data-rest-updates-null-properties-on-patch-when-it-shouldnt?utm_source=chatgpt.com[2], https://www.baeldung.com/jackson-field-absent-vs-null-difference?utm_source=chatgpt.com[3]), потому как по значению null поля входящей DTO невозможно понять — «клиент хочет выставить это поле в null» или «клиент не хочет менять это поле». |
| 39 | + |
| 40 | +Для Java/Kotlin/Jackson в целом есть целый набор устоявшихся решений этой задачи: |
| 41 | + |
| 42 | +. C https://github.com/FasterXML/jackson-modules-java8[jackson-modules-java8] (или Jackson3) в DTO можно использовать `Optional<...>?` (нуллабельный Optional); |
| 43 | +. Можно присылать https://www.rfc-editor.org/rfc/rfc7386[JSON Merge Patch] и https://medium.com/@AlexanderObregon/working-with-json-patch-vs-merge-patch-for-partial-updates-in-spring-boot-apis-57377bfe4e5a[делать патчинг] с помощью Jackson-овго `JsonMergePatch`; |
| 44 | +. Можно присылать https://www.rfc-editor.org/rfc/rfc6902[JSON Patch] и делать патчинг с помощью Jackson-овго `JsonPatch` (пример по той же ссылке, что и выше); |
| 45 | +. Можно использовать https://github.com/OpenAPITools/jackson-databind-nullable[jackson-databind-nullable] (осторожно: модуль с сентября 2025 года ищет маинтейнеров). |
| 46 | + |
| 47 | +Но я решил не использовать ни одно из этих решений. |
| 48 | + |
| 49 | +Optional — вообще не идиоматично для Котлина, а нуллабельные Optional-поля — ещё и для Java. |
| 50 | + |
| 51 | +А тащить варианты с Jackson/Json в слой приложения/домена, где у меня выполняется патчинг сущностей, я счёл неправославными. |
| 52 | + |
| 53 | +Кроме того, в моём случае была сложность в том, что мне патч надо было собрать из двух частей — json с полями (что и как патчим) и контент для поля с картинкой (как патчим поле с картинкой, если загружаем новую). |
| 54 | + |
| 55 | +Поэтому я решил запилить небольшой велосипедик, с кастомным типом для патча поля (PatchField), превращением любого типа с PatchField-полями в мапу и патчингом любого data-класса этой мапой с помощью рефлексии: |
| 56 | + |
| 57 | +[source,kotlin] |
| 58 | +---- |
| 59 | +interface Patch |
| 60 | +
|
| 61 | +sealed interface PatchField<out T> { |
| 62 | +
|
| 63 | + val value: T? |
| 64 | +
|
| 65 | + val isChange: Boolean |
| 66 | + get() = this is Set |
| 67 | +
|
| 68 | + @Suppress("UNCHECKED_CAST") |
| 69 | + fun <R> map(fn: (T) -> R): PatchField<R> = |
| 70 | + when (this) { |
| 71 | + is Set if value != null -> Set(fn(value)) |
| 72 | + else -> this as PatchField<R> |
| 73 | + } |
| 74 | +
|
| 75 | + data object Unchanged : PatchField<Nothing> { |
| 76 | + override val value = null |
| 77 | + } |
| 78 | +
|
| 79 | + data class Set<T>(override val value: T?) : PatchField<T> |
| 80 | +
|
| 81 | +} |
| 82 | +
|
| 83 | +@Suppress("UNCHECKED_CAST") |
| 84 | +fun <T : Any> patch( |
| 85 | + target: T, |
| 86 | + patch: Map<String, Any?> |
| 87 | +): T { |
| 88 | + val targetClass = target::class |
| 89 | +
|
| 90 | + require(targetClass.isData) { |
| 91 | + "Target must be a data class: ${targetClass.simpleName}" |
| 92 | + } |
| 93 | +
|
| 94 | + val constructor = targetClass.primaryConstructor |
| 95 | + ?: error("Data class must have a primary constructor") |
| 96 | +
|
| 97 | + val targetProps = targetClass.memberProperties |
| 98 | + .associateBy { it.name } |
| 99 | +
|
| 100 | + val args = constructor.parameters.associateWith { param -> |
| 101 | + val name = param.name |
| 102 | + ?: error("Unnamed constructor parameter") |
| 103 | +
|
| 104 | + val targetProp = targetProps[name] |
| 105 | + ?: error("No property '$name' in target") |
| 106 | + targetProp.isAccessible = true |
| 107 | +
|
| 108 | + val currentValue = targetProp.getter.call(target) |
| 109 | +
|
| 110 | + val newValue = if (name in patch) { |
| 111 | + patch[name] |
| 112 | + } else { |
| 113 | + currentValue |
| 114 | + } |
| 115 | +
|
| 116 | + newValue |
| 117 | + } |
| 118 | +
|
| 119 | + return constructor.callBy(args) |
| 120 | +} |
| 121 | +
|
| 122 | +fun Patch.toPatchMap(): Map<String, Any?> = |
| 123 | + this::class.memberProperties |
| 124 | + .asSequence() |
| 125 | + .filter { it.returnType.isSubtypeOf(PatchField::class.starProjectedType) } |
| 126 | + .map { prop -> prop.name to (prop.getter.call(this) as PatchField<*>) } |
| 127 | + .filter { (_, field) -> field.isChange } |
| 128 | + .associate { (name, field) -> name to field.value } |
| 129 | +
|
| 130 | +---- |
| 131 | + |
| 132 | +Запилил, перевёл логику на работу через него, дописал тесты на новое поведение, прогнал их, все прошли, закоммитил, запушил, задеплоил и пошёл пилить следующую фичу. |
| 133 | + |
| 134 | +А спустя какое-то время на дейликах юниор начал жаловаться, что чёт нифига не работает: при попытке обновления сущности бэк начинает валится из-за того, что что-то там приватное недоступно. |
| 135 | + |
| 136 | +Но я был занят уже другой фичей, и ни юниор, ни продакт подключиться меня не просили, поэтому я сам не лез. |
| 137 | + |
| 138 | +Спустя **неделю** юниор заводит МР с фиксом: |
| 139 | + |
| 140 | +image::2026-02-04-14-49-23.png[] |
| 141 | + |
| 142 | +Когда я это увидел, я прям почувствовал, как мои седины покрываются позором. |
| 143 | + |
| 144 | +Но! |
| 145 | +Тесты-то зелёные! |
| 146 | +И их 14 (четырнадцать!) штук на один только метод обновления сущности! |
| 147 | +Четырнадцать, Карл! |
| 148 | + |
| 149 | +Тогда я в мастере написал юнит-тест чисто на `patch` — и он сразу же упал с `java.lang.IllegalAccessException: class kotlin.reflect.jvm.internal.calls.CallerImpl$Constructor cannot access a member of class xxx.config.domain.screens.model.Screen with modifiers "private".` |
| 150 | + |
| 151 | +Притом, все АПИ-тесты на апдейт, которые вызывают тот же самый `patch`, безусловно вызывающий приватный конструктор `Screen`-а — проходят успешно. |
| 152 | + |
| 153 | +На этом месте я немного прифигел: как такое вообще возможно, что один и тот же код при вызове из разных мест либо всегда работает, либо всегда падает? |
| 154 | +Но меня спасла [line-through]#интуиция# жопа, испещрённая шрамами: она мне подсказала, что в интеграционных тестах `constructor.isAccessible = true` вызывается силами Spring Data Jdbc при создании сущностей в БД на этапе сетапа фикстуры 🤦♂️. |
| 155 | +Проверил дебаггером — так и есть. |
| 156 | + |
| 157 | +Добавил юнит-тест в МР, прогнал тесты, все прошли, закомиитал, запушил, вмёржил МР и пошёл пилить следующую фичу. |
| 158 | + |
| 159 | +Финита ля комедия. |
| 160 | + |
| 161 | +## Выводы |
| 162 | + |
| 163 | +### Глобальное изменяемое состояние усложняет поддержку |
| 164 | + |
| 165 | +Эта душещипательная и полная взлётов и падений история наглядно демонстрирует что такое «локальность рассуждений» (reasoning locality) и её отсутствие, что такое временнАя сцепленность (temporal coupling) и какие именно проблемы возникают из-за глобального изменяемого состояния (поля `constructor.isAccessible`). |
| 166 | + |
| 167 | +В данном случае из-за отсутствия локальности рассуждений невозможно объяснить, глядя только в код метода `patch`, почему он работает в одних тестах и не работает в других. |
| 168 | +Для того чтобы это объяснить, надо восстановить цепочку, что в случае интеграционных тестов, сначала выполнялась вставка фикстурных данных в БД силами Spring Data Jdbc, который, в свою очередь, при построении мета-модели данных и, в частности, создании `PreferredConstructor : InstanceCreatorMetadataSupport` делает `ctor.setAccessible(true)` |
| 169 | + |
| 170 | +ВременнАя связанность выражается в том, что корректность работы метода `patch` (и метода обновления в целом) зависит от того, был ли **ранее** вызван метод создания сущности (или любой другой метод, который делает `constructor.isAccessible = true`). |
| 171 | + |
| 172 | +В общем, глобальное изменяемое состояние может вас сильно удивить и сожрать от несколько часов до дней на расследование, что вообще происходит. |
| 173 | + |
| 174 | +И хотя в реальном мире полностью обойтись без глобального состояния не получится, в прикладном коде количество изменяемых глобальных переменных можно свести буквально к единицам. |
| 175 | +Упростив тем самым себе жизнь и поддержку кода. |
| 176 | + |
| 177 | +### Зелёные тесты на внешнее API не гарантирует отсутствие тупых багов внутри |
| 178 | + |
| 179 | +Интернет полон мемов, высмеивающих юнит-тесты. |
| 180 | + |
| 181 | +[Attributes,cols="^,^,^",frame=none,grid=none] |
| 182 | +|=== |
| 183 | +a|image::2026-02-03-11-13-53.png[width=310] |
| 184 | +a|image::2026-02-04-09-10-51.png[width=310] |
| 185 | +a|image::2026-02-04-09-11-07.png[width=310] |
| 186 | +|=== |
| 187 | + |
| 188 | +И в моей практике наборы тестов, состоящие исключительно из юнит-тестов, действительно наносят больше вреда, чем пользы. |
| 189 | +Они препятствуют рефакторингу тем, что слишком плотно связаны с реализацией, и при этом не дают вообще никакой уверенности в том, что система работает. |
| 190 | + |
| 191 | +Именно поэтому Эргономичный подход (link:++{{<ref "/posts/22/10/universal-testing-strategy.adoc">}}++[вслед за xUnit Patterns], кстати) фокусируется на тестировании через внешнее API (storytest-driven development с дополнением юнит-тестов для покрытия непокрытых путей). |
| 192 | + |
| 193 | +И вот в первый раз за почти шесть лет этой практики я столкнулся с ситуацией, когда в код, покрытый тестами вдоль и поперёк, казалось бы, закралась тупая ошибка в хэппи пасе и тесты это не отлоовили. |
| 194 | + |
| 195 | +В общем, мемасы высмеивающие интеграционные тесты тоже есть: |
| 196 | + |
| 197 | +image::2026-02-05-11-06-52.png[] |
| 198 | + |
| 199 | +Тем не менее, я считаю, что этот прецедент — не повод смещать фокус на юнит-тесты и уж тем более не повод откзываться от тестирования через внешнее API. |
| 200 | + |
| 201 | +Теоретически, в следствии этого инцидента можно было бы ужесточить стратегию тестирования, добавив правило в духе "**каждый** метод длиннее двух строк". |
| 202 | +Но. |
| 203 | +Сколько времени уйдёт на написание, компиляцию, запуск и поддержку всех этих юнит-тестов? |
| 204 | +И насколько это повысит надёжность системы? |
| 205 | + |
| 206 | +В общем случае, на мой взгляд, овчинка выделки не стоит и по умолчанию в ЭП останется фокус на тестировании через внешнее API с минимально необходимым (на взгляд и совесть разработчика) количеством юнит-тестов. |
| 207 | + |
| 208 | +А эту историю, пожалуй, надо будет куда-то включить в качестве иллюстрации того, что критически важный код должен быть дополнительно покрыт юнит-тестами. |
0 commit comments