Migrate APIv4 into civicrm-core

[totten]

Overview

APIv4 is a revamp of APIv3. It starts with a baseline that includes many of the values that evolved in APIv3, such as:

• API Explorer
• Metadata orientation
• Automated testing
• Bindings (PHP/JS/CLI)

It also makes several improvements, such as:

• More expressive queries and actions
• More consistent data formatting (for input data, output data, and metadata)
• Better structure for defining new entities

APIv4 was initially developed as an extension (org.civicrm.api4) for CiviCRM. Developing as an extension has allowed it to go through an incubation process over a couple years (iterating quickly at first; and then refining the contract in response to real-world usage). At time of writing, the best walk-through of the technical details is the org.civicrm.api4 README file.

This PR migrates the APIv4 codebase from a standalone extension (org.civicrm.api4) -- and into civicrm-core, where APIv3 and APIv4 will sit alongside each other. This migration is broadly motivated by a few observations:

1. The "API" framework -- in any version, as generally understood in the Civi community -- is a cross-cutting concern shared by all major subsystems (contact-management, contribution-management, CMS-integration, ad nauseum). The "API" framework is (and should be) a dependency of most major UIs and extensions.
2. We believe that APIv4 has passed a tipping-point in its stabilization process. There has been decreasing need for flexible/on-demand releases -- and growing need for simpler workflows+coordination. Automated QA, manual QA, and downstream deployments will be simpler with APIv4 in core.

Before

• APIv4 has been an extension, org.civicrm.api4. It has lived in a separate repo, and (at release-time) it has been bundled into the same release as civicrm-core.
• org.civicrm.api4 and civicrm-core were in a bi-directional dependency with non-synchronized version numbers.

After

• For CiviCRM 5.19 and later, APIv4 is built into civicrm-core. There is no bi-directional dependency. (If the old extension is present, it will be disabled.)
• For CiviCRM 5.18 and earlier, the APIv4 extension will remain available in Github and the extension directory.
• General fixes and refinements for APIv4 should be directed at civicrm-core instead of org.civicrm.api4. However, if a critical/security issue arises during the support window for CiviCRM <=5.18, then the corresponding org.civicrm.api4 can be updated.

Technical Details

For developers, BootstrapCSS is recommended for use with the APIv4 Explorer GUI. This has been primarily used/tested with org.civicrm.shoreditch, although it generates standard markup and aims to be compatible with other BootstrapCSS providers (CMS themes/CRM extensions).

Additional details about the migration process are tracked in civicrm/org.civicrm.api4#198.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[totten]

@colemanw I've pushed up a new run of the imported code. It's sort of working -- in that this scenario seems to execute:

[bknix-min:~/bknix/build/dmaster/web/sites/all/modules/civicrm] cv ev -U admin 'return civicrm_api4("Contact","get",["checkPermissions"=>0,"select"=>["display_name"],"limit"=>5]);' -v
{
    "1": {
        "id": "1",
        "display_name": "Default Organization"
    },
    "2": {
        "id": "2",
        "display_name": "Sharyn Cooper"
    },
    "3": {
        "id": "3",
        "display_name": "Jameson family"
    },
    "4": {
        "id": "4",
        "display_name": "Kacey Smith"
    },
    "5": {
        "id": "5",
        "display_name": "Alexia Nielsen"
    }
}

However, it shouldn't be necessary to set checkPermissions=>0 when the user is admin. Yet it is.

[bknix-min:~/bknix/build/dmaster/web/sites/all/modules/civicrm] cv ev -U admin 'return civicrm_api4("Contact","get",["select"=>["display_name"],"limit"=>5]);' -v


  [Civi\API\Exception\UnauthorizedException]
  Authorization failed


Exception trace:
 () at /Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/API/Kernel.php:241
 Civi\API\Kernel->authorize() at /Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/API/Kernel.php:166
 Civi\API\Kernel->runRequest() at /Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/Api4/Generic/AbstractAction.php:200
 Civi\Api4\Generic\AbstractAction->execute() at /Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/api/api.php:46
 civicrm_api4() at phar:///Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/bin/cv.phar/src/Command/EvalCommand.php(50) : eval()'d code:1
 eval() at phar:///Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/bin/cv.phar/src/Command/EvalCommand.php:50

I get a similar error when running the unit-tests:

[bknix-min:~/bknix/build/dmaster/web/sites/all/modules/civicrm] env CIVICRM_UF=UnitTests phpunit5 tests/phpunit/api/v4/AllTests.php  --stop-on-failure
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

E

Time: 429 ms, Memory: 24.00MB

There was 1 error:

1) api\v4\Action\BasicActionsTest::testCrud
Civi\API\Exception\UnauthorizedException: Authorization failed

/Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/API/Kernel.php:241
/Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/API/Kernel.php:166
/Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/Api4/Generic/AbstractAction.php:200
/Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Action/BasicActionsTest.php:14
/Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598

Outstanding issues:

1. The test issue above.
2. Update navigation menu (akin to CRM/Api4/Upgrader.php). Be sensitive to sites that are transitioning from ext=>core version.
3. Deal with the frontend/Bootstrap dependency. I started by playing a bit with this (https://gist.github.com/totten/62fb87b8f2e68189a530557702b2f9db) but stopped because the frontend situation probably needs more discussion.

[colemanw]

@totten I think the problem was that services weren't getting loaded. Again, it was a directory scanning issue. Services do things like permission checks :)

[totten]

@colemanw Yeah, I can see how it's doing a scan for subscribers and has been missing PermissionCheckSubscriber. 173c25a still wasn't registering the listener correctly, but I pushed an update to tweak the path computation a little more. Permission check now works in CLI, and several more tests are passing locally.

[seamuslee001]

Jenkins re test this please

[seamuslee001]

@totten @coleamnw karma is throwing up an error

PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  {
    "message": "An error was thrown in afterAll\nTypeError: undefined is not an object (evaluating 'CRM.vars.api4.schema')",
    "str": "An error was thrown in afterAll\nTypeError: undefined is not an object (evaluating 'CRM.vars.api4.schema')"
  }

[colemanw]

I don't get it. Afaik there are no api4 karma tests.

[seamuslee001]

@colemanw i think its globbing everything in the ang/ folder and i think its tripping on this code https://github.com/civicrm/civicrm-core/pull/15309/files#diff-c0be9ef6aab30b51901d6a52da683562R4

[colemanw]

@totten it seems that most of the stuff in the root ang directory use the standard civicrm/a base page. So maybe putting the explorer in there breaks that assumption?

[seamuslee001]

@colemanw @totten i found that doing this worked for me locally

diff --git a/karma.conf.js b/karma.conf.js
index 2915dc7814..59064d6596 100644
--- a/karma.conf.js
+++ b/karma.conf.js
@@ -21,6 +21,7 @@ module.exports = function(config) {
     autoWatch: true,
     browsers: ['PhantomJS'],
     exclude: [
+      'ang/api4Explorer/Explorer.js',
     ],
     files: [
       'bower_components/phantomjs-polyfill/bind-polyfill.js',

[seamuslee001]

@totten @colemanw to get the api_v4_AllTests command working for phpunit i had to add this in

diff --git a/tools/scripts/phpunit b/tools/scripts/phpunit
index 671c4d7440..2e682bc0db 100755
--- a/tools/scripts/phpunit
+++ b/tools/scripts/phpunit
@@ -50,7 +50,7 @@ array_shift($argv);
 // Convert class names to file names
 $CIVICRM_UF = 'UnitTests';
 foreach ($argv as $k => $v) {
-  if (preg_match('/^(CRM_|api_v3_|EnvTest|WebTest_|E2E_)/', $v)) {
+  if (preg_match('/^(CRM_|api_v3_|api_v4_|EnvTest|WebTest_|E2E_)/', $v)) {
     $argv[$k] = 'tests/phpunit/' . strtr($v, '_', '/') . '.php';
   }
   elseif (preg_match('/^Civi\\\\/', $v)) {

[colemanw]

@totten after checking out this branch and disabling the extension, I tried to visit the api explorer and got:

Exception: "Unrecognized Angular module"
  
  #0 /sites/all/modules/civicrm/Civi/Angular/Manager.php(332): Civi\Angular\Manager->getModule("api4Explorer")
  #1 /sites/all/modules/civicrm/Civi/Angular/AngularLoader.php(138): Civi\Angular\Manager->getResources((Array:1), "css", "cacheUrl")
  #2 /sites/all/modules/civicrm/CRM/Api4/Page/Api4Explorer.php(24): Civi\Angular\AngularLoader->load()
  #3 /sites/all/modules/civicrm/CRM/Core/Invoke.php(290): CRM_Api4_Page_Api4Explorer->run((Array:2), NULL)
  #4 /sites/all/modules/civicrm/CRM/Core/Invoke.php(84): CRM_Core_Invoke::runItem((Array:13))
  #5 /sites/all/modules/civicrm/CRM/Core/Invoke.php(52): CRM_Core_Invoke::_invoke((Array:2))
  #6 /sites/all/modules/civicrm/drupal/civicrm.module(444): CRM_Core_Invoke::invoke((Array:2))
  #7 /includes/menu.inc(527): civicrm_invoke("api4")
  #8 /index.php(20): menu_execute_active_handler()

[seamuslee001]

@colemanw @totten i had the same issue as Coleman but got the explorer working by doing the following changes

diff --git a/Civi/Angular/Manager.php b/Civi/Angular/Manager.php
index c44b854aa6..cb4c4c86d2 100644
--- a/Civi/Angular/Manager.php
+++ b/Civi/Angular/Manager.php
@@ -98,6 +98,8 @@ class Manager {
       $angularModules['ui.sortable'] = include "$civicrm_root/ang/ui.sortable.ang.php";
       $angularModules['unsavedChanges'] = include "$civicrm_root/ang/unsavedChanges.ang.php";
       $angularModules['statuspage'] = include "$civicrm_root/ang/crmStatusPage.ang.php";
+      $angularModules['api4Explorer'] = include "$civicrm_root/ang/api4Explorer.ang.php";
+      $angularModules['api4'] = include "$civicrm_root/ang/api4.ang.php";

       foreach (\CRM_Core_Component::getEnabledComponents() as $component) {
         $angularModules = array_merge($angularModules, $component->getAngularModules());
diff --git a/ang/api4.ang.php b/ang/api4.ang.php
index f7d69b39a3..2c8a85b753 100644
--- a/ang/api4.ang.php
+++ b/ang/api4.ang.php
@@ -1,6 +1,7 @@
 <?php
 // Autoloader data for Api4 angular module.
 return [
+  'ext' => 'civicrm',
   'js' => [
     'ang/api4.js',
     'ang/api4/*.js',
diff --git a/ang/api4Explorer.ang.php b/ang/api4Explorer.ang.php
index e00ea0985c..dab25db40a 100644
--- a/ang/api4Explorer.ang.php
+++ b/ang/api4Explorer.ang.php
@@ -1,6 +1,7 @@
 <?php
 // Autoloader data for Api4 explorer.
 return [
+  'ext' => 'civicrm',
   'js' => [
     'ang/api4Explorer.js',
     'ang/api4Explorer/*.js',

Once i got the explorer loaded i was having some javascript errors and i think it came down to the fact that in crm.ajax.js we don't define specific CRM.$ and CRM._ as $ and _ respectfully but we just have a CRM matching so things seemed to improve with the following changes

--- a/js/crm.ajax.js
+++ b/js/crm.ajax.js
@@ -49,9 +49,9 @@
   // Assign all the metadata properties to it, mirroring the results arrayObject in php
   function arrayObject(data) {
     var result = data.values || [];
-    if (_.isArray(result)) {
+    if (CRM._.isArray(result)) {
       delete(data.values);
-      _.assign(result, data);
+      CRM._.assign(result, data);
     }
     return result;
   }
@@ -59,29 +59,29 @@
   CRM.api4 = function(entity, action, params, index) {
     return new Promise(function(resolve, reject) {
       if (typeof entity === 'string') {
-        $.post(CRM.url('civicrm/ajax/api4/' + entity + '/' + action), {
+        CRM.$.post(CRM.url('civicrm/ajax/api4/' + entity + '/' + action), {
           params: JSON.stringify(params),
           index: index
         })
-          .done(function (data) {
-            resolve(arrayObject(data));
-          })
-          .fail(function (data) {
-            reject(data.responseJSON);
-          });
+        .done(function (data) {
+          resolve(arrayObject(data));
+        })
+        .fail(function (data) {
+          reject(data.responseJSON);
+        });
       } else {
-        $.post(CRM.url('civicrm/ajax/api4'), {
+        CRM.$.post(CRM.url('civicrm/ajax/api4'), {
           calls: JSON.stringify(entity)
         })
-          .done(function(data) {
-            _.each(data, function(item, key) {
-              data[key] = arrayObject(item);
-            });
-            resolve(data);
-          })
-          .fail(function (data) {
-            reject(data.responseJSON);
+        .done(function(data) {
+          CRM._.each(data, function(item, key) {
+            data[key] = arrayObject(item);
           });
+          resolve(data);
+        })
+        .fail(function (data) {
+          reject(data.responseJSON);
+        });
       }
     });
   };

[seamuslee001]

I should add that i haven't tried those changes with the extension active

[colemanw]

@seamuslee001 I opened totten#1 against this branch but am still scratching my head about why angular blows up.

[seamuslee001]

@colemanw if you look at the Angular manager you will notice that it only loads in specific .ang.php files for civicrm which I think is the issue here whilst the extension is in play the hook manages the loading of the modules take that away and because there is no glob pattern if .ang.php files the api4 moudles aren’t loaded

[colemanw]

Ok got it, thanks.

[colemanw]

@civicrm-builder retest this please

[seamuslee001]

@colemanw

  [GitScan\Exception\ProcessErrorException]                                                
  Process failed:                                                                          
  [[ COMMAND: git apply --check --ignore-whitespace ]]                                     
  [[ CWD: /home/jenkins/bknix-dfl/build/core-15309-811eb/web/sites/all/modules/civicrm ]]  
  [[ EXIT CODE: 1 ]]                                                                       
  [[ STDOUT ]]                                                                             
  [[ STDERR ]]                                                                             
  error: css/explorer.css: No such file or directory                                       

[colemanw]

@seamuslee001 that seems like an edge-case bug in our test infra. Perhaps it doesn't like a file being added and deleted within the same PR. I've just rebased the file name change into the original commit to see if that works better.

[seamuslee001]

@colemanw the style cop has logged on https://test.civicrm.org/job/CiviCRM-Core-PR/28952/checkstyleResult/new/

[colemanw]

@colemanw i think its globbing everything in the ang/ folder and i think its tripping on this code https://github.com/civicrm/civicrm-core/pull/15309/files#diff-c0be9ef6aab30b51901d6a52da683562R4

Ironically, the core angular loader doesn't glob the directory so we had to add in those files by hand. @totten

[seamuslee001]

@colemanw test failures look related

[colemanw]

Whoops those changes snuck in by accident

[totten]

Before we merge, I think we'll need to regenerate civicrm_generated.mysql. (But I think it's OK if that waits until the "WIP" flag is done.)

[colemanw]

@totten could you generate that file pls?

[totten]

jenkins, test this please

[colemanw]

@totten I think we're ready for that. The only outstanding issue is the explorer bootstrap theme, but we could take care of that in a subsequent PR. Someone wanna run gencode and remove the WIP tag?

[seamuslee001]

Jenkins re test this please

[seamuslee001]

Jenkins re test this please

[colemanw]

@totten this is passing. Just needs the menu sql generated.

[totten]

@colemanw I like the new warning:

Seems like a nice way to detect whether Bootstrap is working and to also defer the topic of bootstrap-in-core (which is significant thing on its own) for another discussion/day. Tangentially, I think we need more sysadmin-facing docs about theme configuration. https://github.com/civicrm/civicrm-sysadmin-guide/issues/191

[seamuslee001]

@totten @Coleman i performed the following test this morning

1. Created a drupal-empty build
2. untared current RC tarball and installed CiviCRM through the GUI and then enabled APIv4 extension noting that the extension code is bundled in the tar ball
3. Built a local tarball using distmaker based on this branch
4. did rm -rf civicrm/* on the current code base and untared the built tarball over the top
5. Ran the CiviCRM Upgrader in the GUI
6. Noticed error notices about missing extension api4 and also couldn't access the api4 explorer because of an exception and on the extensions page it looked like
   
7. after manually disabling the extension I was able to access the apiv4 explorer.

The status was string(17) "installed-missing"

I note that in the upgrade code it only checks that the status is Manager::STATUS_INSTALLED not manager::STATUS_INSTALLLED_MISSING

[colemanw]

@seamuslee001 thanks for the testing. I think #15343 will address it.
Update: I've just tested the upgrade myself with the extension enabled but missing and it worked well. I've just pushed up a minor api explorer bugfix and I think we're good to MOP.