diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-08-31 00:15:52 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-08-31 00:15:52 +0900 |
| commit | 6a6ec1b8f8a5c70d21f3772ce637b763e8ab21ea (patch) | |
| tree | 5a23dda75a0a8eb39478b77da82cf582e21a1d2c | |
| parent | dc1f030e3904677cd65eac0a90c1d850e0ad1bbf (diff) | |
| download | php-shirabe-6a6ec1b8f8a5c70d21f3772ce637b763e8ab21ea.tar.gz php-shirabe-6a6ec1b8f8a5c70d21f3772ce637b763e8ab21ea.tar.zst php-shirabe-6a6ec1b8f8a5c70d21f3772ce637b763e8ab21ea.zip | |
feat(plugin): carry Http\Response across as a materialized value
A response is built for one request and the graph never retains it, so there
is no entity for a handle to point at. The child holds a real instance
instead, revived from the object record the wire carries, and collect() frees
the copy each world holds — which is what that method is for. The
value-object rule rejects the class only because collect() assigns to $this,
so the category comes from an overrides.list entry.
HttpDownloader::get() and copy() answer with one.
Two gaps stay: decodeJson() reaches Composer\Json\JsonFile, which a guard
shadows, and Composer answers a curl request with the CurlResponse subclass
where this port flattens the value into a Response.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
12 files changed, 218 insertions, 71 deletions
diff --git a/crates/shirabe-php-rpc/php/guards/Composer/Util/Http/CurlResponse.php b/crates/shirabe-php-rpc/php/guards/Composer/Util/Http/CurlResponse.php index ae229589..ffb000f3 100644 --- a/crates/shirabe-php-rpc/php/guards/Composer/Util/Http/CurlResponse.php +++ b/crates/shirabe-php-rpc/php/guards/Composer/Util/Http/CurlResponse.php @@ -19,4 +19,44 @@ class CurlResponse extends Response { \ShirabeUnsupportedClass::fail(self::class, 'getCurlInfo'); } + + public function getStatusCode(): int + { + \ShirabeUnsupportedClass::fail(self::class, 'getStatusCode'); + } + + public function getStatusMessage(): ?string + { + \ShirabeUnsupportedClass::fail(self::class, 'getStatusMessage'); + } + + public function getHeaders(): array + { + \ShirabeUnsupportedClass::fail(self::class, 'getHeaders'); + } + + public function getHeader(string $name): ?string + { + \ShirabeUnsupportedClass::fail(self::class, 'getHeader'); + } + + public function getBody(): ?string + { + \ShirabeUnsupportedClass::fail(self::class, 'getBody'); + } + + public function decodeJson() + { + \ShirabeUnsupportedClass::fail(self::class, 'decodeJson'); + } + + public function collect(): void + { + \ShirabeUnsupportedClass::fail(self::class, 'collect'); + } + + public static function findHeaderValue(array $headers, string $name): ?string + { + \ShirabeUnsupportedClass::fail(self::class, 'findHeaderValue'); + } } diff --git a/crates/shirabe-php-rpc/php/guards/Composer/Util/Http/Response.php b/crates/shirabe-php-rpc/php/guards/Composer/Util/Http/Response.php deleted file mode 100644 index 61eebb7f..00000000 --- a/crates/shirabe-php-rpc/php/guards/Composer/Util/Http/Response.php +++ /dev/null @@ -1,57 +0,0 @@ -<?php - -// Generated by scripts/plugin-stub-generator; do not edit by hand. -// Guard for Composer\Util\Http\Response. -// The Rust side owns this class and the worker has no proxy for it, so this -// declaration shadows the real one: the constants and the hierarchy stay, while -// constructing it or calling anything on it raises an explicit error. - -namespace Composer\Util\Http; - -class Response -{ - public function __construct(array $request, ?int $code, array $headers, ?string $body) - { - \ShirabeUnsupportedClass::fail(self::class, '__construct'); - } - - public function getStatusCode(): int - { - \ShirabeUnsupportedClass::fail(self::class, 'getStatusCode'); - } - - public function getStatusMessage(): ?string - { - \ShirabeUnsupportedClass::fail(self::class, 'getStatusMessage'); - } - - public function getHeaders(): array - { - \ShirabeUnsupportedClass::fail(self::class, 'getHeaders'); - } - - public function getHeader(string $name): ?string - { - \ShirabeUnsupportedClass::fail(self::class, 'getHeader'); - } - - public function getBody(): ?string - { - \ShirabeUnsupportedClass::fail(self::class, 'getBody'); - } - - public function decodeJson() - { - \ShirabeUnsupportedClass::fail(self::class, 'decodeJson'); - } - - public function collect(): void - { - \ShirabeUnsupportedClass::fail(self::class, 'collect'); - } - - public static function findHeaderValue(array $headers, string $name): ?string - { - \ShirabeUnsupportedClass::fail(self::class, 'findHeaderValue'); - } -} diff --git a/crates/shirabe-php-rpc/php/runtime/Shirabe/MaterializedValue.php b/crates/shirabe-php-rpc/php/runtime/Shirabe/MaterializedValue.php index c16b4e18..73a12033 100644 --- a/crates/shirabe-php-rpc/php/runtime/Shirabe/MaterializedValue.php +++ b/crates/shirabe-php-rpc/php/runtime/Shirabe/MaterializedValue.php @@ -17,6 +17,7 @@ use Composer\Semver\Constraint\Constraint; use Composer\Semver\Constraint\MatchAllConstraint; use Composer\Semver\Constraint\MatchNoneConstraint; use Composer\Semver\Constraint\MultiConstraint; +use Composer\Util\Http\Response; final class MaterializedValue { @@ -26,6 +27,7 @@ final class MaterializedValue MultiConstraint::class, MatchAllConstraint::class, MatchNoneConstraint::class, + Response::class, \DateTimeImmutable::class, \DateTime::class, ]; diff --git a/crates/shirabe-php-rpc/src/value.rs b/crates/shirabe-php-rpc/src/value.rs index 7203c9fb..a3e14d45 100644 --- a/crates/shirabe-php-rpc/src/value.rs +++ b/crates/shirabe-php-rpc/src/value.rs @@ -70,6 +70,18 @@ impl PhpObject { pub fn set_protected(&mut self, name: &str, value: PluginValue) { self.props.insert(protected_key(name), value); } + + /// PHP mangles a private property name to `\0<declaring class>\0name`. The declaring class is + /// the one whose body holds the `private` declaration, which is not `self.class` once a + /// subclass inherits it. + pub fn private(&self, declaring_class: &str, name: &str) -> Option<&PluginValue> { + self.props + .get(private_key(declaring_class, name).as_slice()) + } + + pub fn set_private(&mut self, declaring_class: &str, name: &str, value: PluginValue) { + self.props.insert(private_key(declaring_class, name), value); + } } fn protected_key(name: &str) -> Vec<u8> { @@ -78,6 +90,14 @@ fn protected_key(name: &str) -> Vec<u8> { key } +fn private_key(declaring_class: &str, name: &str) -> Vec<u8> { + let mut key = vec![0]; + key.extend_from_slice(declaring_class.as_bytes()); + key.push(0); + key.extend_from_slice(name.as_bytes()); + key +} + /// The value model of the plugin RPC boundary: PHP scalars, arrays, object records, and handle /// descriptors. /// @@ -795,6 +815,19 @@ mod tests { b"O:45:\"Composer\\Semver\\Constraint\\MatchAllConstraint\":1:{s:15:\"\0*\0prettyString\";N;}".as_slice(), ); + // A private property carries the declaring class rather than `*`, which is what lets a + // subclass hold its own property of the same name. + let mut response = PhpObject::new("Composer\\Util\\Http\\Response"); + response.set_private( + "Composer\\Util\\Http\\Response", + "code", + PluginValue::Int(200), + ); + assert_eq!( + serialize(&PluginValue::PhpObject(response)), + b"O:27:\"Composer\\Util\\Http\\Response\":1:{s:33:\"\0Composer\\Util\\Http\\Response\0code\";i:200;}".as_slice(), + ); + let mut date = PhpObject::new("DateTimeImmutable"); date.set_public("date", PluginValue::string("2026-08-07 12:34:56.123456")); date.set_public("timezone_type", PluginValue::Int(3)); @@ -819,6 +852,20 @@ mod tests { assert_eq!(inner.protected("operator"), Some(&PluginValue::Int(4))); assert_eq!(inner.public("operator"), None); assert_eq!(outer.protected("prettyConstraint"), None); + + let mut response = PhpObject::new("Composer\\Util\\Http\\Response"); + response.set_private( + "Composer\\Util\\Http\\Response", + "body", + PluginValue::string("{}"), + ); + roundtrip(PluginValue::PhpObject(response.clone())); + assert_eq!( + response.private("Composer\\Util\\Http\\Response", "body"), + Some(&PluginValue::string("{}")) + ); + assert_eq!(response.protected("body"), None); + assert_eq!(response.public("body"), None); } /// PHP numbers every value of a payload, including the ones inside an object and the diff --git a/crates/shirabe/src/plugin/php_plugin_proxy.rs b/crates/shirabe/src/plugin/php_plugin_proxy.rs index 491a320c..c966897b 100644 --- a/crates/shirabe/src/plugin/php_plugin_proxy.rs +++ b/crates/shirabe/src/plugin/php_plugin_proxy.rs @@ -21,7 +21,7 @@ use crate::package::{DisplayMode, PackageInterfaceHandle}; use crate::plugin::capability::{Capability, CommandProvider}; use crate::plugin::capable::Capable; use crate::plugin::php_plugin_value::{ - date_time_from_wire, date_time_to_wire, link_from_wire, link_to_wire, + date_time_from_wire, date_time_to_wire, link_from_wire, link_to_wire, response_to_wire, }; use crate::plugin::plugin_interface::PluginInterface; use crate::repository::{ @@ -1177,11 +1177,27 @@ fn dispatch_http_downloader_method( downloader.borrow_mut().set_options(options); Ok(PluginValue::Null) } - // TODO(plugin): a `Composer\Util\Http\Response` has no representation on the wire, so - // the two synchronous request methods have nothing to answer with. - "get" | "copy" => Err(runtime_throw(format!( - "Shirabe does not support HttpDownloader::{method_name}() from a plugin yet" - ))), + "get" => { + let url = arg::<String>(method_name, args, 0)?; + let options = + arg_or::<IndexMap<String, PhpMixed>>(method_name, args, 1, IndexMap::new())?; + let response = downloader + .borrow() + .get(&url, options) + .map_err(|error| error_throw("get failed", &error))?; + Ok(response_to_wire(&response)) + } + "copy" => { + let url = arg::<String>(method_name, args, 0)?; + let to = arg::<String>(method_name, args, 1)?; + let options = + arg_or::<IndexMap<String, PhpMixed>>(method_name, args, 2, IndexMap::new())?; + let response = downloader + .borrow() + .copy(&url, &to, options) + .map_err(|error| error_throw("copy failed", &error))?; + Ok(response_to_wire(&response)) + } // TODO(plugin,async): the async surface resolves its promises with a Response the wire // cannot carry, and driving it needs a promise representation that crosses the boundary // unresolved. Neither exists yet. diff --git a/crates/shirabe/src/plugin/php_plugin_value.rs b/crates/shirabe/src/plugin/php_plugin_value.rs index cb9b990e..80415eef 100644 --- a/crates/shirabe/src/plugin/php_plugin_value.rs +++ b/crates/shirabe/src/plugin/php_plugin_value.rs @@ -14,6 +14,7 @@ //! constraint) has no faithful constructor call. use crate::package::Link; +use crate::util::http::Response; use chrono::{DateTime, NaiveDateTime, TimeZone, Utc}; use shirabe_php_rpc::{PhpObject, PhpThrow, PluginValue}; use shirabe_semver::constraint::{ @@ -21,6 +22,7 @@ use shirabe_semver::constraint::{ }; const LINK_CLASS: &str = "Composer\\Package\\Link"; +const RESPONSE_CLASS: &str = "Composer\\Util\\Http\\Response"; const CONSTRAINT_CLASS: &str = "Composer\\Semver\\Constraint\\Constraint"; const MULTI_CONSTRAINT_CLASS: &str = "Composer\\Semver\\Constraint\\MultiConstraint"; const MATCH_ALL_CLASS: &str = "Composer\\Semver\\Constraint\\MatchAllConstraint"; @@ -197,6 +199,48 @@ fn constraint_from_wire(value: &PluginValue) -> Result<AnyConstraint, PhpThrow> }) } +/// A `Response` is built for one request and never retained by the object graph, so the child +/// holds a real instance rather than a handle, and `collect()` frees the copy it holds. +/// +/// TODO(type-model): PHP's `$request` is the whole request array (`url`, `options`, `copyTo`) +/// and this port keeps only the url, which is all `decodeJson()` reads back out of it. +/// +/// TODO(port): Composer answers a curl request with a `Composer\Util\Http\CurlResponse` +/// carrying the transfer info; this port flattens that subclass into `Response` before the +/// value leaves `HttpDownloader`, so `getCurlInfo()` is gone and `get_class()` differs. +pub(crate) fn response_to_wire(response: &Response) -> PluginValue { + let mut object = PhpObject::new(RESPONSE_CLASS); + let mut request = indexmap::IndexMap::new(); + request.insert(b"url".to_vec(), PluginValue::string(response.request_url())); + object.set_private(RESPONSE_CLASS, "request", PluginValue::Array(request)); + object.set_private( + RESPONSE_CLASS, + "code", + PluginValue::Int(response.get_status_code()), + ); + object.set_private( + RESPONSE_CLASS, + "headers", + PluginValue::List( + response + .get_headers() + .iter() + .cloned() + .map(PluginValue::string) + .collect(), + ), + ); + object.set_private( + RESPONSE_CLASS, + "body", + match response.get_body() { + Some(body) => PluginValue::string(body), + None => PluginValue::Null, + }, + ); + PluginValue::PhpObject(object) +} + pub(crate) fn link_to_wire(link: &Link) -> PluginValue { let mut object = PhpObject::new(LINK_CLASS); object.set_protected("source", PluginValue::string(link.get_source())); diff --git a/crates/shirabe/src/util/http/response.rs b/crates/shirabe/src/util/http/response.rs index 4f824baa..d505bde4 100644 --- a/crates/shirabe/src/util/http/response.rs +++ b/crates/shirabe/src/util/http/response.rs @@ -21,6 +21,13 @@ impl Response { } } + /// The url of the request this response answered. PHP keeps the whole request array in a + /// private property with no getter, and the plugin boundary codec needs the url out of it to + /// rebuild the value in the child. + pub(crate) fn request_url(&self) -> &str { + &self.url + } + pub fn get_status_code(&self) -> i64 { self.code } diff --git a/crates/shirabe/tests/plugin/e2e_http_downloader_test.rs b/crates/shirabe/tests/plugin/e2e_http_downloader_test.rs index ecb0b34c..42471b61 100644 --- a/crates/shirabe/tests/plugin/e2e_http_downloader_test.rs +++ b/crates/shirabe/tests/plugin/e2e_http_downloader_test.rs @@ -61,7 +61,8 @@ fn test_plugin_owned_http_downloader_matches_upstream_composer() { // Pinned as well as compared, so a run where neither side wrote a trace cannot pass. The // second options line is the evidence that both worlds merge into one value rather than each - // holding its own copy of the map. + // holding its own copy of the map, and the response lines are the evidence that the value the + // request answers with is a real instance of the class rather than something shaped like one. assert_eq!( "\ event=post-update-cmd @@ -71,6 +72,11 @@ options merged=[\"X-Probe: 2\"] isCurlEnabled=true hints other=null transport=null outputWarnings=ok +get=ok +response class=\"Composer\\\\Util\\\\Http\\\\Response\" body=\"{\\\"probe\\\":true,\\\"n\\\":42}\" headers=[] +getHeader=null +copy=ok file=\"{\\\"probe\\\":true,\\\"n\\\":42}\" +collect=ok ", upstream.trace ); diff --git a/crates/shirabe/tests/plugin/fixtures/e2e-http-downloader/plugin/src/Plugin.php b/crates/shirabe/tests/plugin/fixtures/e2e-http-downloader/plugin/src/Plugin.php index 504c2108..054a47ca 100644 --- a/crates/shirabe/tests/plugin/fixtures/e2e-http-downloader/plugin/src/Plugin.php +++ b/crates/shirabe/tests/plugin/fixtures/e2e-http-downloader/plugin/src/Plugin.php @@ -78,6 +78,32 @@ class Plugin implements PluginInterface, EventSubscriberInterface ); }); + // A file:// url keeps the probe offline and off the curl path, so the trace is the same + // on a machine with no network. Paths stay out of the trace: the two runs work in + // different temporary directories. + $payload = getcwd() . '/probe-payload.json'; + file_put_contents($payload, '{"probe":true,"n":42}'); + + $response = null; + $lines[] = 'get=' . $this->describe(static function () use ($downloader, $payload, &$response): void { + $response = $downloader->get('file://' . $payload); + }); + $lines[] = 'response class=' . json_encode($response === null ? null : \get_class($response)) + . ' body=' . json_encode($response === null ? null : $response->getBody()) + . ' headers=' . json_encode($response === null ? null : $response->getHeaders()); + $lines[] = 'getHeader=' . json_encode($response === null ? null : $response->getHeader('Content-Type')); + + $target = getcwd() . '/probe-copy.json'; + $lines[] = 'copy=' . $this->describe(static function () use ($downloader, $payload, $target): void { + $downloader->copy('file://' . $payload, $target); + }) . ' file=' . json_encode(@file_get_contents($target)); + + // collect() unsets the response's own properties, so the object is spent afterwards and + // nothing may read it again. + $lines[] = 'collect=' . $this->describe(static function () use ($response): void { + $response->collect(); + }); + file_put_contents('http-downloader-trace.txt', implode("\n", $lines) . "\n"); } diff --git a/docs/dev/plugin-class-classification.md b/docs/dev/plugin-class-classification.md index 6f52bd62..a383f7c6 100644 --- a/docs/dev/plugin-class-classification.md +++ b/docs/dev/plugin-class-classification.md @@ -66,8 +66,19 @@ revives a genuine instance of the real class without running a constructor, and getters answer locally with zero round-trips. Identity is not preserved — two calls of the same getter yield two objects in the child (see `docs/dev/php-rpc.md`). Only the classes on the codec's closed list cross -this way today (`Link` and the `composer/semver` constraints it holds); the -rest of the category has no artifact yet and is guarded. +this way today (`Link` and the `composer/semver` constraints it holds, and +`Composer\Util\Http\Response`); the rest of the category has no artifact +yet and is guarded. + +`Response` is on the list by `overrides.list` entry rather than by the +value-object rule, which rejects it because `collect()` assigns to `$this`. +The entity it would otherwise need does not exist: a response is built for +one request and the graph never retains it, and `collect()` frees the copy +each world holds, which is what that method is for. Two of its members do +not work from the child even so: `decodeJson()` reaches +`Composer\Json\JsonFile`, which a guard shadows, and Composer answers a +curl request with the `CurlResponse` subclass where this port flattens the +value into a `Response` before it leaves `HttpDownloader`. #### contract @@ -403,11 +414,11 @@ note). its options, its TLS defaults and the authentication it collects into the run's IO are state the two worlds have to share, and a plugin-`new`ed one allocates a Rust-side entity rather than a second downloader the graph knows -nothing about. What it does not serve is the request surface itself — -`get()` and `copy()` have no wire representation for the -`Composer\Util\Http\Response` they return, and -`add()`/`addCopy()`/`wait()`/`enableAsync()`/`countActiveJobs()` resolve -promises with one. +nothing about. `get()` and `copy()` answer with a +`Composer\Util\Http\Response` the child holds as a value (see below); the +async surface — `add()`, `addCopy()`, `wait()`, `enableAsync()`, +`countActiveJobs()` — is still an explicit error, because driving it needs a +promise representation that crosses the boundary unresolved. `Loop` remains guarded, so `Composer::getLoop()` is still an explicit error and neither the graph's own executor nor its downloader is reachable through diff --git a/scripts/plugin-class-classifier/lists/overrides.list b/scripts/plugin-class-classifier/lists/overrides.list index 099578e0..69084d44 100644 --- a/scripts/plugin-class-classifier/lists/overrides.list +++ b/scripts/plugin-class-classifier/lists/overrides.list @@ -5,3 +5,4 @@ Composer\Autoload\ClassLoader php-native the child bootstraps via the real vendo Composer\InstalledVersions php-native same bootstrap constraint as ClassLoader (real file in vendor/composer is always loaded); its static $installed is genuinely shared state — Rust must push a reload after each install dump (see static-state.list needs-sync) Composer\Platform\HhvmDetector php-native stateless environment probe; only reachable as a consumed ctor param of PlatformRepository, no Rust-owned instance is ever provided to plugins. Plugin-custom detectors passed into a proxied PlatformRepository become an explicit error. Composer\Package\Version\VersionGuesser php-native stateless computation over VCS output; only reachable as a consumed ctor param of RootPackageLoader, no Rust-owned instance is ever provided to plugins. Plugin-custom guessers passed into proxied ctors become an explicit error. +Composer\Util\Http\Response rust-snapshot built for one request and never retained by the graph, so there is no entity to point at; the value-object rule rejects it only because collect() assigns to $this, and freeing the copy each world holds is exactly what that method is for diff --git a/scripts/plugin-stub-generator/guard-exemptions.list b/scripts/plugin-stub-generator/guard-exemptions.list index 90adcb20..7ca14fd9 100644 --- a/scripts/plugin-stub-generator/guard-exemptions.list +++ b/scripts/plugin-stub-generator/guard-exemptions.list @@ -12,3 +12,7 @@ Composer\Package\Link # the dual-mode Composer\EventDispatcher\Event (php/runtime/) carries a natively constructed event # to the Rust side as a P-table entity. Composer\Plugin\PreCommandRunEvent + +# The wire codec revives values of this class from the object record serialize() writes for them +# (php/runtime/Shirabe/MaterializedValue.php), so the real declaration has to stay loadable. +Composer\Util\Http\Response |
