Skip to content

Рефакторинг: дедупликация и декомпозиция god-классов с сохранением поведения + фикс багов/защиты у профилей#73

Merged
xtclovver merged 25 commits into
mainfrom
refactor/dedup-and-decomposition
Jun 11, 2026

Conversation

@xtclovver

@xtclovver xtclovver commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Что сделано

Поэтапный рефакторинг с сохранением поведения: поведение детекции, trust-модель маркетплейса и байты экспорта не изменены и закреплены тестами. Каждый коммит независимо проходил assembleDebug + test + lint.
Улучшение защиты функционала профилей и добавление ключей проверки для маркетплейса и профилей.

Страховочные тесты (написаны до рефакторинга)

  • vpn/: +44 характеризационных теста (инварианты VpnAppCatalog, корпус VpnDumpsysParser, VpnAppClassifier, краевые случаи VpnApkInspector/VpnAppMetadataScanner).
  • Golden-снапшоты трёх форматтеров экспорта (Markdown/JSON/debug, privacy on/off) + parity-тест секций: байты экспорта заморожены, актуальные выводы пишутся в build/export-golden-actual/ для перегенерации.
  • Round-trip тесты редактора кастомных проверок: несимметричный конфиг, дефолты saveAndExit, форк official-профиля.

Дедупликация

  • network/: единый источник паттернов имён интерфейсов (STACKED_BASE_INTERFACES — намеренное строгое подмножество, расхождение seth/dummy задокументировано и закреплено тестом).
  • probe/: строгие IP-валидаторы вынесены в IpLiterals; свободная эвристика PublicIpClient.looksLikeIp намеренно не тронута (извлечение IP из HTML mail.ru).
  • checker/: общий SignalOutcome; fetchBuiltinProvider — общий скелет пяти GeoIP-провайдеров; сканер-блоки BypassChecker вынесены в хелперы; publishOnReady свернул 11 одинаковых await/publish-блоков VpnCheckRunner.

Декомпозиция god-классов

  • SettingsCustomCheckEditorFragment: 1593 → 422 строки; секции редактора вынесены в customcheck/ui/editor/ (SectionBinder на секцию с симметричными bind/collect, общие EndpointPills/InlineEndpointSection).
  • MainActivity: 3358 → 3053 строки; таблица тайлов ui/main/CategoryTiles вместо восьми when-мапперов, MainExportController для экспортного флоу, дедупликация маппинга стадий загрузки. Рефлексивный MainActivityUiRenderingTest проходит без изменений.

Исправленные баги (найдены по ходу)

  • Гонка в ScanCancellationSignal.cancel: toList() на size==1 идёт через first() и бросал NoSuchElementException при конкурентном unregister() (проявлялось как флак отмены скана).
  • Гонка в CdnPullingCheckerTest: запись в обычный ArrayList из параллельных корутин.
  • Потеря {ip}-URL кастомных GeoIP-провайдеров в UrlSanitizer.
  • Локализация захардкоженного заголовка «## IP каналы» в Markdown-экспорте.

Не вошло (вынесено в отдельные задачи)

  • 8b: вынос render-методов MainActivity (~1300 строк) в ui/main/render/.

Верификация

./gradlew assembleDebug test lint — зелено на каждом коммите (JDK 21 / Android Studio JBR).

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

