Merge pull request #3703 from oliverguenther/feature/remote_repository_url

[Packager] Let managed remote return a URL

app/workers/scm/create_remote_repository_job.rb

@@ -37,6 +37,14 @@
 # Until then, a synchronous process is more failsafe.
 class Scm::CreateRemoteRepositoryJob < Scm::RemoteRepositoryJob
   def perform
-    send(repository_request.merge(action: :create))
+    response = send(repository_request.merge(action: :create))
+    repository.root_url = response['path']
+    repository.url = response['url']
+
+    unless repository.save
+      raise OpenProject::Scm::Exceptions::ScmError.new(
+        I18n.t('repositories.errors.remote_save_failed')
+      )
+    end
   end
 end

app/workers/scm/relocate_repository_job.rb

@@ -43,9 +43,16 @@ class Scm::RelocateRepositoryJob < Scm::RemoteRepositoryJob
   ##
   # POST to the remote managed repository a request to relocate the repository
   def relocate_remote
-    send(repository_request.merge(
+    response = send(repository_request.merge(
            action: :relocate,
            old_repository: repository.root_url))
+    repository.root_url = response['path']
+    repository.url = response['url']
+
+    unless repository.save
+      Rails.logger.error("Could not relocate the remote repository " \
+                         "#{repository.repository_identifier}.")
+    end
   end
 
   ##

app/workers/scm/remote_repository_job.rb

@@ -39,7 +39,6 @@ class Scm::RemoteRepositoryJob
 
   attr_reader :repository
 
-
   ##
   # Initialize the job, optionally saving the whole repository object
   # (use only when not serializing the job.)
@@ -65,9 +64,9 @@ class Scm::RemoteRepositoryJob
     response = ::Net::HTTP.start(uri.hostname, uri.port) do |http|
       http.request(req)
     end
+    info = try_to_parse_response(response.body)
 
     unless response.is_a? ::Net::HTTPSuccess
-      info = try_to_parse_response(response.body)
       raise OpenProject::Scm::Exceptions::ScmError.new(
               I18n.t('repositories.errors.remote_call_failed',
                      code: response.code,
@@ -75,6 +74,8 @@ class Scm::RemoteRepositoryJob
               )
             )
     end
+
+    info
   end
 
   def try_to_parse_response(body)

config/locales/en.yml

@@ -1569,6 +1569,7 @@ en:
       disabled_or_unknown_vendor: "The SCM vendor %{vendor} is disabled or no longer available."
       remote_call_failed: "Calling the managed remote failed with message '%{message}' (Code: %{code})"
       remote_invalid_response: "Received an invalid response from the managed remote."
+      remote_save_failed: "Could not save the repository with the parameters retrieved from the remote."
     git:
       instructions:
         managed_url: "This is the URL of the managed (local) Git repository."

doc/operation_guides/manual/repository-integration.md

@@ -95,6 +95,11 @@ Upon creating and deleting repositories in the frontend, OpenProject will POST t
 		"token": <Fixed access token passed to the endpoint>
 	}
 
+The endpoint is expected to return a JSON with at least a `message` property when the response is not successful (2xx).
+When the response is successful, it must at least return a `url` property that contains an accessible URL, an optionally, a `path` property to access the repository locally.
+Note that for Git repositories, OpenProject currently can only read them locally (i.e, through an NFS mount), so a path is mandatory here.
+For Subversion, you can either return a `file:///<path>` URL, or a local path.
+
 Our main use-case for this feature is to reduce the complexity of permission issues around Subversion mainly in packager, for which a simple Apache wrapper script is used in `extra/Apache/OpenProjectRepoman.pm`.
 This functionality is very limited, but may be extended when other use cases arise.
 It supports notifications for creating repositories (action `create`), moving repositories (action `relocate`, when a project's identifier has changed), and deleting repositories (action `delete`).

extra/Apache/OpenProjectRepoman.pm

@@ -153,7 +153,10 @@ sub _handle_request {
   return {
     success => JSON::PP::true,
     message => "The action has completed sucessfully.",
-    repository => $target
+    repository => $target,
+    path => $target,
+    # This is only useful in the packager context
+    url => 'file://' + $target
   };
 }
 

spec/features/repositories/create_repository_spec.rb

@@ -215,7 +215,9 @@ describe 'Create repository', type: :feature, js: true, selenium: true do
       }
 
       before do
-        stub_request(:post, url).to_return(status: 200)
+        stub_request(:post, url)
+          .to_return(status: 200,
+                     body: { success: true, url: 'file:///foo/bar' }.to_json)
       end
 
       it_behaves_like 'it can create the managed repository'

spec/features/repositories/repository_settings_spec.rb

@@ -160,7 +160,9 @@ describe 'Repository Settings', type: :feature, js: true do
           :managed
         )
 
