Skip to content
This repository has been archived by the owner on Mar 1, 2018. It is now read-only.

false error after multiple pkg import from shared #126

Open
dayglojesus opened this issue Jan 25, 2012 · 5 comments
Open

false error after multiple pkg import from shared #126

dayglojesus opened this issue Jan 25, 2012 · 5 comments
Labels

Comments

@dayglojesus
Copy link
Contributor

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

@jnraine
Copy link
Owner

jnraine commented Jan 25, 2012

Can you provide some logs from the issue?

@treydock
Copy link
Contributor

treydock commented Apr 1, 2012

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
Copy link
Owner

jnraine commented Apr 1, 2012

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
Copy link
Contributor

treydock commented Apr 2, 2012

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
Copy link
Owner

jnraine commented Apr 2, 2012

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
[email protected]
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:
#126 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants