(stable) Promote 2018 Week 44

resources/sql/autopatches/20181031.board.01.queryreset.php

@@ -0,0 +1,50 @@
+<?php
+
+// See T13208. It was previously possible to replace a saved query with another
+// saved query, causing loss of the first query. Find projects which have their
+// default query set to an invalid query and throw the setting away.
+
+$viewer = PhabricatorUser::getOmnipotentUser();
+
+$table = new PhabricatorProject();
+$conn = $table->establishConnection('w');
+
+$iterator = new LiskMigrationIterator($table);
+$search_engine = id(new ManiphestTaskSearchEngine())
+  ->setViewer($viewer);
+
+foreach ($iterator as $project) {
+  $default_filter = $project->getDefaultWorkboardFilter();
+  if (!strlen($default_filter)) {
+    continue;
+  }
+
+  if ($search_engine->isBuiltinQuery($default_filter)) {
+    continue;
+  }
+
+  $saved = id(new PhabricatorSavedQueryQuery())
+    ->setViewer($viewer)
+    ->withQueryKeys(array($default_filter))
+    ->executeOne();
+  if ($saved) {
+    continue;
+  }
+
+  $properties = $project->getProperties();
+  unset($properties['workboard.filter.default']);
+
+  queryfx(
+    $conn,
+    'UPDATE %T SET properties = %s WHERE id = %d',
+    $table->getTableName(),
+    phutil_json_encode($properties),
+    $project->getID());
+
+  echo tsprintf(
+    "%s\n",
+    pht(
+      'Project ("%s") had an invalid query saved as a default workboard '.
+      'query. The query has been reset. See T13208.',
+      $project->getDisplayName()));
+}

src/__phutil_library_map__.php

