Expose Surface for use with OpenStreetmap Module et al.

@tbcs, thanks for raising this, and yes, let's collaborate:)

I agree the functionality overlaps, and I think the split you suggest is the right one. openstreetmap owns the content model and the entities the data becomes. overpass_api owns the query layer: building a query, running it, and getting the result back. Those are two jobs, and today each module does a bit of the other's.

If openstreetmap depended on overpass_api for talking to an interpreter, you would lose the Overpass code you maintain today and this module would gain a real consumer. That seems worth more to both of us than either module growing towards the other.

Two of my open issues touch this and I would rather decide them with you than around you.

#3614000 adds a config entity for saving and exporting queries. That sits in the query layer, so it stays here, but its shape should suit what Query Tools needs rather than only what my own UI needs. If you can say what your presets carry today, I will build against that.

#3614002 creates Feeds sources from the query builder to import OSM data. On the split above that is the content side, so it belongs with OpenStreetMap Module rather than with this one. There is a difference worth naming though: as I read it, openstreetmap imports into OSM nodes, whereas what I had in mind targets any entity type, through Feeds. A term, a taxonomy of amenities, a custom entity for a municipal asset register: the OSM data is the source, and what it becomes is the site builder's decision rather than the module's.

If that direction interests you, I would rather move it in the OpenStreetMap Module's issue queue than keep it in this one, and work on it there.

I should also ask murz, who owns overpass_api, since a dependency in this direction is a commitment on this module's API and that is his call as much as mine.

What would be the most useful next step from your side? I can look at what overpass_api would need to expose for Query Tools to sit on top of it, if that helps.

@tbcs, I'll update here in a week. Thanks for your commits and messages too:)

@tbcs, I've just checked your bug fixes, thanks for the effort. I'm still going through the other parts of the MR.

@tbcs, thanks again for this, and sorry it took a while. I've now been through all ten commits.

The bug fixes are clearly welcome: remark errors reported with HTTP 200, ConnectException not being caught under Guzzle 7, the unused headers and injected client in rawQuery(), the bare ; for an empty filter, the parenthesised {{bbox}}, and Utils::returnBytes() on -1. Thank you for tracking those down.

Splitting the MR

Since this issue is now about the API surface, I'd like the MR to carry only that, and the rest to move to the issues it belongs to, so each part can be reviewed and land on its own. Concretely:

