(unique) change scoped validations to a #where query

spider-gazelle · Jun 6, 2019 · 9b0c2eb · 9b0c2eb
1 parent 4f08082
commit 9b0c2eb
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 53 deletions.
diff --git a/README.md b/README.md
@@ -4,6 +4,19 @@
 
 Extending [ActiveModel](https://github.com/spider-gazelle/active-model) for attribute definitions, callbacks and validations
 
+- [RethinkDB ORM for Crystal Lang](#rethinkdb-orm-for-crystal-lang)
+  - [Callbacks](#callbacks)
+  - [Associations](#associations)
+    - [`belongs_to`](#belongsto)
+    - [`has_many`](#hasmany)
+    - [`has_one`](#hasone)
+    - [Dependency](#dependency)
+  - [Indexes](#indexes)
+  - [Changefeeds](#changefeeds)
+  - [Validations](#validations)
+    - [`ensure_unique`](#ensureunique)
+  - [Timestamps](#timestamps)
+
 ## Callbacks
 
 Register callbacks for `save`, `update`, `create` and `destroy` by setting the corresponding before/after callback handler.
@@ -146,7 +159,8 @@ Builds on [active-model's validation](https://github.com/spider-gazelle/active-m
 ### `ensure_unique`
 
 Fails to validate if field with duplicate value present in db.
-If scope is set, the callback/block must accept a tuple with types matching the scope.
+If scope is set, the callback/block signature must be a tuple with types matching that of the scope.
+The field(s) are set with the result of the transform block upon successful validation
 
 Parameter               |                                                         | Default
 ----------------------- | ------------------------------------------------------- | -------

diff --git a/spec/associations_spec.cr b/spec/associations_spec.cr
@@ -105,8 +105,7 @@ describe RethinkORM::Associations do
     coffee.programmer = programmer
     coffee.save
 
-    found_coffee = Coffee.by_programmer_id(programmer.id).to_a.first?
-    found_coffee.should eq coffee
+    Coffee.by_programmer_id(programmer.id).first.should eq coffee
 
     coffee.destroy
     programmer.destroy

diff --git a/spec/spec_models.cr b/spec/spec_models.cr
@@ -143,14 +143,14 @@ class Snowflake < RethinkORM::Base
     personality.downcase
   end
 
-  ensure_unique scope: [:taste, :vibe], field: :taste do |(taste, vibe)|
-    "#{taste.downcase}-#{vibe.downcase}"
+  ensure_unique scope: [:taste, :vibe], field: :taste do |taste, vibe|
+    {taste.downcase, vibe.downcase}
   end
 
   ensure_unique scope: [:vibe, :size], field: :vibe, callback: :dip
 
   def dip(vibe : String, size : Int32)
-    "#{vibe.downcase}-#{size}"
+    {vibe.downcase, size}
   end
 
   def id(value : T) forall T

diff --git a/spec/unique_spec.cr b/spec/unique_spec.cr
@@ -26,8 +26,8 @@ describe RethinkORM::Validators do
     end
 
     it "accepts a transform block for the field" do
-      special_snowflake = Snowflake.create!(personality: "gentle")
-      louder_snowflake = Snowflake.new(personality: "GENTLE")
+      special_snowflake = Snowflake.create!(personality: "GENTLE")
+      louder_snowflake = Snowflake.new(personality: "gentle")
 
       special_snowflake.valid?.should be_true
       louder_snowflake.valid?.should be_false
@@ -36,23 +36,25 @@ describe RethinkORM::Validators do
     end
 
     it "passes scoped fields to transform block" do
-      fresh_flake = Snowflake.create!(vibe: "awful", taste: "great")
-      fresh_flake.taste.should eq "great-awful"
+      fresh_flake = Snowflake.new(vibe: "awful", taste: "GREAT")
+      fresh_flake.taste.should eq "GREAT"
       fresh_flake.valid?.should be_true
-      fresh_flake.taste.should eq "great-awful"
+      fresh_flake.taste.should eq "great"
+      fresh_flake.save!
 
-      less_fresh = Snowflake.new(vibe: "awful", taste: "GREAT")
+      less_fresh = Snowflake.new(vibe: "awful", taste: "great")
       less_fresh.valid?.should be_false
       less_fresh.errors[0].to_s.should eq "Snowflake taste should be unique"
     end
 
     it "passes scoped fields to transform callback" do
-      fresh_flake = Snowflake.create!(vibe: "awful", size: 123)
-      fresh_flake.vibe.should eq "awful-123"
+      fresh_flake = Snowflake.new(vibe: "AWFUL", size: 123)
+      fresh_flake.vibe.should eq "AWFUL"
       fresh_flake.valid?.should be_true
-      fresh_flake.vibe.should eq "awful-123"
+      fresh_flake.vibe.should eq "awful"
+      fresh_flake.save!
 
-      less_fresh = Snowflake.new(vibe: "AWFUL", size: 123)
+      less_fresh = Snowflake.new(vibe: "awful", size: 123)
       less_fresh.valid?.should be_false
       less_fresh.errors[0].to_s.should eq "Snowflake vibe should be unique"
     end

diff --git a/src/rethinkdb-orm/associations.cr b/src/rethinkdb-orm/associations.cr
@@ -111,7 +111,7 @@ module RethinkORM::Associations
     {% if dependent.id == :destroy || dependent.id == :delete %}
 
     def destroy_{{ method.id }}
-      return unless association = {{ method.id }}
+      return unless (association = {{ method.id }})
       if association.is_a?(RethinkORM::AssociationCollection)
         association.each { |model| model.destroy }
       else

diff --git a/src/rethinkdb-orm/validators/unique.cr b/src/rethinkdb-orm/validators/unique.cr
@@ -1,59 +1,76 @@
 module RethinkORM::Validators
   # In case of transformations on field, allow user defined transform
-  macro ensure_unique(field, scope = nil, create_index = true, callback = nil, &transform)
-
+  macro ensure_unique(field, scope = [] of Nil, create_index = true, callback = nil, &transform)
       {% if create_index %}
         secondary_index({{ field }})
       {% end %}
 
-
       validate "#{ {{ field }} } should be unique", ->(this: self) do
-        value = this.{{ field.id }}
-        return true if value.nil?
-
-        {% scope_array = [] of Nil %}
-        {% if scope %}
-          {% scope_array = scope %}
+        {% if scope.empty? %}
+          {% scope = [field] %}
+          {% proc_return_type = FIELDS[field.id][:klass] %}
+        {% else %}
+          {% proc_return_type = "Tuple(#{scope.map { |s| FIELDS[s.id][:klass] }.join(", ").id})".id %}
         {% end %}
 
-        {% if scope_array.empty? %}
-
-          argument = value
-          {% proc_type = FIELDS[field.id][:klass] %}
+        # Construct proc type fron scope array (forgive me mother, for I have sinned)
+        {% proc_arg_type = "#{scope.map { |s| FIELDS[s.id][:klass] }.join(", ").id}".id %}
+        {% signature = "#{scope.map { |s| "#{s.id}: #{FIELDS[s.id][:klass]}" }.join(", ").id}".id %}
 
-        {% else %}
-          # Check if any arguments to the transform are nil
-          {% for s in scope_array %}
-            return true if this.{{s.id}}.nil?
-          {% end %}
-
-          argument = {
-          {% for s in scope_array %}
-            this.{{s.id}}.not_nil!,
-          {% end %}
-          }
-
-          # Forgive me mother, for I have sinned
-          {% proc_type = "Tuple(#{scope_array.map { |s| FIELDS[s.id][:klass] }.join(", ").id})".id %}
+        # Return if any values are nil
+        {% for s in scope %}
+          return true if this.{{s.id}}.nil?
         {% end %}
 
+        # Handle Transformation block/callback
         {% if transform %}
-          # Construct a proc from a given block
-          value = ->( {{ transform.args[0] }}  : {{ proc_type }} ) { {{ transform.body }} }.call argument
+          # Construct a proc from a given block, call with argument
+          transform_proc : Proc({{ proc_arg_type }}, {{ proc_return_type }}) = ->({{ signature.id }}) { {{ transform.body }} }
+
+          result : {{ proc_return_type }} = transform_proc.call(
+            {% for s in scope %}this.{{s.id}}.not_nil!,{% end %}
+          )
         {% elsif callback %}
-          value = argument.is_a?(Tuple) ? this.{{ callback.id }} *argument : this.{{ callback.id }} argument
+          result : {{ proc_return_type }} = this.{{ callback.id }}(
+            {% for s in scope %}this.{{s.id}}.not_nil!,{% end %}
+          )
+        {% else %}
+          # No transform
+          result = {
+            {% for s in scope %}this.{{s.id}}.not_nil!,{% end %}
+          }
         {% end %}
 
-        {% if create_index %}
+        # Fetch Document
+        {% if scope.size == 1 %}
           # Utilise generated secondary index
-          instance = self.get_all([value], index: {{ field.id.stringify }}).to_a.shift?
+          {% if create_index %}
+            doc = self.get_all([result], index: {{ field.id.stringify }}).first?
+          {% else %}
+            doc = self.where({{field.id}}: result).first?
+          {% end %}
         {% else %}
-          # Otherwise, fallback to where query
-          instance = self.where({{field.id}}: value).to_a.shift?
+          # Where query with all scoped fields
+          doc = self.where(
+            {% for s, index in scope %}{{s.id}}: result[{{ index.id }}], {% end %}
+          ).first?
+        {% end %}
+
+        # Fields are not present in another document under present table
+        success = !(doc && doc.id != this.id)
+        {% if transform || callback %}
+          # Set fields in unique scope with result of transform block if document is unique
+          if success && !this.persisted?
+            {% if scope.size == 1 %}
+              this.{{ field.id }} = result
+            {% else %}
+              {% for s, index in scope %}
+                this.{{ s.id }} = result[{{ index.id }}]
+              {% end %}
+            {% end %}
+          end
         {% end %}
 
-        success = !(instance && instance.id != this.id)
-        this.{{ field.id }} = value if success && !this.persisted?
         success
       end
     end