@@ -1326,6 +1326,7 @@ phutil_register_library_map(array(
     'HarbormasterBuildPlanNameNgrams' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlanNameNgrams.php',
     'HarbormasterBuildPlanPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPlanPHIDType.php',
     'HarbormasterBuildPlanQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanQuery.php',
+    'HarbormasterBuildPlanSearchAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildPlanSearchAPIMethod.php',
     'HarbormasterBuildPlanSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildPlanSearchEngine.php',
     'HarbormasterBuildPlanTransaction' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php',
     'HarbormasterBuildPlanTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanTransactionQuery.php',
@@ -6775,6 +6776,7 @@ phutil_register_library_map(array(
       'PhabricatorPolicyInterface',
       'PhabricatorSubscribableInterface',
       'PhabricatorNgramsInterface',
+      'PhabricatorConduitResultInterface',
       'PhabricatorProjectInterface',
     ),
     'HarbormasterBuildPlanDatasource' => 'PhabricatorTypeaheadDatasource',
@@ -6785,6 +6787,7 @@ phutil_register_library_map(array(
     'HarbormasterBuildPlanNameNgrams' => 'PhabricatorSearchNgrams',
     'HarbormasterBuildPlanPHIDType' => 'PhabricatorPHIDType',
     'HarbormasterBuildPlanQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'HarbormasterBuildPlanSearchAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
     'HarbormasterBuildPlanSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'HarbormasterBuildPlanTransaction' => 'PhabricatorApplicationTransaction',
     'HarbormasterBuildPlanTransactionQuery' => 'PhabricatorApplicationTransactionQuery',

src/applications/diffusion/request/DiffusionRequest.php

@@ -489,7 +489,7 @@ abstract class DiffusionRequest extends Phobject {
     // Consume the back part of the URI, up to the first "$". Use a negative
     // lookbehind to prevent matching '$$'. We double the '$' symbol when
     // encoding so that files with names like "money/$100" will survive.
-    $pattern = '@(?:(?:^|[^$])(?:[$][$])*)[$]([\d-,]+)$@';
+    $pattern = '@(?:(?:^|[^$])(?:[$][$])*)[$]([\d,-]+)$@';
     if (preg_match($pattern, $blob, $matches)) {
       $result['line'] = $matches[1];
       $blob = substr($blob, 0, -(strlen($matches[1]) + 1));

src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php

@@ -45,6 +45,16 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     return true;
   }
 
+  public function shouldAllocateSupplementalResource(
+    DrydockBlueprint $blueprint,
+    DrydockResource $resource,
+    DrydockLease $lease) {
+    // We want to use every host in an Almanac service, since the amount of
+    // hardware is fixed and there's normally no value in packing leases onto a
+    // subset of it. Always build a new supplemental resource if we can.
+    return true;
+  }
+
   public function canAllocateResourceForLease(
     DrydockBlueprint $blueprint,
     DrydockLease $lease) {

src/applications/drydock/blueprint/DrydockBlueprintImplementation.php

@@ -139,6 +139,38 @@ abstract class DrydockBlueprintImplementation extends Phobject {
     DrydockResource $resource,
     DrydockLease $lease);
 
+  /**
+   * Return true to try to allocate a new resource and expand the resource
+   * pool instead of permitting an otherwise valid acquisition on an existing
+   * resource.
+   *
+   * This allows the blueprint to provide a soft hint about when the resource
+   * pool should grow.
+   *
+   * Returning "true" in all cases generally makes sense when a blueprint
+   * controls a fixed pool of resources, like a particular number of physical
+   * hosts: you want to put all the hosts in service, so whenever it is
+   * possible to allocate a new host you want to do this.
+   *
+   * Returning "false" in all cases generally make sense when a blueprint
+   * has a flexible pool of expensive resources and you want to pack leases
+   * onto them as tightly as possible.
+   *
+   * @param DrydockBlueprint The blueprint for an existing resource being
+   *   acquired.
+   * @param DrydockResource The resource being acquired, which we may want to
+   *   build a supplemental resource for.
+   * @param DrydockLease The current lease performing acquisition.
+   * @return bool True to prefer allocating a supplemental resource.
+   *
+   * @task lease
+   */
+  public function shouldAllocateSupplementalResource(
+    DrydockBlueprint $blueprint,
+    DrydockResource $resource,
+    DrydockLease $lease) {
+    return false;
+  }
 
 /* -(  Resource Allocation  )------------------------------------------------ */
 

src/applications/drydock/storage/DrydockBlueprint.php

@@ -278,6 +278,15 @@ final class DrydockBlueprint extends DrydockDAO
     return $interface;
   }
 
+  public function shouldAllocateSupplementalResource(
+    DrydockResource $resource,
+    DrydockLease $lease) {
+    return $this->getImplementation()->shouldAllocateSupplementalResource(
+      $this,
+      $resource,
+      $lease);
+  }
+
 
 /* -(  PhabricatorApplicationTransactionInterface  )------------------------- */
 

src/applications/drydock/worker/DrydockLeaseUpdateWorker.php

@@ -306,6 +306,7 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
     $allocated = false;
     foreach ($resources as $resource) {
       try {
+        $resource = $this->newResourceForAcquisition($resource, $lease);
         $this->acquireLease($resource, $lease);
         $allocated = true;
         break;
@@ -319,6 +320,10 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
         // got around to acquiring it, we just got unlucky. We can yield and
         // try again later.
         $yields[] = $ex;
+      } catch (PhabricatorWorkerYieldException $ex) {
+        // We can be told to yield, particularly by the supplemental allocator
+        // trying to give us a supplemental resource.
+        $yields[] = $ex;
       } catch (Exception $ex) {
         $exceptions[] = $ex;
       }
@@ -791,6 +796,73 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
     }
   }
 
+  private function newResourceForAcquisition(
+    DrydockResource $resource,
+    DrydockLease $lease) {
+
+    // If the resource has no leases against it, never build a new one. This is
+    // likely already a new resource that just activated.
+    $viewer = $this->getViewer();
+
+    $statuses = array(
+      DrydockLeaseStatus::STATUS_PENDING,
+      DrydockLeaseStatus::STATUS_ACQUIRED,
+      DrydockLeaseStatus::STATUS_ACTIVE,
+    );
+
+    $leases = id(new DrydockLeaseQuery())
+      ->setViewer($viewer)
+      ->withResourcePHIDs(array($resource->getPHID()))
+      ->withStatuses($statuses)
+      ->setLimit(1)
+      ->execute();
+    if (!$leases) {
+      return $resource;
+    }
+
+    // If we're about to get a lease on a resource, check if the blueprint
+    // wants to allocate a supplemental resource. If it does, try to perform a
+    // new allocation instead.
+    $blueprint = $resource->getBlueprint();
+    if (!$blueprint->shouldAllocateSupplementalResource($resource, $lease)) {
+      return $resource;
+    }
+
+    // If the blueprint is already overallocated, we can't allocate a new
+    // resource. Just return the existing resource.
+    $remaining = $this->removeOverallocatedBlueprints(
+      array($blueprint),
+      $lease);
+    if (!$remaining) {
+      return $resource;
+    }
+
+    // Try to build a new resource.
+    try {
+      $new_resource = $this->allocateResource($blueprint, $lease);
+    } catch (Exception $ex) {
+      $blueprint->logEvent(
+        DrydockResourceAllocationFailureLogType::LOGCONST,
+        array(
+          'class' => get_class($ex),
+          'message' => $ex->getMessage(),
+        ));
+
+      return $resource;
+    }
+
+    // If we can't actually acquire the new resource yet, just yield.
+    // (We could try to move forward with the original resource instead.)
+    $acquirable = $this->removeUnacquirableResources(
+      array($new_resource),
+      $lease);
+    if (!$acquirable) {
+      throw new PhabricatorWorkerYieldException(15);
+    }
+
+    return $new_resource;
+  }
+
 
 /* -(  Activating Leases  )-------------------------------------------------- */
 

src/applications/harbormaster/conduit/HarbormasterBuildPlanSearchAPIMethod.php

@@ -0,0 +1,18 @@
+<?php
+
+final class HarbormasterBuildPlanSearchAPIMethod
+  extends PhabricatorSearchEngineAPIMethod {
+
+  public function getAPIMethodName() {
+    return 'harbormaster.buildplan.search';
+  }
+
+  public function newSearchEngine() {
+    return new HarbormasterBuildPlanSearchEngine();
+  }
+
+  public function getMethodSummary() {
+    return pht('Retrieve information about Harbormaster build plans.');
+  }
+
+}

src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php

@@ -9,6 +9,7 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
     PhabricatorPolicyInterface,
     PhabricatorSubscribableInterface,
     PhabricatorNgramsInterface,
+    PhabricatorConduitResultInterface,
     PhabricatorProjectInterface {
 
   protected $name;
@@ -207,4 +208,34 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
     );
   }
 
+
+/* -(  PhabricatorConduitResultInterface  )---------------------------------- */
+
+
+  public function getFieldSpecificationsForConduit() {
+    return array(
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('name')
+        ->setType('string')
+        ->setDescription(pht('The name of this build plan.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('status')
+        ->setType('map<string, wild>')
+        ->setDescription(pht('The current status of this build plan.')),
+    );
+  }
+
+  public function getFieldValuesForConduit() {
+    return array(
+      'name' => $this->getName(),
+      'status' => array(
+        'value' => $this->getPlanStatus(),
+      ),
+    );
+  }
+
+  public function getConduitSearchAttachments() {
+    return array();
+  }
+
 }

src/applications/project/controller/PhabricatorProjectBoardViewController.php

@@ -203,7 +203,7 @@ final class PhabricatorProjectBoardViewController
       // with the column filter. If the user currently has constraints on the
       // board, we want to add a new column or project constraint, not
       // completely replace the constraints.
-      $saved_query = clone $saved;
+      $saved_query = $saved->newCopy();
 
       if ($query_column->getProxyPHID()) {
         $project_phids = $saved_query->getParameter('projectPHIDs');

src/applications/search/engine/PhabricatorApplicationSearchEngine.php

@@ -103,6 +103,14 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
   }
 
   public function saveQuery(PhabricatorSavedQuery $query) {
+    if ($query->getID()) {
+      throw new Exception(
+        pht(
+          'Query (with ID "%s") has already been saved. Queries are '.
+          'immutable once saved.',
+          $query->getID()));
+    }
+
     $query->setEngineClassName(get_class($this));
 
     $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();

src/applications/search/storage/PhabricatorSavedQuery.php

@@ -63,6 +63,13 @@ final class PhabricatorSavedQuery extends PhabricatorSearchDAO
     return $this->assertAttachedKey($this->parameterMap, $key);
   }
 
+  public function newCopy() {
+    return id(new self())
+      ->setParameters($this->getParameters())
+      ->setQueryKey(null)
+      ->setEngineClassName($this->getEngineClassName());
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */