Merge pull request #1281 from hennevogel/remove_activexml_from_packag…

…e_controller

[webui] Remove ActiveXML from linking packages

src/api/app/controllers/webui/package_controller.rb

@@ -44,7 +44,7 @@ class Webui::PackageController < Webui::WebuiController
                                             :update_build_log, :devel_project, :buildresult, :rpmlint_result,
                                             :rpmlint_log, :meta, :attributes, :repositories, :files]
 
-  before_filter :do_backend_login, only: [:branch, :save_new_link, :save_modified_file, :save_meta, :change_flag,
+  before_filter :do_backend_login, only: [:save_modified_file, :save_meta, :change_flag,
                                           :abort_build, :trigger_rebuild, :wipe_binaries, :remove]
 
   prepend_before_filter :lockout_spiders, :only => [:revisions, :dependency, :rdiff, :binary, :binaries, :requests]
@@ -533,90 +533,72 @@ def branch_dialog
   end
 
   def branch
-    begin
-      path = "/source/#{CGI.escape(params[:project])}/#{CGI.escape(params[:package])}?cmd=branch"
-      result = ActiveXML::Node.new(frontend.transport.direct_http( URI(path), :method => 'POST', :data => ''))
-      result_project = result.find_first( "/status/data[@name='targetproject']" ).text
-      result_package = result.find_first( "/status/data[@name='targetpackage']" ).text
-    rescue ActiveXML::Transport::Error => e
-      message = e.summary
-      if e.code == 'double_branch_package'
-        flash[:notice] = 'You already branched the package and got redirected to it instead'
-        bprj, bpkg = message.split('exists: ')[1].split('/', 2) # Hack to find out branch project / package
-        redirect_to controller: :package, action: :show, project: bprj, package: bpkg
-        return
-      else
-        flash[:error] = message
-        redirect_to controller: :package, action: :show, project: params[:project], package: params[:package]
-        return
-      end
-    end
-    flash[:success] = "Branched package #{@project} / #{@package}"
-    redirect_to controller: :package, action: :show,
-                project: result_project, package: result_package
+    authorize @package, :branch?
+    authorize Project.new(name: User.current.branch_project_name(@project.name)), :create?
+
+    BranchPackage.new(project: @project.name, package: @package.name).branch
+    Event::BranchCommand.create(project: @project.name, package: @package.name,
+                                targetproject: User.current.branch_project_name(@project.name), targetpackage: @package.name,
+                                user: User.current.login)
+    redirect_to(package_show_path(project: User.current.branch_project_name(@project.name), package: @package),
+                notice: "Successfully branched package")
+  rescue BranchPackage::DoubleBranchPackageError => e
+      redirect_to(package_show_path(project: User.current.branch_project_name(@project.name), package: @package),
+                  notice: 'You have already branched this package')
+  rescue APIException => e
+      redirect_to :back, error: e.message
   end
 
-
   def save_new_link
-    @linked_project = params[:linked_project].strip
-    @linked_package = params[:linked_package].strip
-    @target_package = params[:target_package].strip
-    @revision       = nil
-    @current_revision = true if params[:current_revision]
-
-    unless Package.valid_name? @linked_package
-      flash[:error] = "Invalid package name: '#{@linked_package}'"
-      redirect_to controller: :project, action: :new_package_branch, project: params[:project]
-      return
-    end
-
-    unless Project.valid_name? @linked_project
-      flash[:error] = "Invalid project name: '#{@linked_project}'"
-      redirect_to controller: :project, action: :new_package_branch, project: params[:project]
-      return
-    end
-
-    unless Package.exists_by_project_and_name(@linked_project, @linked_package)
-      flash[:error] = "Unable to find package '#{@linked_package}' in project '#{@linked_project}'."
-      redirect_to controller: :project, action: :new_package_branch, project: @project
-      return
-    end
-
-    @target_package = @linked_package if @target_package.blank?
-    unless Package.valid_name? @target_package
-      flash[:error] = "Invalid target package name: '#{@target_package}'"
-      redirect_to controller: :project, action: :new_package_branch, project: @project
-      return
-    end
-    if Package.exists_by_project_and_name @project.name, @target_package
-      flash[:error] = "Package '#{@target_package}' already exists in project '#{@project}'"
-      redirect_to controller: :project, action: :new_package_branch, project: @project
-      return
-    end
-
-    dirhash = Package.dir_hash(@linked_project, @linked_package)
-    revision = dirhash['xsrcmd5'] || dirhash['rev']
-    unless revision
-      flash[:error] = "Unable to branch package '#{@linked_package}', it has no source revision yet"
-      redirect_to controller: :project, action: :new_package_branch, project: @project
-      return
+    # Are we linking a package from a remote instance?
+    # Then just try, the remote instance will handle checking for existence
+    # authorization etc.
+    if Project.find_remote_project(params[:linked_project])
+      source_project_name = params[:linked_project]
+      source_package_name = params[:linked_package]
+    # If we are linking a local package we have to do it ourselves
+    else
+      source_package = Package.find_by_project_and_name(params[:linked_project], params[:linked_package])
+      unless source_package
+        redirect_to :back, error: "Failed to branch: Package does not exist."
+        return
+      end
+      authorize source_package, :branch?
+      source_project_name = source_package.project.name
+      source_package_name = source_package.name
+    end
+    revision = nil
+    unless params[:current_revision].blank?
+      dirhash = Package.dir_hash(source_project_name, source_package_name)
+      if dirhash['error']
+        redirect_to :back, error: dirhash['error']
+      end
+      revision = dirhash['xsrcmd5'] || dirhash['rev']
+      unless revision
+        redirect_to :back, error: "Failed to branch: Package has no source revision yet"
+        return
+      end
     end
 
-    @revision = revision if @current_revision
+    params[:target_package] = source_package_name if params[:target_package].blank?
 
-    logger.debug "link params doing branch: #{@linked_project}, #{@linked_package}"
     begin
-      path = Package.source_path(@linked_project, @linked_package, nil, { cmd:            :branch,
-                                                                          target_project: @project.name,
-                                                                          target_package: @target_package})
-      path += "&rev=#{CGI.escape(@revision)}" if @revision
-      frontend.transport.direct_http( URI(path), :method => 'POST', :data => '')
-      flash[:success] = "Branched package #{@project.name} / #{@target_package}"
-    rescue ActiveXML::Transport::Error => e
-      flash[:error] = e.summary
+      BranchPackage.new(project: source_project_name,
+                        package: source_package_name,
+                        target_project: @project.name,
+                        target_package: params[:target_package],
+                        rev: revision).branch
+      Event::BranchCommand.create(project: source_project_name, package: source_package_name,
+                                  targetproject: @project.name, targetpackage: params[:target_package],
+                                  user: User.current.login)
+      redirect_to(package_show_path(project: @project, package: params[:target_package]),
+                  notice: "Successfully branched package")
+    rescue BranchPackage::DoubleBranchPackageError => e
+      redirect_to(package_show_path(project: @project, package: params[:target_package]),
+                  notice: 'You have already branched this package')
+    rescue => e
+      redirect_to :back, error: "Failed to branch: #{e.message}"
     end
-
-    redirect_to controller: :package, action: :show, project: @project, package: @target_package
   end
 
   def save

src/api/app/controllers/webui/webui_controller.rb

@@ -36,6 +36,7 @@ def handle_unverified_request
        when "update?" then "update"
        when "edit?" then "edit"
        when "destroy?" then "delete"
+       when "branch?" then "branch"
        else exception.query
     end
     if exception.message

src/api/app/helpers/branch_package.rb

@@ -599,7 +599,7 @@ def set_target_project
       @auto_cleanup = ::Configuration.cleanup_after_days
     end
     if @target_project
-      valid_project_name! @target_project
+      raise InvalidProjectNameError, "invalid project name '#{@target_project}'" if !Project.valid_name?(@target_project)
     end
   end
 end

src/api/app/policies/application_policy.rb

@@ -3,6 +3,7 @@ class ApplicationPolicy
 
   def initialize(user, record)
     raise Pundit::NotAuthorizedError, "must be logged in" unless user
+    raise Pundit::NotAuthorizedError, "record does not exist" unless record
     @user = user
     @record = record
   end

src/api/app/policies/attrib_policy.rb

@@ -1,11 +1,4 @@
 class AttribPolicy < ApplicationPolicy
-  attr_reader :user, :attrib
-
-  def initialize(user, attrib)
-    raise Pundit::NotAuthorizedError, "Sorry, you must be signed in to perform this action." unless user
-    @user = user
-    @attrib = attrib
-  end
 
   def create?
     # Admins can write everything
@@ -14,24 +7,24 @@ def create?
     # check for modifiable_by rules
     type_perms = []
     namespace_perms = []
-    if @attrib.attrib_type
-      type_perms = @attrib.attrib_type.attrib_type_modifiable_bies
-      if @attrib.attrib_type.attrib_namespace
-        namespace_perms = @attrib.attrib_type.attrib_namespace.attrib_namespace_modifiable_bies
+    if @record.attrib_type
+      type_perms = @record.attrib_type.attrib_type_modifiable_bies
+      if @record.attrib_type.attrib_namespace
+        namespace_perms = @record.attrib_type.attrib_namespace.attrib_namespace_modifiable_bies
       end
     end
     # no specific rules set for the attribute, check if the user can modify the container
-    if type_perms.empty? && namespace_perms.empty? && @attrib.container.present?
-      if @attrib.container.kind_of? Project
-        return @user.can_modify_project?(@attrib.container)
+    if type_perms.empty? && namespace_perms.empty? && @record.container.present?
+      if @record.container.kind_of? Project
+        return @user.can_modify_project?(@record.container)
       else
-        return @user.can_modify_package?(@attrib.container)
+        return @user.can_modify_package?(@record.container)
       end
     else
       type_perms.each do |rule|
         next if rule.user and rule.user != @user
         next if rule.group and not @user.is_in_group? rule.group
-        next if rule.role and not @user.has_local_role?(rule.role, @attrib.container)
+        next if rule.role and not @user.has_local_role?(rule.role, @record.container)
         return true
       end
       namespace_perms.each do |rule|

src/api/app/policies/group_policy.rb

@@ -1,26 +1,16 @@
 class GroupPolicy < ApplicationPolicy
-  attr_reader :user, :group
-
-  def initialize(user, group)
-    raise Pundit::NotAuthorizedError, "Sorry, you must be signed in to perform this action." unless user
-    @user = user
-    @group = group
-  end
 
   def create?
     # Only admins can create new groups atm
     @user.is_admin?
   end
 
   def update?
-    # is update okay at all?
-    return create?  if group.nil?
-
     # admins can do it always
     return true if @user.is_admin?
 
     # and also group maintainers
-    group.group_maintainers.where(user: @user).length > 0
+    @record.group_maintainers.where(user: @user).length > 0
   end
 
   def destroy?

src/api/app/policies/package_policy.rb

@@ -1,16 +1,20 @@
 class PackagePolicy < ApplicationPolicy
-  attr_reader :user, :package
 
-  def initialize(user, package)
-    @user = user
-    @package = package
+  def branch?
+    # same as Package.check_source_access!
+    if @record.disabled_for?('sourceaccess', nil, nil) or record.project.disabled_for?('sourceaccess', nil, nil)
+      unless @user.can_source_access?(@record)
+        return false
+      end
+    end
+    true
   end
 
   def update?
-    @user.can_modify_package?(@package)
+    @user.can_modify_package?(@record)
   end
 
   def destroy?
-    @user.can_modify_package?(@package)
+    @user.can_modify_package?(@record)
   end
 end

src/api/app/policies/project_policy.rb

@@ -1,22 +1,16 @@
 class ProjectPolicy < ApplicationPolicy
-  attr_reader :user, :project
-
-  def initialize(user, project)
-    @user = user
-    @project = project
-  end
 
   def create?
-    @project.check_write_access
+    @record.check_write_access
   end
 
   def update?
     # The ordering is important because of the lock status check
-    return false unless @user.can_modify_project?(@project)
+    return false unless @user.can_modify_project?(@record)
     return true if @user.is_admin?
 
     # Regular users are not allowed to modify projects with remote references
-    !@project.is_remote? && !@project.has_remote_repositories?
+    !@record.is_remote? && !@record.has_remote_repositories?
   end
 
   def destroy?

src/api/app/views/webui/project/new_package_branch.html.erb

@@ -24,12 +24,12 @@ package.</p>
 <%= form_tag controller: "package", action: "save_new_link", project: @project.name do %>
 <p>
   <strong>Name of original project:</strong><br/>
-  <%= text_field_tag 'linked_project', @linked_project, :size => 80, :id => 'linked_project' %><br/>
+  <%= text_field_tag 'linked_project', nil, :size => 80, :id => 'linked_project' %><br/>
   <strong>Name of package in original project:</strong><br/>
-  <%= text_field_tag 'linked_package', @linked_package, :size => 80, :id => 'linked_package' %><br/>
+  <%= text_field_tag 'linked_package', nil, :size => 80, :id => 'linked_package' %><br/>
   <strong>Name of branched package in target project:</strong> (Leave blank to use the
   same name as in the original project) <br/>
-  <%= text_field_tag 'target_package', @target_package, :size => 80 %><br/>
+  <%= text_field_tag 'target_package', nil, :size => 80 %><br/>
   <%= check_box_tag 'current_revision', false %>Stay on current revision, don't merge future upstream changes automatically
 </p>
 <p><%= submit_tag "Create Branch" %></p>

src/api/test/functional/webui/maintenance_workflow_test.rb

@@ -31,7 +31,7 @@ def test_full_maintenance_workflow
     find(:css, '#branch_dialog').must_have_text %r{Do you really want to branch package}
     find_button('Ok').click
 
-    find(:css, '#flash-messages').must_have_text %r{Branched package BaseDistro2\.0:LinkedUpdateProject.*pack2}
+    find(:css, '#flash-messages').must_have_text %r{Successfully branched package}
 
     # do not die with unchanged package
     Suse::Backend.put("/source/home:tom:branches:BaseDistro2.0:LinkedUpdateProject/pack2/DUMMY_FILE", "dummy")