-        stub_request(:post, url).to_return(status: 200)
+        stub_request(:post, url)
+          .to_return(status: 200,
+                     body: { success: true, url: 'file:///foo/bar' }.to_json)
 
         repo.save!
         repo

spec/services/scm/create_managed_repository_service_spec.rb

@@ -146,7 +146,7 @@ describe Scm::CreateManagedRepositoryService do
     }
 
     let(:repository) {
-      repo = Repository::Subversion.new(scm_type: :managed)
+      repo = FactoryGirl.build(:repository_subversion, scm_type: :managed)
       repo.project = project
       repo.configure(:managed, nil)
       repo
@@ -158,8 +158,18 @@ describe Scm::CreateManagedRepositoryService do
     end
 
     context 'with a remote callback' do
+      let(:returned_url) { 'file:///tmp/some/url/to/repo' }
+      let(:root_url) { '/tmp/some/url/to/repo' }
       before do
-        stub_request(:post, url).to_return(status: 200)
+        stub_request(:post, url)
+          .to_return(
+            status: 200,
+            body: { url: returned_url, path: root_url }.to_json
+          )
+
+        # Avoid setting up a second call to the remote during save
+        # since we only templated the repository, not created one!
+        expect(repository).to receive(:save).and_return(true)
       end
 
       it 'calls the callback' do
@@ -167,6 +177,9 @@ describe Scm::CreateManagedRepositoryService do
           .to receive(:new).and_call_original
 
         expect(service.call).to be true
+        expect(repository.root_url).to eq(root_url)
+        expect(repository.url).to eq(returned_url)
+
         expect(WebMock)
           .to have_requested(:post, url)
           .with(body: hash_including(action: 'create'))

spec/services/scm/delete_managed_repository_service_spec.rb

@@ -132,7 +132,7 @@ describe Scm::DeleteManagedRepositoryService do
 
     context 'with a valid remote' do
       before do
-        stub_request(:post, url).to_return(status: 200)
+        stub_request(:post, url).to_return(status: 200, body: {}.to_json )
       end
 
       it 'calls the callback' do

spec/support/scm/relocate_repository.rb

@@ -46,14 +46,18 @@ shared_examples_for 'repository can be relocated' do |vendor|
     let(:config) { { manages: url } }
 
     let(:repository) {
-      stub_request(:post, url).to_return(status: 200)
+      stub_request(:post, url)
+        .to_return(status: 200,
+                   body: { success: true, url: 'file:///foo/bar', path: '/tmp/foo/bar' }.to_json)
       FactoryGirl.create("repository_#{vendor}".to_sym,
                          project: project,
                          scm_type: :managed)
     }
 
     before do
-      stub_request(:post, url).to_return(status: 200)
+      stub_request(:post, url)
+        .to_return(status: 200,
+                   body: { success: true, url: 'file:///new/bar', path: '/tmp/new/bar' }.to_json)
     end
 
     it 'sends a relocation request when project identifier is updated' do