xtclovver added 17 commits June 5, 2026 01:49
Add VpnAppClassifierTest (marker priority, case sensitivity, go version
extraction) and extend VpnAppCatalogTest with catalog integrity
invariants, VpnDumpsysParserTest with parser corpus cases,
VpnApkInspectorTest with split-apk/path-edge cases,
VpnAppMetadataScannerTest with null-package/permission/format cases.
Tests pin current behavior before refactoring; no production changes.
fetchEndpoint stubs are invoked from parallel coroutines, but the tests
collected labels into a plain ArrayList, causing intermittent
ArrayIndexOutOfBoundsException. Use CopyOnWriteArrayList.
NetworkInterfaceNameNormalizer no longer redeclares its own base-pattern
list; it now uses NetworkInterfacePatterns.STACKED_BASE_INTERFACES, a
deliberate strict subset of STANDARD_INTERFACES (seth/dummy excluded to
keep v4- de-stacking behavior byte-identical). Pinned the divergence and
classification behavior in NetworkInterfaceNameNormalizerTest.
Move the strict charset+InetAddress validators verbatim out of
CdnPullingClient; the client keeps one-line delegates so all call sites
and tests stay untouched. PublicIpClient.looksLikeIp is deliberately NOT
unified: its loose heuristic is required for mail.ru HTML extraction
(now documented). Add IpLiteralsTest pinning edge cases including the
JDK quirk where IPv4-mapped IPv6 literals parse as Inet4Address.
- Share a single SignalOutcome data class instead of identical nested
  copies in DirectSignsChecker and IndirectSignsChecker.
- GeoIpChecker: extract fetchBuiltinProvider, the common
  fetch/validate/error skeleton of the five builtin providers; each
  provider keeps its own URL construction and field-path cascades.
- BypassChecker: extract startProxyScanAsync/startXrayScanAsync so
  check() is orchestration only; progress-callback wiring unchanged.

No thresholds, signals, finding order or result structures changed.
IndirectSigns/LocationSignals/CallTransport check() bodies were already
step-decomposed and are deliberately left as is.
…Ready

Replace 11 identical await -> throwIfCancelled -> publish wrapper blocks
with a generic publishOnReady helper using CheckUpdate constructor
references. Same supervisorScope receiver, same null contract, same
publish order; BypassProgress, IpConsensusReady and VerdictReady paths
untouched.
Pin the byte-exact output of all three CheckResult formatters (Markdown,
JSON, debug diagnostics) for the rich fixture, privacy mode on and off,
with UTC/Locale.US pinned and a fixed timestamp. Actual outputs are
dumped to build/export-golden-actual/ for easy regeneration after an
intentional format change. The parity test additionally asserts every
populated section appears in every formatter, guarding the three
independent traversals against silent drift.

