From ea9187ea92ecdee9a28612c83205cb8c7ddde259 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 11 Apr 2018 09:02:11 -0700
Subject: [PATCH] Allow Almanac properties to be set and deleted via Conduit

Summary:
Depends on D19342. Ref T12414. Ref T13120. This adds an EditEngine extension for editing Almanac properties.

The actual wire format is a little weird. Normally, we'd have a transaction for each property, but since you can pick any property names you want we can't really do that (we'd have to generate infinite transactions).

The transaction wire format anticipates that transactions may eventually get some kind of metadata -- each transaction looks like this:

```
{
  "type": "title",
  "value": "Example title"
}
```

...and we can add more keys there. For example, I could have made this transaction look like this:

```
{
  "type": "property.set",
  "almanac.property.key": "some-key",
  "value": "some-value"
}
```

However, I don't want to just accept any possible key freely, and it might be a decent chunk of work to formalize this better. It also doesn't feel great.

I just built special transaction types intead, so you:

```
{
  "type": "property.set",
  "value": {
   "some-key": "some-value",
   ...
  }
}
```

Internally, we may generate more than one transaction as a result (if the "value" has more than one key).

This feels a bit more natural and is probably easier for clients to use anyway.

Test Plan: Set and deleted Service, Device and Binding properties via the API.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13120, T12414

Differential Revision: https://secure.phabricator.com/D19343
---
 src/__phutil_library_map__.php                | 10 +++++
 .../editor/AlmanacBindingEditEngine.php       |  3 +-
 .../editor/AlmanacDeviceEditEngine.php        |  3 +-
 .../editor/AlmanacServiceEditEngine.php       |  3 +-
 .../AlmanacDeletePropertyEditField.php        | 22 ++++++++++
 .../AlmanacDeletePropertyEditType.php         | 36 +++++++++++++++
 .../AlmanacPropertiesEditEngineExtension.php  | 44 +++++++++++++++++++
 .../AlmanacSetPropertyEditField.php           | 22 ++++++++++
 .../AlmanacSetPropertyEditType.php            | 28 ++++++++++++
 9 files changed, 168 insertions(+), 3 deletions(-)
 create mode 100644 src/applications/almanac/engineextension/AlmanacDeletePropertyEditField.php
 create mode 100644 src/applications/almanac/engineextension/AlmanacDeletePropertyEditType.php
 create mode 100644 src/applications/almanac/engineextension/AlmanacPropertiesEditEngineExtension.php
 create mode 100644 src/applications/almanac/engineextension/AlmanacSetPropertyEditField.php
 create mode 100644 src/applications/almanac/engineextension/AlmanacSetPropertyEditType.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 2e5a384ac8..da5674cbbd 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -44,6 +44,8 @@ phutil_register_library_map(array(
     'AlmanacCreateServicesCapability' => 'applications/almanac/capability/AlmanacCreateServicesCapability.php',
     'AlmanacCustomServiceType' => 'applications/almanac/servicetype/AlmanacCustomServiceType.php',
     'AlmanacDAO' => 'applications/almanac/storage/AlmanacDAO.php',
+    'AlmanacDeletePropertyEditField' => 'applications/almanac/engineextension/AlmanacDeletePropertyEditField.php',
+    'AlmanacDeletePropertyEditType' => 'applications/almanac/engineextension/AlmanacDeletePropertyEditType.php',
     'AlmanacDevice' => 'applications/almanac/storage/AlmanacDevice.php',
     'AlmanacDeviceController' => 'applications/almanac/controller/AlmanacDeviceController.php',
     'AlmanacDeviceDeletePropertyTransaction' => 'applications/almanac/xaction/AlmanacDeviceDeletePropertyTransaction.php',
@@ -129,6 +131,7 @@ phutil_register_library_map(array(
     'AlmanacNetworkTransactionType' => 'applications/almanac/xaction/AlmanacNetworkTransactionType.php',
     'AlmanacNetworkViewController' => 'applications/almanac/controller/AlmanacNetworkViewController.php',
     'AlmanacPropertiesDestructionEngineExtension' => 'applications/almanac/engineextension/AlmanacPropertiesDestructionEngineExtension.php',
+    'AlmanacPropertiesEditEngineExtension' => 'applications/almanac/engineextension/AlmanacPropertiesEditEngineExtension.php',
     'AlmanacPropertiesSearchEngineAttachment' => 'applications/almanac/engineextension/AlmanacPropertiesSearchEngineAttachment.php',
     'AlmanacProperty' => 'applications/almanac/storage/AlmanacProperty.php',
     'AlmanacPropertyController' => 'applications/almanac/controller/AlmanacPropertyController.php',
@@ -165,6 +168,8 @@ phutil_register_library_map(array(
     'AlmanacServiceTypeTestCase' => 'applications/almanac/servicetype/__tests__/AlmanacServiceTypeTestCase.php',
     'AlmanacServiceTypeTransaction' => 'applications/almanac/xaction/AlmanacServiceTypeTransaction.php',
     'AlmanacServiceViewController' => 'applications/almanac/controller/AlmanacServiceViewController.php',
+    'AlmanacSetPropertyEditField' => 'applications/almanac/engineextension/AlmanacSetPropertyEditField.php',
+    'AlmanacSetPropertyEditType' => 'applications/almanac/engineextension/AlmanacSetPropertyEditType.php',
     'AlmanacTransactionType' => 'applications/almanac/xaction/AlmanacTransactionType.php',
     'AphlictDropdownDataQuery' => 'applications/aphlict/query/AphlictDropdownDataQuery.php',
     'Aphront304Response' => 'aphront/response/Aphront304Response.php',
@@ -5246,6 +5251,8 @@ phutil_register_library_map(array(
     'AlmanacCreateServicesCapability' => 'PhabricatorPolicyCapability',
     'AlmanacCustomServiceType' => 'AlmanacServiceType',
     'AlmanacDAO' => 'PhabricatorLiskDAO',
+    'AlmanacDeletePropertyEditField' => 'PhabricatorEditField',
+    'AlmanacDeletePropertyEditType' => 'PhabricatorEditType',
     'AlmanacDevice' => array(
       'AlmanacDAO',
       'PhabricatorPolicyInterface',
@@ -5365,6 +5372,7 @@ phutil_register_library_map(array(
     'AlmanacNetworkTransactionType' => 'AlmanacTransactionType',
     'AlmanacNetworkViewController' => 'AlmanacNetworkController',
     'AlmanacPropertiesDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension',
+    'AlmanacPropertiesEditEngineExtension' => 'PhabricatorEditEngineExtension',
     'AlmanacPropertiesSearchEngineAttachment' => 'AlmanacSearchEngineAttachment',
     'AlmanacProperty' => array(
       'AlmanacDAO',
@@ -5413,6 +5421,8 @@ phutil_register_library_map(array(
     'AlmanacServiceTypeTestCase' => 'PhabricatorTestCase',
     'AlmanacServiceTypeTransaction' => 'AlmanacServiceTransactionType',
     'AlmanacServiceViewController' => 'AlmanacServiceController',
+    'AlmanacSetPropertyEditField' => 'PhabricatorEditField',
+    'AlmanacSetPropertyEditType' => 'PhabricatorEditType',
     'AlmanacTransactionType' => 'PhabricatorModularTransactionType',
     'AphlictDropdownDataQuery' => 'Phobject',
     'Aphront304Response' => 'AphrontResponse',
diff --git a/src/applications/almanac/editor/AlmanacBindingEditEngine.php b/src/applications/almanac/editor/AlmanacBindingEditEngine.php
index 07a9e36e9e..5146578fff 100644
--- a/src/applications/almanac/editor/AlmanacBindingEditEngine.php
+++ b/src/applications/almanac/editor/AlmanacBindingEditEngine.php
@@ -91,7 +91,8 @@ final class AlmanacBindingEditEngine
   }
 
   protected function newObjectQuery() {
-    return new AlmanacBindingQuery();
+    return id(new AlmanacBindingQuery())
+      ->needProperties(true);
   }
 
   protected function getObjectCreateTitleText($object) {
diff --git a/src/applications/almanac/editor/AlmanacDeviceEditEngine.php b/src/applications/almanac/editor/AlmanacDeviceEditEngine.php
index 84ca59d310..fa0625e6ed 100644
--- a/src/applications/almanac/editor/AlmanacDeviceEditEngine.php
+++ b/src/applications/almanac/editor/AlmanacDeviceEditEngine.php
@@ -30,7 +30,8 @@ final class AlmanacDeviceEditEngine
   }
 
   protected function newObjectQuery() {
-    return new AlmanacDeviceQuery();
+    return id(new AlmanacDeviceQuery())
+      ->needProperties(true);
   }
 
   protected function getObjectCreateTitleText($object) {
diff --git a/src/applications/almanac/editor/AlmanacServiceEditEngine.php b/src/applications/almanac/editor/AlmanacServiceEditEngine.php
index 93912fad8b..00e54962b3 100644
--- a/src/applications/almanac/editor/AlmanacServiceEditEngine.php
+++ b/src/applications/almanac/editor/AlmanacServiceEditEngine.php
@@ -79,7 +79,8 @@ final class AlmanacServiceEditEngine
   }
 
   protected function newObjectQuery() {
-    return new AlmanacServiceQuery();
+    return id(new AlmanacServiceQuery())
+      ->needProperties(true);
   }
 
   protected function getObjectCreateTitleText($object) {
diff --git a/src/applications/almanac/engineextension/AlmanacDeletePropertyEditField.php b/src/applications/almanac/engineextension/AlmanacDeletePropertyEditField.php
new file mode 100644
index 0000000000..82aa4b82b5
--- /dev/null
+++ b/src/applications/almanac/engineextension/AlmanacDeletePropertyEditField.php
@@ -0,0 +1,22 @@
+<?php
+
+final class AlmanacDeletePropertyEditField
+  extends PhabricatorEditField {
+
+  protected function newControl() {
+    return null;
+  }
+
+  protected function newHTTPParameterType() {
+    return null;
+  }
+
+  protected function newConduitParameterType() {
+    return new ConduitStringListParameterType();
+  }
+
+  protected function newEditType() {
+    return new AlmanacDeletePropertyEditType();
+  }
+
+}
diff --git a/src/applications/almanac/engineextension/AlmanacDeletePropertyEditType.php b/src/applications/almanac/engineextension/AlmanacDeletePropertyEditType.php
new file mode 100644
index 0000000000..d9d95c355a
--- /dev/null
+++ b/src/applications/almanac/engineextension/AlmanacDeletePropertyEditType.php
@@ -0,0 +1,36 @@
+<?php
+
+final class AlmanacDeletePropertyEditType
+  extends PhabricatorEditType {
+
+  public function generateTransactions(
+    PhabricatorApplicationTransaction $template,
+    array $spec) {
+
+    $value = idx($spec, 'value');
+    if (!is_array($value)) {
+      throw new Exception(
+        pht(
+          'Transaction value when deleting Almanac properties must be a list '.
+          'of property names.'));
+    }
+
+    $xactions = array();
+    foreach ($value as $idx => $property_key) {
+      if (!is_string($property_key)) {
+        throw new Exception(
+          pht(
+            'When deleting Almanac properties, each property name must '.
+            'be a string. The value at index "%s" is not a string.',
+            $idx));
+      }
+
+      $xactions[] = $this->newTransaction($template)
+        ->setMetadataValue('almanac.property', $property_key)
+        ->setNewValue(true);
+    }
+
+    return $xactions;
+  }
+
+}
diff --git a/src/applications/almanac/engineextension/AlmanacPropertiesEditEngineExtension.php b/src/applications/almanac/engineextension/AlmanacPropertiesEditEngineExtension.php
new file mode 100644
index 0000000000..965c193f40
--- /dev/null
+++ b/src/applications/almanac/engineextension/AlmanacPropertiesEditEngineExtension.php
@@ -0,0 +1,44 @@
+<?php
+
+final class AlmanacPropertiesEditEngineExtension
+  extends PhabricatorEditEngineExtension {
+
+  const EXTENSIONKEY = 'almanac.properties';
+
+  public function isExtensionEnabled() {
+    return true;
+  }
+
+  public function getExtensionName() {
+    return pht('Almanac Properties');
+  }
+
+  public function supportsObject(
+    PhabricatorEditEngine $engine,
+    PhabricatorApplicationTransactionInterface $object) {
+    return ($object instanceof AlmanacPropertyInterface);
+  }
+
+  public function buildCustomEditFields(
+    PhabricatorEditEngine $engine,
+    PhabricatorApplicationTransactionInterface $object) {
+
+    return array(
+      id(new AlmanacSetPropertyEditField())
+        ->setKey('property.set')
+        ->setTransactionType($object->getAlmanacPropertySetTransactionType())
+        ->setConduitDescription(
+          pht('Pass a map of values to set one or more properties.'))
+        ->setConduitTypeDescription(pht('Map of property names to values.'))
+        ->setIsConduitOnly(true),
+      id(new AlmanacDeletePropertyEditField())
+        ->setKey('property.delete')
+        ->setTransactionType($object->getAlmanacPropertyDeleteTransactionType())
+        ->setConduitDescription(
+          pht('Pass a list of property names to delete properties.'))
+        ->setConduitTypeDescription(pht('List of property names.'))
+        ->setIsConduitOnly(true),
+    );
+  }
+
+}
diff --git a/src/applications/almanac/engineextension/AlmanacSetPropertyEditField.php b/src/applications/almanac/engineextension/AlmanacSetPropertyEditField.php
new file mode 100644
index 0000000000..aec3ea6c9f
--- /dev/null
+++ b/src/applications/almanac/engineextension/AlmanacSetPropertyEditField.php
@@ -0,0 +1,22 @@
+<?php
+
+final class AlmanacSetPropertyEditField
+  extends PhabricatorEditField {
+
+  protected function newControl() {
+    return null;
+  }
+
+  protected function newHTTPParameterType() {
+    return null;
+  }
+
+  protected function newConduitParameterType() {
+    return new ConduitWildParameterType();
+  }
+
+  protected function newEditType() {
+    return new AlmanacSetPropertyEditType();
+  }
+
+}
diff --git a/src/applications/almanac/engineextension/AlmanacSetPropertyEditType.php b/src/applications/almanac/engineextension/AlmanacSetPropertyEditType.php
new file mode 100644
index 0000000000..8a654382c6
--- /dev/null
+++ b/src/applications/almanac/engineextension/AlmanacSetPropertyEditType.php
@@ -0,0 +1,28 @@
+<?php
+
+final class AlmanacSetPropertyEditType
+  extends PhabricatorEditType {
+
+  public function generateTransactions(
+    PhabricatorApplicationTransaction $template,
+    array $spec) {
+
+    $value = idx($spec, 'value');
+    if (!is_array($value)) {
+      throw new Exception(
+        pht(
+          'Transaction value when setting Almanac properties must be a map '.
+          'with property names as keys.'));
+    }
+
+    $xactions = array();
+    foreach ($value as $property_key => $property_value) {
+      $xactions[] = $this->newTransaction($template)
+        ->setMetadataValue('almanac.property', $property_key)
+        ->setNewValue($property_value);
+    }
+
+    return $xactions;
+  }
+
+}