Merge branch '281174-add-source-sha256-to-the-internal-pages-api' into 'master'

Add additional fields to pages API

See merge request gitlab-org/gitlab!47423

app/models/pages/lookup_path.rb

@@ -40,37 +40,35 @@ module Pages
     def artifacts_archive
       return unless Feature.enabled?(:pages_serve_from_artifacts_archive, project)
 
-      archive = project.pages_metadatum.artifacts_archive
-
-      archive&.file
+      project.pages_metadatum.artifacts_archive
     end
 
     def deployment
       return unless Feature.enabled?(:pages_serve_from_deployments, project)
 
-      deployment = project.pages_metadatum.pages_deployment
-
-      deployment&.file
+      project.pages_metadatum.pages_deployment
     end
 
     def zip_source
       source = deployment || artifacts_archive
 
-      return unless source
+      return unless source&.file
 
-      if source.file_storage?
-        return unless Feature.enabled?(:pages_serve_with_zip_file_protocol, project)
+      return if source.file.file_storage? && !Feature.enabled?(:pages_serve_with_zip_file_protocol, project)
 
-        {
-          type: 'zip',
-          path: 'file://' + source.path
-        }
-      else
-        {
-          type: 'zip',
-          path: source.url(expire_at: 1.day.from_now)
-        }
-      end
+      # artifacts archive doesn't support this
+      file_count = source.file_count if source.respond_to?(:file_count)
+
+      global_id = ::Gitlab::GlobalId.build(source, id: source.id).to_s
+
+      {
+        type: 'zip',
+        path: source.file.url_or_file_path(expire_at: 1.day.from_now),
+        global_id: global_id,
+        sha256: source.file_sha256,
+        file_size: source.size,
+        file_count: file_count
+      }
     end
 
     def file_source

app/uploaders/gitlab_uploader.rb

@@ -118,6 +118,14 @@ class GitlabUploader < CarrierWave::Uploader::Base
     storage.store!(file)
   end
 
+  def url_or_file_path(url_options = {})
+    if file_storage?
+      'file://' + path
+    else
+      url(url_options)
+    end
+  end
+
   private
 
   # Designed to be overridden by child uploaders that have a dynamic path

spec/fixtures/api/schemas/internal/pages/lookup_path.json

@@ -15,7 +15,11 @@
       "required": ["type", "path"],
       "properties" : {
         "type": { "type": "string", "enum": ["file", "zip"] },
-        "path": { "type": "string" }
+        "path": { "type": "string" },
+        "global_id": { "type": "string" },
+        "sha256": { "type": "string" },
+        "file_size": { "type": "integer" },
+        "file_count": { "type": ["integer", "null"] }
       },
       "additionalProperties": false
     },

spec/models/pages/lookup_path_spec.rb

@@ -65,11 +65,18 @@ RSpec.describe Pages::LookupPath do
         project.pages_metadatum.update!(pages_deployment: deployment)
       end
 
-      it 'uses deployment from object storage', :aggregate_failures do
+      it 'uses deployment from object storage' do
         Timecop.freeze do
-          expect(source[:type]).to eq('zip')
-          expect(source[:path]).to eq(deployment.file.url(expire_at: 1.day.from_now))
-          expect(source[:path]).to include("Expires=86400")
+          expect(source).to(
+            eq({
+                 type: 'zip',
+                 path: deployment.file.url(expire_at: 1.day.from_now),
+                 global_id: "gid://gitlab/PagesDeployment/#{deployment.id}",
+                 sha256: deployment.file_sha256,
+                 file_size: deployment.size,
+                 file_count: deployment.file_count
+               })
+          )
         end
       end
 
@@ -78,10 +85,18 @@ RSpec.describe Pages::LookupPath do
           deployment.file.migrate!(::ObjectStorage::Store::LOCAL)
         end
 
-        it 'uses file protocol', :aggregate_failures do
+        it 'uses file protocol' do
           Timecop.freeze do
-            expect(source[:type]).to eq('zip')
-            expect(source[:path]).to eq('file://' + deployment.file.path)
+            expect(source).to(
+              eq({
+                   type: 'zip',
+                   path: 'file://' + deployment.file.path,
+                   global_id: "gid://gitlab/PagesDeployment/#{deployment.id}",
+                   sha256: deployment.file_sha256,
+                   file_size: deployment.size,
+                   file_count: deployment.file_count
+                 })
+            )
           end
         end
 
@@ -110,11 +125,18 @@ RSpec.describe Pages::LookupPath do
         project.mark_pages_as_deployed(artifacts_archive: artifacts_archive)
       end
 
-      it 'uses artifacts object storage', :aggregate_failures do
+      it 'uses artifacts object storage' do
         Timecop.freeze do