No shared traversal/visitor is introduced on purpose: the formatters
diverge deliberately (separators, masking policy, flag emission), so
unification would change export bytes.
Allow-list app/src/test/resources/export/golden/*.md so golden
regeneration keeps working.
Round-trip test (non-default config in every section survives open-save
unchanged), defaults test (untouched new profile saves the exact literal
defaults from saveAndExit), and official-fork test (saving a profile
whose official flag survived the storage trust chain produces a new id,
an edited-suffix name, cleared marketplaceInfo and sourceProfileId).
SettingsCustomCheckEditorFragment (1593 -> 422 lines) now delegates each
checker section to a SectionBinder in customcheck/ui/editor/: bind()
loads the profile slice into the body views, collect() reconstructs it
with the exact legacy defaults, summary() renders the collapsed line.
Shared plumbing extracted alongside: EndpointPills (pill rows),
InlineEndpointSection (add/edit wiring for the three
InlineEndpointEditorController-backed sections), EditorSectionBinders
(visual order). Reachability domains moved into their binder; the
generic custom-domain list, section accordion state, dependency
enforcement and the save/fork flow stay in the fragment.

Read/write semantics are pinned by SettingsCustomCheckEditorRoundTripTest
(previous commit) and were not changed. Dead makeDialogContainer/
makeDialogEdit helpers dropped.
ui/main/CategoryTiles declares the twelve category tiles (id, header
title/icon resources, per-tile view ids) in accordion order; MainActivity
category constants now alias the table ids and setupCategoryAccordion/
createTileHolder consume TileSpec directly. Removes the eight mechanical
when-mappers (~130 lines). Tile behavior, the tiles field and TileHolder
member names are unchanged (MainActivityUiRenderingTest reflection net
passes unmodified).
ui/main/MainExportController owns the format/action dialogs, the two
CreateDocument launchers, clipboard copy and document writing, moved
verbatim from MainActivity. The controller is constructed as an activity
field initializer, preserving the pre-STARTED launcher registration
timing; the export snapshot and the debug-clipboard toggle stay in the
activity and are supplied via lambdas.
callbacks.entries.toList() takes the size==1 fast path through first(),
which throws NoSuchElementException when a checker unregisters its
callback concurrently. Snapshot through the concurrent map iterator
instead; invocation-after-clear ordering and the register() double-check
contract are unchanged. Surfaced as an intermittent failure of
VpnCheckRunnerTest cancelled-execution test.
showLoadingCardForStage and markLoadingStagesCancelled repeated the same
stage -> card/icon/status/findings/info-section bundle twice; both now
share a single categoryViewsForStage table. bindCardStoppedState mirrors
bindCardLoadingState, removing the four copies of the stopped-state
icon/status block. Per-stage behavior is unchanged.

Deliberate deviation from the original phase plan: a separate
LoadingStageCoordinator class would have needed ~10 injected callbacks
for the same effect; in-place deduplication wins on risk and size.
@xtclovver xtclovver added the enhancement New feature or request label Jun 11, 2026
…nd trip

UrlSanitizer.sanitizeHttpsUrl runs the URL through java.net.URI, which
throws on the illegal { } chars, so any custom GeoIP provider whose URL
embeds the documented {ip} placeholder was silently dropped on every
profile save/load. Add sanitizeGeoIpProviderUrl, which validates a copy
with {ip} replaced by a public IP literal (8.8.8.8) and returns the
original string when it passes. Wire deserializeGeoIp to it.

The {ip} substitution only exists in GeoIpChecker.fetchCustomProvider;
IpComparison/CdnPulling send URLs verbatim, so they keep the generic
sanitizer. canonicalHash semantics unchanged.
@xtclovver xtclovver changed the title Рефакторинг: дедупликация и декомпозиция god-классов с сохранением поведения Рефакторинг: дедупликация и декомпозиция god-классов с сохранением поведения + фикс багов/защиты у профилей Jun 11, 2026
CheckResultMarkdownExportFormatter hardcoded the Russian '## IP каналы' section header, leaking Russian into en/fa/zh exports. Use the existing R.string.ip_channels_title resource via the Context the formatter already receives. Goldens regenerated (US locale -> 'IP channels').
Replace hardcoded RU strings in CheckResultMarkdownExportFormatter with
context.getString lookups so en/fa/zh exports no longer leak Russian:
- IP-channels table column headers (Channel/Country/Sources)
- "Flags:" prefix
- operator-whitelist section (title, Detected yes/no, control suffix,
  Duration label/ms unit, available/unavailable)

Add export_* string resources to all four locales (en/ru/fa/zh-rCN) and
drop the object-level AVAILABLE/UNAVAILABLE consts. Regenerate export
golden snapshots (now render under Locale.US in English) and update the
operator-whitelist unit-test assertions accordingly.
@sonarqubecloud

Copy link
Copy Markdown

@xtclovver xtclovver merged commit dfb46b7 into main Jun 11, 2026
3 checks passed
xtclovver added a commit that referenced this pull request Jun 11, 2026
Подготовка релиза v2.9.0 (versionCode 20900).

- Добавлены fastlane changelog 20900.txt для en-US/ru-RU/fa/zh-CN,
  охватывающие коммиты #73/#74/#75 (6 детекторов VPN/прокси,
  детектор эмулятора/изоляции, профили маркетплейса, Ed25519-подпись
  каталога, фиксы).
- README (все 4 локали) синхронизированы с новыми детекторами:
  дерево архитектуры, паттерны интерфейсов (4.2), BypassChecker
  (Clash API + SOCKS5-auth, 6.4), NativeSignsChecker (TUN/TAP по типу,
  host-route /32, эмулятор, изоляция, 12.1-12.4), тип VPN-транспорта (3.1).
- build.gradle.kts: versionCode 20802 -> 20900, versionName 2.8.2 -> 2.9.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant