Make response stream caching max-age a configuration (#4348)

cypress/integration/09_admin_links.spec.js

@@ -41,6 +41,7 @@ context('Administration pages', () => {
     })
     cy.visit(baseurl + "/admin/dkan/datastore")
     cy.get('label[for="edit-rows-limit"]').should('have.text', 'Rows limit')
+    cy.get('label[for="edit-response-stream-max-age"]').should('have.text', 'Response Stream Max-Age')
   })
 
   it('Admin can access the Datastore import status dashboard.', () => {

modules/datastore/config/install/datastore.settings.yml

@@ -2,3 +2,4 @@ rows_limit: 500
 purge_file: 1
 purge_table: 1
 triggering_properties: []
+response_stream_max_age: 3600

modules/datastore/config/schema/datastore.schema.yml

@@ -16,3 +16,5 @@ datastore.settings:
       type: boolean
     drop_datastore_on_post_import_error:
       type: boolean
+    response_stream_max_age:
+      type: integer

modules/datastore/datastore.install

@@ -60,6 +60,16 @@ function datastore_update_9001(&$sandbox) {
   $schema->createTable($table_name, $spec);
 }
 
+/**
+ * Set the default value for the response stream max age.
+ */
+function datastore_update_9002(&$sandbox) {
+  $config = \Drupal::service('config.factory')->getEditable('datastore.settings');
+  if ($config->get('response_stream_max_age') === NULL) {
+    $config->set('response_stream_max_age', 3600)->save();
+  }
+}
+
 /**
  * Implements hook_schema().
  */

modules/datastore/src/Controller/QueryDownloadController.php

@@ -18,11 +18,6 @@
  */
 class QueryDownloadController extends AbstractQueryController {
 
-  /**
-   * Max-age cache control header value in seconds.
-   */
-  protected const RESPONSE_STREAM_MAX_AGE = 3600;
-
   /**
    * {@inheritDoc}
    */
@@ -31,9 +26,8 @@ public function __construct(QueryService $queryService, DatasetInfo $datasetInfo
     // We do not want to cache streaming CSV content internally in Drupal,
     // because datasets can be very large. However, we do want CDNs to be able
     // to cache the CSV stream for a reasonable amount of time.
-    // @todo Replace this constant with some form of customizable caching
-    //   strategy.
-    $this->cacheMaxAge = static::RESPONSE_STREAM_MAX_AGE;
+    $config = $configFactory->get('datastore.settings');
+    $this->cacheMaxAge = $config->get('response_stream_max_age');
   }
 
   /**

modules/datastore/src/Form/DatastoreSettingsForm.php

@@ -65,15 +65,23 @@ public function buildForm(array $form, FormStateInterface $form_state) {
       '#min' => 1,
       '#title' => $this->t('Rows limit'),
       '#default_value' => $this->config('datastore.settings')->get('rows_limit'),
-      '#description' => $this->t('Maximum number of rows the datastore endpoints can return 
-        in a single request. Caution: setting too high can lead to timeouts or memory issues. 
+      '#description' => $this->t('Maximum number of rows the datastore endpoints can return
+        in a single request. Caution: setting too high can lead to timeouts or memory issues.
         Default 500; values above 20,000 not recommended.'),
     ];
 
+    $form['response_stream_max_age'] = [
+      '#type' => 'number',
+      '#title' => $this->t('Response Stream Max-Age'),
+      '#default_value' => $this->config('datastore.settings')->get('response_stream_max_age'),
+      '#min' => 0,
+      '#description' => $this->t('Set the cache max-age for streaming CSV responses, in seconds. Default: 3600 (1 hour).'),
+    ];
+
     $form['triggering_properties'] = [
       '#type' => 'checkboxes',
       '#title' => $this->t('Datastore triggering properties'),
-      '#description' => $this->t('Metadata properties whose change will trigger a re-import of 
+      '#description' => $this->t('Metadata properties whose change will trigger a re-import of
         an associated resource to the datastore.'),
       '#options' => $this->schemaHelper->retrieveSchemaProperties('dataset'),
       '#default_value' => $this->config('datastore.settings')->get('triggering_properties'),
@@ -87,6 +95,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   public function submitForm(array &$form, FormStateInterface $form_state) {
     $this->config('datastore.settings')
       ->set('rows_limit', $form_state->getValue('rows_limit') ?: QueryController::DEFAULT_ROWS_LIMIT)
+      ->set('response_stream_max_age', $form_state->getValue('response_stream_max_age'))
       ->set('triggering_properties', $form_state->getValue('triggering_properties'))
       ->save();
     parent::submitForm($form, $form_state);

modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php

@@ -219,6 +219,7 @@ public function testStreamedQueryJson() {
   public function testStreamedLimit() {
     $queryLimit = 75;
     $pageLimit = 50;
+    $responseStreamMaxAge = 3600;
     $data = json_encode([
       "resources" => [
         [
@@ -230,7 +231,7 @@ public function testStreamedLimit() {
       "limit" => $queryLimit,
     ]);
     // Set the row limit to 50 even though we're requesting 1000.
-    $container = $this->getQueryContainer($pageLimit);
+    $container = $this->getQueryContainer($pageLimit, $responseStreamMaxAge);
     $downloadController = QueryDownloadController::create($container);
     $request = $this->mockRequest($data);
     ob_start([self::class, 'getBuffer']);
@@ -338,15 +339,17 @@ public function testStreamedBadSchema() {
   }
 
   /**
-   * Create a mock chain for the main container passed to the controller.
+   * Create a mock object for the main container passed to the controller.
    *
-   * @param array $info
-   *   Dataset info array mock to be returned by DatasetInfo::gather().
+   * @param int $rowLimit
+   *    The row limit for a query.
+   * @param int|null $responseStreamMaxAge
+   *    The max age for the response stream in cache, or NULL to use the default.
    *
-   * @return \MockChain\Chain
-   *   MockChain chain object.
+   * @return \PHPUnit\Framework\MockObject\MockObject
+   *   MockChain mock object.
    */
-  private function getQueryContainer(int $rowLimit) {
+  private function getQueryContainer(int $rowLimit, ?int $responseStreamMaxAge = NULL) {
     $options = (new Options())
       ->add("dkan.metastore.storage", DataFactory::class)
       ->add("dkan.datastore.service", DatastoreService::class)
@@ -413,7 +416,9 @@ private function getQueryContainer(int $rowLimit) {
       ->add(Query::class, "getQueryStorageMap", $storageMap)
       ->add(Query::class, 'getDatastoreService',  DatastoreService::class)
       ->add(DatastoreService::class, 'getDataDictionaryFields', NULL)
-      ->add(ImmutableConfig::class, 'get', $rowLimit);
+      // @todo Use an Options or Sequence return here; this will only work for one arg at a time.
+      ->add(ImmutableConfig::class, 'get', $rowLimit)
+      ->add(ImmutableConfig::class, 'get', $responseStreamMaxAge);
 
     return $chain->getMock();
   }