From e83188a1727f2a598f41872dd96e011c442f0b64 Mon Sep 17 00:00:00 2001 From: catmando Date: Thu, 31 May 2018 17:05:13 -0400 Subject: [PATCH] closes #99 --- .../reactive_record/isomorphic_base.rb | 103 +++++++++--------- lib/reactive_record/permissions.rb | 2 +- spec/batch1/misc/validate_spec.rb | 19 +++- spec/batch6/server_method_spec.rb | 13 +++ 4 files changed, 82 insertions(+), 55 deletions(-) diff --git a/lib/reactive_record/active_record/reactive_record/isomorphic_base.rb b/lib/reactive_record/active_record/reactive_record/isomorphic_base.rb index 70ac1683..5cb060e5 100644 --- a/lib/reactive_record/active_record/reactive_record/isomorphic_base.rb +++ b/lib/reactive_record/active_record/reactive_record/isomorphic_base.rb @@ -248,7 +248,7 @@ def self.gather_records(records_to_process, force, record_being_saved) def save_or_validate(save, validate, force, &block) if data_loading? sync! - elsif force || changed? + elsif force || changed? || (validate && new?) HyperMesh.load do ReactiveRecord.loads_pending! unless self.class.pending_fetches.empty? end.then { send_save_to_server(save, validate, force, &block) } @@ -370,25 +370,26 @@ def self.save_records(models, associations, acting_user, validate, save) end end reactive_records[model_to_save[:id]] = vectors[vector] = record = find_record(model, id, vector, save) # ??? || validate ??? - if record and record.respond_to?(:id) and record.id + next unless record + if record.respond_to?(:id) && record.id # we have an already exising activerecord model keys = record.attributes.keys attributes.each do |key, value| if is_enum?(record, key) - record.send("#{key}=",value) + record.send("#{key}=", value) elsif keys.include? key record[key] = value - elsif !value.nil? and aggregation = record.class.reflect_on_aggregation(key.to_sym) and !(aggregation.klass < ActiveRecord::Base) + elsif value && (aggregation = record.class.reflect_on_aggregation(key.to_sym)) && !(aggregation.klass < ActiveRecord::Base) aggregation.mapping.each_with_index do |pair, i| record[pair.first] = value[i] end elsif record.respond_to? "#{key}=" - record.send("#{key}=",value) + record.send("#{key}=", value) else # TODO once reading schema.rb on client is implemented throw an error here end end - elsif record + else # either the model is new, or its not even an active record model dont_save_list << record unless save keys = record.attributes.keys @@ -436,12 +437,12 @@ def self.save_records(models, associations, acting_user, validate, save) elsif parent.class.reflect_on_association(association[:attribute].to_sym).collection? #puts ">>>>>>>>>> #{parent.class.name}.send('#{association[:attribute]}') << #{reactive_records[association[:child_id]]})" dont_save_list.delete(parent) - if false and parent.new? - parent.send("#{association[:attribute]}") << reactive_records[association[:child_id]] if parent.new? - #puts "updated" - else + #if false and parent.new? + #parent.send("#{association[:attribute]}") << reactive_records[association[:child_id]] + # puts "updated" + #else #puts "skipped" - end + #end else #puts ">>>>ASSOCIATION>>>> #{parent.class.name}.send('#{association[:attribute]}=', #{reactive_records[association[:child_id]]})" parent.send("#{association[:attribute]}=", reactive_records[association[:child_id]]) @@ -450,61 +451,59 @@ def self.save_records(models, associations, acting_user, validate, save) end end if associations - #puts "!!!!!!!!!!!!associations updated" + # get rid of any records that don't require further processing, as a side effect + # we also save any records that need to be saved (these may be rolled back later.) + + reactive_records.keep_if do |reactive_record_id, record| + next false unless record # throw out items where we couldn't find a record + next true if record.frozen? # skip (but process later) frozen records + next true if dont_save_list.include?(record) # skip if the record is on the don't save list + next true if record.changed.include?(record.class.primary_key) # happens on an aggregate + next false if record.id && !record.changed? # throw out any existing records with no changes + # if we get to here save the record and return true to keep it + op = new_models.include?(record) ? :create_permitted? : :update_permitted? + record.check_permission_with_acting_user(acting_user, op).save(validate: false) || true + end - has_errors = false - error_messages = [] + # if called from ServerDataCache then save and validate are both false, and we just return the + # vectors. ServerDataCache has its own transaction which it will rollback when its done - #puts "ready to start saving... dont_save_list = #{dont_save_list}" + return vectors unless save || validate - saved_models = reactive_records.collect do |reactive_record_id, model| - #puts "saving rr_id: #{reactive_record_id} model.object_id: #{model.object_id} frozen? <#{model.frozen?}>" - next unless model - if !save|| model.frozen? || dont_save_list.include?(model) || model.changed.include?(model.class.primary_key) - # the above check for changed including the private key happens if you have an aggregate that includes its own id - # puts "validating frozen model #{model.class.name} #{model} (reactive_record_id = #{reactive_record_id})" - valid = model.valid? - # puts "has_errors before = #{has_errors}, validate= #{validate}, !valid= #{!valid} (validate and !valid) #{validate and !valid}" - has_errors ||= (validate and !valid) - # puts "validation complete errors = <#{!valid}>, #{model.errors.messages} has_errors #{has_errors}" - error_messages << [model, model.errors.messages] unless valid - [reactive_record_id, model.class.name, model.__hyperloop_secure_attributes(acting_user), (valid ? nil : model.errors.messages)] - elsif !model.id || model.changed? - # puts "saving #{model.class.name} #{model} (reactive_record_id = #{reactive_record_id})" - saved = model.check_permission_with_acting_user(acting_user, new_models.include?(model) ? :create_permitted? : :update_permitted?).save(validate: validate) - has_errors ||= !saved - messages = model.errors.messages if (validate and !saved) or (!validate and !model.valid?) - error_messages << [model, messages] if messages - # puts "saved complete errors = <#{!saved}>, #{messages} has_errors #{has_errors}" - [reactive_record_id, model.class.name, model.__hyperloop_secure_attributes(acting_user), messages] - end - end.compact + # otherwise either save or validate or both are true, so we convert the remaining react_records into + # arrays with the id, model name, legal attributes, and any error messages. We also accumulate + # the all the error messages during a save so we can dump them to the server log. - if has_errors && save - ::Rails.logger.debug "\033[0;31;1mERROR: HyperModel saving records failed:\033[0;30;21m" - error_messages.each do |model, messages| - messages.each do |message| - ::Rails.logger.debug "\033[0;31;1m\t#{model}: #{message}\033[0;30;21m" - end - end - raise "HyperModel saving records failed!" + all_messages = [] + + saved_models = reactive_records.collect do |reactive_record_id, model| + messages = model.errors.messages if validate && !model.valid? + all_messages << messages if save && messages + attributes = model.__hyperloop_secure_attributes(acting_user) + [reactive_record_id, model.class.name, attributes, messages] end - if save || validate + # if we are not saving (i.e. just validating) then we rollback the transaction - {success: true, saved_models: saved_models } + raise ActiveRecord::Rollback, 'This Rollback is intentional!' unless save - else - # vectors.each { |vector, model| model.reload unless model.nil? or model.new_record? or model.frozen? } - vectors + # if there are error messages then we dump them to the server log, and raise an error + # to roll back the transaction and set success to false. + unless all_messages.empty? + ::Rails.logger.debug "\033[0;31;1mERROR: HyperModel saving records failed:\033[0;30;21m" + all_messages.each do |message| + ::Rails.logger.debug "\033[0;31;1m\t#{model}: #{message}\033[0;30;21m" + end + raise 'HyperModel saving records failed!' end end + { success: true, saved_models: saved_models } + rescue Exception => e - #ReactiveRecord::Pry.rescued(e) - if save + if save || validate {success: false, saved_models: saved_models, message: e} else {} diff --git a/lib/reactive_record/permissions.rb b/lib/reactive_record/permissions.rb index cad1815a..6e90b3b8 100644 --- a/lib/reactive_record/permissions.rb +++ b/lib/reactive_record/permissions.rb @@ -103,7 +103,7 @@ def check_permission_with_acting_user(user, permission, *args) self.acting_user = old self else - Hyperloop::InternalPolicy.raise_operation_access_violation(:crud_access_violation, "for #{permission}(#{args}) acting_user: #{user}") + Hyperloop::InternalPolicy.raise_operation_access_violation(:crud_access_violation, "for #{self} - #{permission}(#{args}) acting_user: #{user}") end end diff --git a/spec/batch1/misc/validate_spec.rb b/spec/batch1/misc/validate_spec.rb index 8fbcb1f1..7d58a474 100644 --- a/spec/batch1/misc/validate_spec.rb +++ b/spec/batch1/misc/validate_spec.rb @@ -15,14 +15,28 @@ regulate_all_broadcasts { |policy| policy.send_all } allow_change(to: :all, on: [:create, :update, :destroy]) { true } end - size_window(:large, :landscape) + #size_window(:large, :landscape) User.validates :last_name, exclusion: { in: %w[f**k], message: 'no swear words allowed' } + TestModel.validates_presence_of :child_models client_option raise_on_js_errors: :off end + it "can validate the presence of an association" do + expect_promise do + @test_model = TestModel.new + @test_model.validate.then { |test_model| test_model.errors.messages } + end.not_to be_empty + expect_promise do + @test_model.child_models << ChildModel.new + @test_model.validate.then { |test_model| test_model.errors.messages } + end.to be_empty + expect(TestModel.count).to be_zero + expect(ChildModel.count).to be_zero + end + it "can validate only using the validate method" do expect_promise do - User.new(last_name: 'f**k').validate.tap { |p| puts "got the promise in spec" }.then do |new_user| + User.new(last_name: 'f**k').validate.then do |new_user| new_user.errors.messages end end.to eq("last_name"=>[{"message"=>"no swear words allowed"}]) @@ -93,4 +107,5 @@ class << self end expect(page).to have_content('.valid? true') end + end diff --git a/spec/batch6/server_method_spec.rb b/spec/batch6/server_method_spec.rb index 788b60f8..94b32ddf 100644 --- a/spec/batch6/server_method_spec.rb +++ b/spec/batch6/server_method_spec.rb @@ -42,6 +42,7 @@ def server_method_count end server_method(:test, default: 0) { TodoItem.server_method_count += 1 } end + TestModel.server_method(:test) { child_models.count } end TodoItem.create mount 'ServerMethodTester' do @@ -75,4 +76,16 @@ class ServerMethodTester < Hyperloop::Component end end.to eq(5) end + + it "the server method can access any unsaved associations" do + expect_promise do + test_model = TestModel.new + ChildModel.new(test_model: test_model) + ReactiveRecord.load do + test_model.test + end + end.to eq(1) + expect(TestModel.count).to be_zero + expect(ChildModel.count).to be_zero + end end