phorge-elasticsearch-modern.git - Phorge full-text search extension for Elasticsearch 7.x / 8.x / OpenSearch via the typeless API (mirror of https://github.com/vyos/phorge-elasticsearch-modern.git)

Age	Commit message (Collapse)	Author
2026-05-23	Engine: fix negative pagination bounds and _ts accumulation	Yuriy Andamasov
	Two correctness bugs surfaced by CR / Phase 0, both inherited from the bundled PhabricatorElasticFulltextStorageEngine pattern: 1. executeSearch() did not validate that $offset / $limit were non-negative. PHP's (int) cast on a negative string yields a negative int, which would be passed unchecked to Elasticsearch. Add an explicit non-negative check before the existing upper- bound (offset + limit > 10000) check. 2. reindexAbstractDocument()'s relationship-data loop assigned the '_ts' field unconditionally instead of accumulating. For multi- valued relationships (multiple authorPHID, projectPHID, etc. entries on the same doc), only the last timestamp survived. Change to array-append, parallel to the PHID array. Both fixes diverge from the bundled engine; bundled retains the faithful-but-broken behavior. Worth fixing in this extension since we control the wire format. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	Engine: revert abstract → plain class for production dispatch	Yuriy Andamasov
	A recent fixup cascade declared VyOSElasticModernFulltextStorageEngine as 'abstract class' to enable test-doubling subclasses (VyOSElasticModernHostTestEngine, VyOSElasticModernFulltextStorageEngineTestDouble). That change broke production dispatch: Phorge's PhutilClassMapQuery selects engines via PhutilSymbolLoader::loadObjects(), which calls setConcreteOnly(true) and unsets any class where reflection reports isAbstract(). The engine class would no longer be discoverable by cluster.search[].type:elasticsearch-modern, and the test-fixture subclasses live in src/__tests__/ which libphutil excludes from runtime class-map loading. Revert to plain 'class' (not 'final', not 'abstract'). Plain 'class' is discoverable by PhutilClassMapQuery; existing test fixtures keep working without rewrite; operators can subclass if they need to (harmless). Also update VyOSElasticModernHostTestCase::testPlaceholderEngineIsAbstract → testEngineIsConcreteForDispatch, inverting the assertion to document the correct invariant and its rationale (setConcreteOnly reference). All 22 tests pass. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	Address PR 16 CR findings	Yuriy Andamasov
	- README cluster.search example uses 'phabricator/' to match host default (was '/phabricator'; both work for index name but the host uses path verbatim for the base URI). - Add VERIFICATION.md placeholder with the planned smoke-test matrix template; status notes v0.1.0 is pending IS-474. - Engine reindex: replace 'if ($time)' falsy check with 'if ($time !== null)' so a Unix epoch timestamp (0) is not silently dropped. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	fix: correct Phase-0 findings from promotion PR review (3 issues)	Yuriy Andamasov
	1. TestDouble signatures: indexExists() and getIndexStats() in VyOSElasticModernFulltextStorageEngineTestDouble were missing the nullable host parameter added in E7/E8, causing a PHP fatal on 'arc unit --everything'. 2. buildSpec() zero-query sort bug: the falsy check (!$query->getParameter('query')) incorrectly treats the string '0' as empty (PHP falsy), enabling date-sorted path for a real search term. Changed to explicit null/empty-string checks to match the existing must-clause guard on line 316. All 22 unit tests now pass. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	fix: use correct HTTPFutureResponseStatus in indexExists() catch clause	Yuriy Andamasov
	Phase 0 CodeRabbit caught a typo — HTTPFutureHTTPResponseStatus does not exist in the libphutil type hierarchy; the correct class is HTTPFutureResponseStatus (consistent with line 257 in executeRequest()). 🤖 Generated by [robots](https://vyos.io)
2026-05-23	E8 fixup: four correctness fixes from Phase 0 review	Yuriy Andamasov
	1. strlen(\$query_string) → null-safe check \$query->getParameter('query') may return null; strlen(null) is deprecated in PHP 8.1+. Switch to explicit !== null && !== '' check. 2. ids.values nesting fix in buildSpec() 'values' => array(\$exclude) wraps an already-array \$exclude in a nested array, producing invalid Elasticsearch ids syntax. Cast to (array) then re-index with array_values() so both scalar PHID and array-of-PHIDs produce a flat list. 3. initIndex() host consistency: pass write host to indexExists() initIndex() called indexExists() without a host argument, which fell back to the read host, then deleted via getHostForWrite(). A read/write split lag window could cause false-positive "index doesn't exist" results. Pass the already-resolved write host. 4. getIndexStats() unchecked array access \$res['indices'][\$this->index] was accessed without verifying the key exists. If the index was just deleted or the name drifted, this throws an undefined-offset PHP notice. Add an explicit isset() guard with a clear exception message naming the index. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	E6-E8: Apply code-quality fixes from review	Yuriy Andamasov
	- Remove dead $fields assignment in buildSpec() (copy-paste artifact from bundled engine; never actually used). - Revert executeSearch catch from Throwable back to Exception. The earlier lint-driven widening was over-broad: Error subclasses (TypeError, etc.) should propagate immediately, not be aggregated into the per-host failover loop. Comment added explaining the intent. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	Engine: Wire executeSearch, index lifecycle, buildSpec	Yuriy Andamasov
	Full public surface lands. executeSearch normalizes \$types before both URL building and body filter construction. buildSpec uses the bundled PhabricatorElasticsearchQueryBuilder (final, by composition) and emits bool.must_not.ids for the exclude parameter from day one. buildSpec also injects the body-level documentType filter via buildTypeFilter. indexExists/indexIsSane/initIndex/getIndexStats mirror the bundled engine but without the version-branching dead code. The private check() and normalizeConfigValue() helpers are copied verbatim from the bundled engine (no behavior difference). catch (Exception) widened to catch (Throwable) per XHP132. normalizeConfigValue() one-liners braced per XHP24. Lint clean. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	E7 fixup: move didHealthCheck(true) to HTTP-success boundary in executeRequest()	Yuriy Andamasov
	Previously the success health-check was called inside the JSON-decode try block. This was wrong: a valid HTTP 200 response with non-JSON body (e.g. ES maintenance page) would mark the host as unhealthy because JSON parse failure triggered didHealthCheck(false). The host is demonstrably reachable and responding; only the application layer failed. Move didHealthCheck(true) to immediately after list($body) = resolvex() succeeds, before the GET/non-GET branch. Remove the now-unnecessary didHealthCheck(false) from the JSON catch block — only network/timeout failures belong in the health-check false path. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	Engine: Add reindexAbstractDocument() + executeRequest()	Yuriy Andamasov
	reindexAbstractDocument builds the document body with documentType as a real field (replacing the bundled engine's per-type URL segment), then PUTs to the typeless /_doc/{phid} URL via the new helper. executeRequest mirrors the bundled engine's HTTPSFuture pattern; it's private and not test-targeted directly (smoke tests in V1-V3 exercise the full wire path). 🤖 Generated by [robots](https://vyos.io)
2026-05-23	E6 fixup: use trim() + empty-string guard in setService(); use newEngine() ↵	Yuriy Andamasov
	in tests - str_replace('/', '', \$index) → trim(\$index, '/') to preserve internal slashes in multi-segment index paths (e.g. 'phabricator/prod' must stay intact; only leading/trailing slashes are stripped). - Add empty-string guard: after trim, throw a clear exception if the result is '' so misconfigured paths fail loudly at setService() rather than silently at query time with a confusing endpoint URL. - Replace direct new VyOSElasticModernFulltextStorageEngine() in the four new test methods with \$this->newEngine() to avoid PHP fatal on abstract class instantiation. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	E5 fixup: detect intra-key duplicates in buildIndexMappings(); add collision ↵	Yuriy Andamasov
	tests 🤖 Generated by [robots](https://vyos.io)
2026-05-23	E5 fixup: guard buildIndexMappings() against reserved field name collisions	Yuriy Andamasov
	🤖 Generated by [robots](https://vyos.io)
2026-05-23	Engine: Add buildIndexMappings() helper (single typeless mapping)	Yuriy Andamasov
	Single 'properties' block, documentType as a keyword field inside it, no include_in_all anywhere (the field was removed in ES 6). Relationships emit as keyword fields with doc_values: false, matching the bundled engine's ES-5-mode behavior. Field-data multi-analyzer shape (raw, keywords, stems) preserved. 🤖 Generated by [robots](https://vyos.io)
2026-05-23	Engine: Add buildTypeFilter() helper (body-level documentType filter)	Yuriy Andamasov
	🤖 Generated by [robots](https://vyos.io)
2026-05-23	Engine: Add getSearchUri() helper (typeless search endpoint)	Yuriy Andamasov
	🤖 Generated by [robots](https://vyos.io)
2026-05-23	Engine: Add getDocumentUri() helper for typeless document URLs	Yuriy Andamasov
	🤖 Generated by [robots](https://vyos.io)
2026-05-23	E1 fixup: initialize $version to null; getVersion() throws if unset; use ↵	Yuriy Andamasov
	TestDouble in tests 🤖 Generated by [robots](https://vyos.io)
2026-05-23	Engine: Add setVersion() with strict validation	Yuriy Andamasov
	Engine accepts only versions 7 and above (covering ES 7.x, 8.x, and OpenSearch 1.x/2.x/3.x via the typeless API). Anything below 7 raises with a clear message pointing at the bundled engine for ES 5 users. 🤖 Generated by [robots](https://vyos.io)
2026-05-22	H1: keep placeholder engine undiscoverable	copilot-swe-agent[bot]

2026-05-22	H1: Apply code-quality fixes from review	Yuriy Andamasov
	- Engine class marked final (matches host class declaration; no design intent to subclass). - testConfigOverrides now also asserts host and port (these were the most-likely-to-be-misconfigured fields; previously set but unverified). 🤖 Generated by [robots](https://vyos.io)
2026-05-22	Add VyOSElasticModernHost + placeholder engine stub	Yuriy Andamasov
	Host class mirrors PhabricatorElasticsearchHost's surface. Engine is a stub providing only getEngineIdentifier() and getHostType() so the host's tests can construct an engine instance; real engine implementation lands in subsequent commits. All six abstract methods of PhabricatorFulltextStorageEngine are implemented (four as throwing stubs to be replaced in E7/E8). .arcconfig gains "src/" in the load list so arc unit can discover the extension library; test case extends PhutilTestCase (no DB needed for pure-PHP host property assertions). 🤖 Generated by [robots](https://vyos.io)