From e98a5c1fe8c526a4cecf80e48a7454de156d3f95 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 15 Dec 2021 00:22:22 -0800
Subject: [PATCH] (REF) Civi::pipe - Tighten up docblocks and type-hints

---
 Civi.php                    |  9 +++++++--
 Civi/Pipe/PipeSession.php   |  5 +++--
 Civi/Pipe/PublicMethods.php | 14 +++++++-------
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/Civi.php b/Civi.php
index a11c38b5a1..9e4b8dcbfd 100644
--- a/Civi.php
+++ b/Civi.php
@@ -124,9 +124,14 @@ class Civi {
    *    - 'l': Report on login support in connection header.
    *    - 't': Trusted session. Logins do not require credentials. API calls may execute with or without permission-checks.
    *    - 'u': Untrusted session. Logins require credentials. API calls may only execute with permission-checks.
-   *   Note: The `Civi::pipe()` entry-point is designed to be amenable to shell orchestration (SSH/cv/drush/wp-cli/etc).
+   *
+   *   The `Civi::pipe()` entry-point is designed to be amenable to shell orchestration (SSH/cv/drush/wp-cli/etc).
    *   The negotiation flags are therefore condensed to individual characters.
-   *   Note: Future flags may be added to the default list. But be careful about removing flags from the default list.
+   *
+   *   It is possible to preserve compatibility while adding new default-flags. However, removing default-flags
+   *   is more likely to be a breaking-change.
+   *
+   *   When adding a new flag, consider whether mutable `option()`s may be more appropriate.
    * @see \Civi\Pipe\PipeSession
    */
   public static function pipe(string $negotiationFlags = 'vtl'): void {
diff --git a/Civi/Pipe/PipeSession.php b/Civi/Pipe/PipeSession.php
index f15c1a4f98..cd900067ad 100644
--- a/Civi/Pipe/PipeSession.php
+++ b/Civi/Pipe/PipeSession.php
@@ -18,9 +18,10 @@ class PipeSession {
   protected const METHOD_REGEX = ';^[a-z][a-zA-Z0-9_]*$;';
 
   /**
-   * Open-ended object. Any public method will be available during this session.
+   * Open-ended object. Any public method from this object will be available during this session.
    *
    * @var object
+   * @see \Civi\Pipe\PublicMethods
    */
   protected $methods;
 
@@ -75,7 +76,7 @@ class PipeSession {
    * @inheritDoc
    */
   protected function onRequest(string $requestLine): ?string {
-    return JsonRpc::run($requestLine, function($method, $params) {
+    return JsonRpc::run($requestLine, function(string $method, array $params) {
       $method = str_replace('.', '_', $method);
       if (!preg_match(self::METHOD_REGEX, $method)) {
         throw new \InvalidArgumentException('Method not found', -32601);
diff --git a/Civi/Pipe/PublicMethods.php b/Civi/Pipe/PublicMethods.php
index f64c743796..bf687860fd 100644
--- a/Civi/Pipe/PublicMethods.php
+++ b/Civi/Pipe/PublicMethods.php
@@ -44,7 +44,7 @@ class PublicMethods {
    *   Tuple: [$entity, $action, $params]
    * @return array|\Civi\Api4\Generic\Result|int
    */
-  public function api3($session, $request) {
+  public function api3(PipeSession $session, array $request) {
     $request[2] = array_merge($request[2] ?? [], ['version' => 3]);
     $request[2]['check_permissions'] = !$session->isTrusted() || $this->isCheckPermissions($request[2], 'check_permissions');
     // ^^ Untrusted sessions MUST check perms. All sessions DEFAULT to checking perms. Trusted sessions MAY disable perms.
@@ -68,7 +68,7 @@ class PublicMethods {
    *   Tuple: [$entity, $action, $params]
    * @return array|\Civi\Api4\Generic\Result|int
    */
-  public function api4($session, $request) {
+  public function api4(PipeSession $session, array $request) {
     $request[2] = array_merge($request[2] ?? [], ['version' => 4]);
     $request[2]['checkPermissions'] = !$session->isTrusted() || $this->isCheckPermissions($request[2], 'checkPermissions');
     // ^^ Untrusted sessions MUST check perms. All sessions DEFAULT to checking perms. Trusted sessions MAY disable perms.
@@ -88,10 +88,10 @@ class PublicMethods {
    * Simple test; send/receive a fragment of data.
    *
    * @param \Civi\Pipe\PipeSession $session
-   * @param mixed $request
-   * @return mixed
+   * @param array $request
+   * @return array
    */
-  public function echo($session, $request) {
+  public function echo(PipeSession $session, array $request) {
     return $request;
   }
 
@@ -102,7 +102,7 @@ class PublicMethods {
    * @param array{contactId: int, userId: int, user: string} $request
    * @return array|\Civi\Api4\Generic\Result|int
    */
-  public function login($session, $request) {
+  public function login(PipeSession $session, array $request) {
     if (!function_exists('authx_login')) {
       throw new \CRM_Core_Exception("Cannot authenticate. Authx is not configured.");
     }
@@ -139,7 +139,7 @@ class PublicMethods {
    *   List of updated options.
    *   If the list of updates was empty, then return all options.
    */
-  public function options($session, $request) {
+  public function options(PipeSession $session, array $request) {
     $storageMap = [
       'apiCheckPermissions' => $this,
       'apiError' => $this,
-- 
2.25.1