false error after multiple pkg import from shared

[dayglojesus]

Importing multiple packages from the Shared packages produces a false error. The packages are imported properly, but munkiserver reports that the op failed.

[jnraine]

Can you provide some logs from the issue?

[treydock]

I just ran into this. Here's the error...

The last line shows the reason.

  Package Load (0.7ms)  SELECT `packages`.* FROM `packages` WHERE `packages`.`unit_id` = 4 AND `packages`.`package_branch_id` = 3
  PackageBranch Load (0.2ms)  SELECT `package_branches`.* FROM `package_branches` WHERE `package_branches`.`id` = 3 LIMIT 1
   (0.6ms)  SELECT COUNT(*) FROM `packages` WHERE `packages`.`package_branch_id` = 3 AND `packages`.`unit_id` = 4 AND `packages`.`environment_id` IS NULL
CACHE: Expiring the catalogs for {:type=>:catalog, :unit=>4, :environment_id=>1}
  SQL (0.3ms)  INSERT INTO `packages` (`RestartAction`, `autoremove`, `created_at`, `description`, `environment_id`, `filename`, `force_install_after_date`, `icon_id`, `installed_size`, `installer_choices_xml`, `installer_item_checksum`, `installer_item_location`, `installer_item_size`, `installer_type`, `installs`, `maximum_os_version`, `minimum_os_version`, `package_branch_id`, `package_category_id`, `package_path`, `postinstall_script`, `postuninstall_script`, `preinstall_script`, `preuninstall_script`, `raw_mode_id`, `raw_tags`, `receipts`, `shared`, `supported_architectures`, `unattended_install`, `unattended_uninstall`, `uninstall_method`, `uninstall_script`, `uninstallable`, `uninstaller_item_location`, `uninstaller_item_size`, `unit_id`, `updated_at`, `use_installer_choices`, `version`, `version_tracker_version`) VALUES ('None', 0, '2011-12-06 01:14:24', 'Adobe Flash Player', 1, NULL, NULL, 45, 36776, NULL, NULL, '2011.12.05.1914244524_Adobe Flash Player-11.1.102.55.dmg', 14076, '', '--- []\n\n', '', '10.6.0', 3, 2, NULL, '', '', '', '', 1, '--- \nblocking_applications: \n- Firefox.app\n- Safari.app\n- Opera.app\ninstaller_item_hash: 7835076d5a855fd384ab1756351dc36174031b6c2a7ddb990832d987a154a336\n', '--- \n- filename: Adobe Flash Player.pkg\n installed_size: 36776\n packageid: com.adobe.pkg.FlashPlayer\n version: 11.1.102.55\n', 0, '--- \n- \"\"\n', 0, 0, 'removepackages', '', 1, '', NULL, 4, '2012-02-07 19:08:18', 0, '11.1.102.55', NULL)
   (0.2ms)  UPDATE `icons` SET `updated_at` = '2012-04-01 19:10:41' WHERE `icons`.`id` = 45
[paperclip] Saving attachments.
CACHE: Expiring the catalogs for {:type=>:catalog, :unit=>4, :environment_id=>1}
   (0.1ms)  COMMIT
WARNING: Can't mass-assign protected attributes: id

Instead of using Package.new in the import_package method, use dup or clone? Issue is dup doesn't work until I think 3.1. I use it in 3.1.3 with great success. However seems in 3.0.10 it doesn't remove ID resulting in PRIMARY KEY conflicts.

Here's the initial work, but it's not fully tested. Will do pull request once I get a chance to test on my production env where I got the false error.

diff --git a/app/models/package.rb b/app/models/package.rb
index 1efe01a..68998f3 100644
--- a/app/models/package.rb
+++ b/app/models/package.rb
@@ -690,7 +690,13 @@ class Package < ActiveRecord::Base
   # new package will inherite most of the attributes from orginal
   # except for the list of the attributes below
   def self.import_package(unit, shared_package)
-    package = Package.new(shared_package.attributes)
+    # Hack to remove ID since dup in < Rails-3.1 doesn't work
+    shared_package.id = nil
+    package = shared_package.dup
+    # Dup all the has_many associations
+    association_names(:has_many).each do |assoc|
+      package[assoc.to_sym] = shared_package.send(assoc).dup unless shared_package.send(assoc).blank?
+    end
     # Do custom stuff to imported package
     package.unit = unit
     package.environment = Environment.start
@@ -700,7 +706,18 @@ class Package < ActiveRecord::Base
     package.shared = false
     package
   end
-  
+
+  def self.association_names(type)
+    names = []
+    Package.reflections.each do |key, value|
+      if value.instance_of?(ActiveRecord::Reflection::AssociationReflection) && value.macro == type
+        names << key
+      end
+    end
+    names
+  end
+
+ 
   # over write the default get description, check if nil then get the description from version_trackers
   def description
     value = super

Bit more complicated than using new, but if the intent is the create a copy of the shared_package this may be the better method rather than adding id to attr_accessible.

[jnraine]

Thanks for this. I'm a little weary of dup'ing all the associations along with the record -- importing a shared package might cause all kinds of records to be dup'ed, many which shouldn't be. For example, importing a package with dependencies ('requires') would cause the imported package to have references to packages from the other unit.

Perhaps something as simple as this:

def self.import_package(package, unit)
  returning(package.dup) do |p|
    p.id = nil
    p.unit = unit
    p.environment = Environment.start
    p.shared = false
  end
end

[treydock]

Yep that's better. I was confusing "install_items" association with the "installs" attribute.

I got this warning with #returning

DEPRECATION WARNING: Object#returning has been deprecated in favor of Object#tap.

tap doesn't seem to behave the same when making changes as returning.

# app/controllers/packages_controller.rb
  def import_multiple_shared
    shared_packages = Package.shared.where("unit_id != #{current_unit.id}").find(params[:selected_records])
    results = []
    shared_packages.each do |shared_package|
      package = Package.import_package(shared_package, current_unit)
....

# app/model/package.rb
  def self.import_package(package, unit)
    p = package.dup
    # Do custom stuff to imported package
    p.id = nil # Hack to remove ID since dup in < Rails-3.1 doesn't work
    p.unit = unit
    p.environment = Environment.start
    p.shared = false
    p   
  end 

[jnraine]

Yeah, i like tap better actually, but is only Ruby 1.9 so I opted for returning, at least for now.

On Apr 1, 2012, at 5:27 PM, treydock reply@reply.github.com wrote:

Yep that's better. I was confusing "install_items" association with the "installs" attribute.

I got this warning with #returning

DEPRECATION WARNING: Object#returning has been deprecated in favor of Object#tap.

tap doesn't seem to behave the same when making changes as returning.

# app/controllers/packages_controller.rb
 def import_multiple_shared
   shared_packages = Package.shared.where("unit_id != #{current_unit.id}").find(params[:selected_records])
   results = []
   shared_packages.each do |shared_package|
     package = Package.import_package(shared_package, current_unit)
....

# app/model/package.rb
 def self.import_package(package, unit)
   p = package.dup
   # Do custom stuff to imported package
   p.id = nil # Hack to remove ID since dup in < Rails-3.1 doesn't work
   p.unit = unit
   p.environment = Environment.start
   p.shared = false
   p
 end

---

Reply to this email directly or view it on GitHub: https://github.com/jnraine/munkiserver/issues/126#issuecomment-4870786