Permalink
Browse files

Various performance fixes mostly related to avoid extraneous method i…

…nvocation
  • Loading branch information...
1 parent 80ed6de commit c4657ad7f30de9aa5caad000158fdb43755cb5eb @cheald cheald committed Sep 28, 2012
View
@@ -3,10 +3,10 @@ module MongoMapper
module Extensions
module Time
def to_mongo(value)
- if value.nil? || value == ''
+ if !value.present?
nil
else
- time_class = ::Time.try(:zone).present? ? ::Time.zone : ::Time
+ time_class = ::Time.try(:zone) || ::Time
time = value.is_a?(::Time) ? value : time_class.parse(value.to_s)
at(time.to_f).utc if time # ensure milliseconds are preserved with to_f (issue #308)
end
View
No changes.
View
@@ -181,7 +181,7 @@ def persisted?
end
def attributes=(attrs)
- return if attrs.blank?
+ return if attrs == nil or attrs.blank?
attrs.each_pair do |key, value|
if respond_to?(:"#{key}=")
@@ -244,10 +244,16 @@ def id=(value)
self[:_id] = value
end
- def [](name)
- read_key(name)
+ def read_key(key_name)
+ if key = keys[key_name.to_s]
+ value = key.get(instance_variable_get(:"@#{key_name}"))
+ set_parent_document(key, value) if key.embeddable?
+ instance_variable_set(:"@#{key_name}", value)
+ end
end
+ alias_method :[], :read_key
+
def []=(name, value)
ensure_key_exists(name)
write_key(name, value)
@@ -271,7 +277,7 @@ def embedded_keys
private
def load_from_database(attrs)
- return if attrs.blank?
+ return if attrs == nil or attrs.blank?
attrs.each do |key, value|
if respond_to?(:"#{key}=") && !self.class.key?(key)
self.send(:"#{key}=", value)
@@ -282,7 +288,7 @@ def load_from_database(attrs)
end
def ensure_key_exists(name)
- self.class.key(name) unless respond_to?("#{name}=")
+ self.class.key(name) unless respond_to?(:"#{name}=")
@bkeepers
bkeepers Sep 28, 2012

I think this could be used to cause a DOS, eventually exhausting memory by creating records with random attributes. attr_accessible would protect against that, but still an issue if someone is using attr_protected.

@jnunemaker
jnunemaker Sep 28, 2012 collaborator

Now that I think back I am pretty sure that was why I used strings.

end
def set_parent_document(key, value)
@@ -291,21 +297,13 @@ def set_parent_document(key, value)
end
end
- def read_key(key_name)
- if key = keys[key_name.to_s]
- value = key.get(instance_variable_get(:"@#{key_name}"))
- set_parent_document(key, value)
- instance_variable_set(:"@#{key_name}", value)
- end
- end
-
def read_key_before_type_cast(name)
instance_variable_get(:"@#{name}_before_type_cast")
end
def write_key(name, value)
key = keys[name.to_s]
- set_parent_document(key, value)
+ set_parent_document(key, value) if key.embeddable?
instance_variable_set :"@#{name}_before_type_cast", value
instance_variable_set :"@#{name}", key.set(value)
end
View
@@ -5,6 +5,8 @@ module Keys
class Key
attr_accessor :name, :type, :options, :default
+ ID_STR = '_id'
+
def initialize(*args)
options_from_args = args.extract_options!
@name, @type = args.shift.to_s, args.shift
@@ -20,8 +22,14 @@ def ==(other)
end
def embeddable?
- return false unless type.respond_to?(:embeddable?)
- type.embeddable?
+ # This is ugly, but it's fast. We can't use ||= because false is an expected and common value.
+ @embeddable = @embeddable != nil ? @embeddable : begin
+ if type.respond_to?(:embeddable?)
+ type.embeddable?
+ else
+ false
+ end
+ end
end
def number?
@@ -34,9 +42,9 @@ def default?
def get(value)
# Special Case: Generate default _id on access
- value = default_value if name == '_id' && value.nil?
+ value = default_value if value.nil? and name == ID_STR
- if options[:typecast].present?
+ if options[:typecast]
type.from_mongo(value).map! { |v| typecast_class.from_mongo(v) }
else
type.from_mongo(value)
@@ -45,7 +53,7 @@ def get(value)
def set(value)
type.to_mongo(value).tap do |values|
- if options[:typecast].present?
+ if options[:typecast]
values.map! { |v| typecast_class.to_mongo(v) }
end
end

0 comments on commit c4657ad

Please sign in to comment.