-          expect(source[:type]).to eq('zip')
-          expect(source[:path]).to eq(artifacts_archive.file.url(expire_at: 1.day.from_now))
-          expect(source[:path]).to include("Expires=86400")
+          expect(source).to(
+            eq({
+                 type: 'zip',
+                 path: artifacts_archive.file.url(expire_at: 1.day.from_now),
+                 global_id: "gid://gitlab/Ci::JobArtifact/#{artifacts_archive.id}",
+                 sha256: artifacts_archive.file_sha256,
+                 file_size: artifacts_archive.size,
+                 file_count: nil
+               })
+          )
         end
       end
 
@@ -123,8 +145,16 @@ RSpec.describe Pages::LookupPath do
 
         it 'uses file protocol', :aggregate_failures do
           Timecop.freeze do
-            expect(source[:type]).to eq('zip')
-            expect(source[:path]).to eq('file://' + artifacts_archive.file.path)
+            expect(source).to(
+              eq({
+                   type: 'zip',
+                   path: 'file://' + artifacts_archive.file.path,
+                   global_id: "gid://gitlab/Ci::JobArtifact/#{artifacts_archive.id}",
+                   sha256: artifacts_archive.file_sha256,
+                   file_size: artifacts_archive.size,
+                   file_count: nil
+                 })
+            )
           end
         end
 

spec/requests/api/internal/pages_spec.rb

@@ -191,6 +191,7 @@ RSpec.describe API::Internal::Pages do
             expect(json_response['certificate']).to eq(pages_domain.certificate)
             expect(json_response['key']).to eq(pages_domain.key)
 
+            deployment = project.pages_metadatum.pages_deployment
             expect(json_response['lookup_paths']).to eq(
               [
                 {
@@ -200,7 +201,11 @@ RSpec.describe API::Internal::Pages do
                   'prefix' => '/',
                   'source' => {
                     'type' => 'zip',
-                    'path' => project.pages_metadatum.pages_deployment.file.url(expire_at: 1.day.from_now)
+                    'path' => deployment.file.url(expire_at: 1.day.from_now),
+                    'global_id' => "gid://gitlab/PagesDeployment/#{deployment.id}",
+                    'sha256' => deployment.file_sha256,
+                    'file_size' => deployment.size,
+                    'file_count' => deployment.file_count
                   }
                 }
               ]
@@ -227,6 +232,7 @@ RSpec.describe API::Internal::Pages do
             expect(response).to have_gitlab_http_status(:ok)
             expect(response).to match_response_schema('internal/pages/virtual_domain')
 
+            deployment = project.pages_metadatum.pages_deployment
             expect(json_response['lookup_paths']).to eq(
               [
                 {
@@ -236,7 +242,11 @@ RSpec.describe API::Internal::Pages do
                   'prefix' => '/myproject/',
                   'source' => {
                     'type' => 'zip',
-                    'path' => project.pages_metadatum.pages_deployment.file.url(expire_at: 1.day.from_now)
+                    'path' => deployment.file.url(expire_at: 1.day.from_now),
+                    'global_id' => "gid://gitlab/PagesDeployment/#{deployment.id}",
+                    'sha256' => deployment.file_sha256,
+                    'file_size' => deployment.size,
+                    'file_count' => deployment.file_count
                   }
                 }
               ]
@@ -268,6 +278,7 @@ RSpec.describe API::Internal::Pages do
             expect(response).to have_gitlab_http_status(:ok)
             expect(response).to match_response_schema('internal/pages/virtual_domain')
 
+            deployment = project.pages_metadatum.pages_deployment
             expect(json_response['lookup_paths']).to eq(
               [
                 {
@@ -277,7 +288,11 @@ RSpec.describe API::Internal::Pages do
                   'prefix' => '/',
                   'source' => {
                     'type' => 'zip',
-                    'path' => project.pages_metadatum.pages_deployment.file.url(expire_at: 1.day.from_now)
+                    'path' => deployment.file.url(expire_at: 1.day.from_now),
+                    'global_id' => "gid://gitlab/PagesDeployment/#{deployment.id}",
+                    'sha256' => deployment.file_sha256,
+                    'file_size' => deployment.size,
+                    'file_count' => deployment.file_count
                   }
                 }
               ]

spec/uploaders/gitlab_uploader_spec.rb

@@ -141,5 +141,23 @@ RSpec.describe GitlabUploader do
         end
       end
     end
+
+    describe '#url_or_file_path' do
+      let(:options) { { expire_at: 1.day.from_now } }
+
+      it 'returns url when in remote storage' do
+        expect(subject).to receive(:file_storage?).and_return(false)
+        expect(subject).to receive(:url).with(options).and_return("http://example.com")
+
+        expect(subject.url_or_file_path(options)).to eq("http://example.com")
+      end
+
+      it 'returns url when in remote storage' do
+        expect(subject).to receive(:file_storage?).and_return(true)
+        expect(subject).to receive(:path).and_return("/tmp/file")
+
+        expect(subject.url_or_file_path(options)).to eq("file:///tmp/file")
+      end
+    end
   end
 end