Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use last field of a compound PK when setting auto ids #689

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,6 @@ echo 'user root;' >> spec_openresty/s2/nginx.conf

./busted -o utfTerminal
./busted -o utfTerminal spec_postgres/
./busted -o utfTerminal spec_mysql/
./busted -o utfTerminal spec_openresty/
./busted -o utfTerminal spec_cqueues/
8 changes: 6 additions & 2 deletions lapis/db/mysql/model.lua
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,12 @@ do
local res = db.insert(self:table_name(), values, self:primary_keys())
if res then
local new_id = res.last_auto_id or res.insert_id
if not values[self.primary_key] and new_id and new_id ~= 0 then
values[self.primary_key] = new_id
local pk = self.primary_key
if type(pk) == "table" then
pk = pk[#pk]
end
if not values[pk] and new_id and new_id ~= 0 then
values[pk] = new_id
end
return self:load(values)
else
Expand Down
8 changes: 6 additions & 2 deletions lapis/db/mysql/model.moon
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@ class Model extends BaseModel
-- either luasql (field res.last_auto_id) or
-- lua-resty-mysql (field res.insert_id) and
new_id = res.last_auto_id or res.insert_id
if not values[@primary_key] and new_id and new_id != 0
values[@primary_key] = new_id
pk = @primary_key
-- In a compound primary key, the auto_increment field must be last.
if type(pk) == "table"
pk = pk[#pk]
if not values[pk] and new_id and new_id != 0
values[pk] = new_id
@load values
else
nil, "Failed to create #{@__name}"
Expand Down
6 changes: 4 additions & 2 deletions spec/core_model_specs.moon
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ assert_same_rows = (a, b) ->

(models) ->
import it, describe, before_each, after_each from require "busted"
import Users, Posts, Likes from models
import Users, Posts, Images, Likes from models

describe "basic model", ->
before_each ->
Expand Down Expand Up @@ -182,10 +182,11 @@ assert_same_rows = (a, b) ->
before_each ->
Users\create_table!
Posts\create_table!
Images\create_table!
Likes\create_table!

package.loaded.models = {
:Users, :Posts, :Likes
:Users, :Posts, :Images, :Likes
}

query_log = {}
Expand Down Expand Up @@ -272,6 +273,7 @@ assert_same_rows = (a, b) ->
before_each ->
Users\create_table!
Posts\create_table!
Images\create_table!
Likes\create_table!

before_each ->
Expand Down
49 changes: 47 additions & 2 deletions spec_mysql/model_spec.moon
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
db = require "lapis.db.mysql"

import setup_db, teardown_db from require "spec_mysql.helpers"
import Users, Posts, Likes from require "spec_mysql.models"
import Users, Posts, Images, Likes from require "spec_mysql.models"

describe "model", ->
setup ->
Expand All @@ -12,7 +12,7 @@ describe "model", ->

describe "core model", ->
build = require "spec.core_model_specs"
build { :Users, :Posts, :Likes }
build { :Users, :Posts, :Images, :Likes }

it "should get columns of model", ->
Users\create_table!
Expand All @@ -39,3 +39,48 @@ describe "model", ->
-- this fails in postgres, but mysql gives default values
Posts\create {}

describe "with compound auto_increment", ->
Users\create_table!
Posts\create_table!
Images\create_table!

user1 = Users\create { name: "bob" }
post1 = Posts\create { user_id: user1.user_id }
post2 = Posts\create { user_id: user1.user_id }

first = Images\create {
user_id: user1.id
post_id: post1.id
url: "first"
}
second = Images\create {
user_id: user1.id
post_id: post1.id
url: "second"
}
third = Images\create {
user_id: user1.id
post_id: post2.id
url: "third"
}

it "should increment keys", ->
assert.truthy first.id < second.id
assert.truthy second.id < third.id

it "should find entites", ->
assert.same first, Images\find post1.id, first.id
assert.same {first, second}, Images\find_all {post1.id}, "post_id"
assert.same {third}, Images\find_all {post2.id}, "post_id"

it "should allow relations", ->
package.loaded.models = {
:Users, :Posts, :Images, :Likes
}

assert.same user1, first\get_user!
assert.same post1, first\get_post!
assert.same post2, third\get_post!

assert.same {first.id, second.id}, [v.id for v in *post1\get_images!]
assert.same {third.id}, [v.id for v in *post2\get_images!]
40 changes: 36 additions & 4 deletions spec_mysql/models.moon
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,52 @@ class Users extends Model
class Posts extends Model
@timestamp: true

@relations: {
{"images", has_many: "Images"}
}

@create_table: =>
drop_tables @
create_table @table_name!, {
{"id", types.id}
{"user_id", types.integer null: true}
{"title", types.text null: false}
{"body", types.text null: false}
Comment on lines -24 to -25
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mysql tests were broken without this. It probably slipped since the mysql tests weren't included in ci.sh.

{"title", types.text null: true}
{"body", types.text null: true}
{"created_at", types.datetime}
{"updated_at", types.datetime}
}

@truncate: =>
truncate_tables @

class Images extends Model
@primary_key: {"user_id", "id"}
@timestamp: true

@relations: {
{"user", belongs_to: "Users"}
{"post", belongs_to: "Posts"}
}

@create_table: =>
drop_tables @
create_table @table_name!, {
{"post_id", types.integer}
-- Can't use types.id for "id" because it specifies primary_key
{"id", types.integer auto_increment: true}
{"user_id", types.integer null: true}
{"url", types.text null: false}
{"created_at", types.datetime}
{"updated_at", types.datetime}

"PRIMARY KEY (post_id, id)"
-- auto_increment must be a key of its own (PK or otherwise)
"KEY id (id)"
}

@truncate: =>
truncate_tables @

class Likes extends Model
@primary_key: {"user_id", "post_id"}
@timestamp: true
Expand All @@ -44,7 +76,7 @@ class Likes extends Model
create_table @table_name!, {
{"user_id", types.integer}
{"post_id", types.integer}
{"count", types.integer}
{"count", types.integer default: 0}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also broke mysql tests. Fixed.

{"created_at", types.datetime}
{"updated_at", types.datetime}

Expand All @@ -54,4 +86,4 @@ class Likes extends Model
@truncate: =>
truncate_tables @

{:Users, :Posts, :Likes}
{:Users, :Posts, :Images, :Likes}
3 changes: 2 additions & 1 deletion spec_openresty/resty_mysql_spec.moon
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ runner = NginxRunner base_path: "spec_openresty/s2/"
import SpecServer from require "lapis.spec.server"
server = SpecServer runner

import Users, Posts, Likes from require "spec_mysql.models"
import Users, Posts, Images, Likes from require "spec_mysql.models"

import setup_db, teardown_db from require "spec_mysql.helpers"

Expand All @@ -15,6 +15,7 @@ describe "resty", ->

Users\create_table!
Posts\create_table!
Images\create_table!
Likes\create_table!

server\load_test_server!
Expand Down
3 changes: 2 additions & 1 deletion spec_openresty/s2/app.moon
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
lapis = require "lapis"
db = require "lapis.db"

import Users, Posts, Likes from require "spec_mysql.models"
import Users, Posts, Images, Likes from require "spec_mysql.models"

assert = require "luassert"

Expand All @@ -21,6 +21,7 @@ class extends lapis.Application
@before_filter ->
Users\truncate!
Posts\truncate!
Images\truncate!
Likes\truncate!

"/": =>
Expand Down
28 changes: 27 additions & 1 deletion spec_postgres/model_spec.moon
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ class Users extends Model
class Posts extends Model
@timestamp: true

@relations: {
{"images", has_many: "Images"}
}

@create_table: =>
drop_tables @
create_table @table_name!, {
Expand All @@ -37,6 +41,28 @@ class Posts extends Model
@truncate: =>
truncate_tables @

class Images extends Model
@timestamp: true

@create_table: =>
drop_tables @
create_table @table_name!, {
{"post_id", types.integer}
-- TODO Can't use types.id for "id" because it specifies primary_key.
-- We could pre-process the fields and collect primary keys first, then
-- create a "PRIMARY KEY" string internally.
{"id", types.integer auto_increment: true}
{"user_id", types.integer null: true}
{"url", types.text null: false}
{"created_at", types.time}
{"updated_at", types.time}

"PRIMARY KEY (post_id, id)"
}

@truncate: =>
truncate_tables @

class Likes extends Model
@primary_key: {"user_id", "post_id"}
@timestamp: true
Expand Down Expand Up @@ -79,7 +105,7 @@ describe "model", ->

describe "core model", ->
build = require "spec.core_model_specs"
build { :Users, :Posts, :Likes }
build { :Users, :Posts, :Images, :Likes }

it "should get columns of model", ->
Users\create_table!
Expand Down