• Here (#3618910: Expose Surface for use with OpenStreetmap Module et al.): the first two commits (interface, result object, exceptions; batching and streaming), plus the API-side files from "Hardening" and "Fix the preset duplicate form": OverpassApi.php, OverpassApiInterface.php, OverpassApiHttpException.php, Utils.php and their tests.
• #3614000: Add overpass_api_preset submodule: Provide a Config Entity to save, reuse, and export Overpass queries: the preset entity and executor, squashed with "Land the overpass_api_ui half of the query builder refactor" (the branch is broken between the two), plus the preset route, list operation and duplicate form fix. I'd like the entity in an overpass_api_preset submodule, as that issue describes; more on that there.
• #3614002: Add overpass_api_feeds submodule: Allow creating Feeds sources directly from Query Builder (overpass_api_ui) to import OSM data: the Feeds fetcher and its tests. I agree with your split: fetcher here, parser in openstreetmap. One question about that on #3614002: Add overpass_api_feeds submodule: Allow creating Feeds sources directly from Query Builder (overpass_api_ui) to import OSM data.
• [#NEW-HARDENING]: service injection in SettingsForm and QueryBuilderForm, the lower install defaults, and "Fix the settings form fataling on Drupal 11" squashed into it. Released 1.0.x builds the form through autowiring and works on D11; the fatal comes from the new constructor in "Hardening", so the two belong together.
• [#NEW-PACKAGING]: the packaging commit, the Extend page grouping, and the removal of tests/src/PHPCodeSniffer/Standard.php.

Your authorship stays on all of it, of course.

Requests for the API part

1. OverpassApiInterface::query(): please document object[] only. The deprecated returnRaw branch can stay in OverpassApi until 2.0.0 (osm_localities still uses it), but I'd rather not put a deprecated flag into the interface contract.
2. OverpassResultInterface::getElements(): please state that elements are returned as Overpass sent them, untransformed. Some consumers build on the raw elements directly (for example a way's nodes together with out geom), and I'd like that to be part of the contract rather than an implementation detail.
3. getSlotAvailability(): could it return ?int instead of bool|int? 0 for a free slot, the seconds to wait otherwise, NULL when the status report cannot be read. This is more than style: in sendQuery(), if ($slotFreeTimeout !== TRUE) { $this->lock->acquire($this->lockName, $slotFreeTimeout); } passes FALSE to the lock, DatabaseLockBackend::acquire() clamps it to 1 ms, and an unreadable status is silently treated as a free slot. The same code is in 1.0.x today, so it isn't something you introduced, but the interface is the right moment to fix it: with NULL the caller has to pick a fallback, lock_time for instance.
4. halaxa/json-machine is in require, so the hook_requirements() check for it can go. Keeping it in require is fine with me.

Release

My plan is to ship the API part as 1.0.0-beta5, with release notes for the three behaviour changes (remark errors now throw OverpassApiRuntimeException, OverpassApi::__construct() takes the lock backend, returnRaw is deprecated), and then go to 1.0.0 on the new interface rather than open a 2.0.x branch.

@murz, are you OK with breaking these in a beta? osm_localities is the consumer I know of: its main sync uses returnRaw and keeps working, but OsmSyncBatch catches only OverpassApiHttpException, so a timed-out count query would now surface as OverpassApiRuntimeException. Catching OverpassApiException there would cover both.

For reference, here is where each part of MR !9 would go. Commits are named by their short hash in the MR.

Landing order: this issue first. Then presets, form hardening and packaging, in any order. The Feeds fetcher after presets, since it depends on them.

@tbcs, thank you for the quick re-cut and for opening #3630442: Inject services into SettingsForm and QueryBuilderForm, and lower the retry defaults, #3630443: Move UI dependencies to suggest, default to the https endpoint, and add CI and #3630444: Polygon areas for presets. All four requests are in, and the lock_time fallback for an unreadable status report is exactly what I hoped for.

I checked the remark detection against live Overpass answers: "runtime error: Query run out of memory using about 2048 MB of RAM." and "runtime error: Query ran out of memory in "query" at line 1. It would need at least 32 MB of RAM to continue." both come back as HTTP 200 with no elements, and both are caught by the pattern. An area that does not exist returns an empty result with no remark, which correctly is not an error.

I pushed one commit on top, because it is about the contract this MR establishes: "Wrap every transport failure in OverpassApiException". The interface says the service throws OverpassApiException on an unrecoverable transport error, but three paths let a raw Guzzle exception through:

• the fail-fast branch of sendQuery() rethrew the Guzzle exception as is, so a 400 (an Overpass QL parse error) arrived as ClientException;
• a ConnectException still failing on the last attempt was rethrown raw;
• getStatus() had no catch at all, and with use_slots_timeouts it runs inside sendQuery().

They now go through a small wrapTransportException(): OverpassApiHttpException when there is a response, otherwise OverpassApiException with the Guzzle exception as previous. testRawQueryDoesNotRetryBadRequest() asserts the type now, and two new tests cover an exhausted connect failure and getStatus(). All 54 unit tests pass with PHPUnit 11, and the new ones fail against the previous head. Please have a look and tell me if you'd rather shape it differently.

One housekeeping point: the MR title and description still describe the original ten commits (presets, Feeds fetcher, packaging, overpass_api.preset.* config dependencies). Could you update them to the API surface only? I'll also tidy the stray capitals in two of the commit messages ("paSs", "boDy", "untransformeD", "lonGer") in the squash message when merging.

If nothing else comes up, I'll merge this in a few days and release it as 1.0.0-beta5, with release notes for the behaviour changes: remark errors are now thrown as OverpassApiRuntimeException, transport errors always arrive as OverpassApiException, OverpassApi::__construct() takes the lock backend, and returnRaw is deprecated. osm_localities keeps working; the one visible change there is that a timed-out count query now surfaces as OverpassApiRuntimeException.