From c3d364d1539f21f91ac5c161a772d42b9fd0d88a Mon Sep 17 00:00:00 2001 From: Jeremy Woertink Date: Mon, 5 Jul 2021 10:21:26 -0700 Subject: [PATCH 1/7] Attempting to implement the json serializable. First spec passes, but it's super hacky --- .../20210703234151_add_metadata_to_blobs.cr | 15 ++++++++ spec/json_column_spec.cr | 11 ++++++ spec/support/factories/blob_factory.cr | 1 + spec/support/models/blob.cr | 8 +++++ src/avram/insert.cr | 5 ++- src/avram/model.cr | 35 ++++++++++++++++--- src/avram/query_builder.cr | 10 ++++-- src/avram/save_operation.cr | 3 ++ 8 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 db/migrations/20210703234151_add_metadata_to_blobs.cr diff --git a/db/migrations/20210703234151_add_metadata_to_blobs.cr b/db/migrations/20210703234151_add_metadata_to_blobs.cr new file mode 100644 index 000000000..f70ca5800 --- /dev/null +++ b/db/migrations/20210703234151_add_metadata_to_blobs.cr @@ -0,0 +1,15 @@ +class AddMetadataToBlobs::V20210703234151 < Avram::Migrator::Migration::V1 + def migrate + alter :blobs do + # Serialized columns should end in _raw to differentiate between + # the original raw data, and the serialized object + add metadata_raw : JSON::Any, default: JSON::Any.new({} of String => JSON::Any) + end + end + + def rollback + alter :blobs do + remove :metadata_raw + end + end +end diff --git a/spec/json_column_spec.cr b/spec/json_column_spec.cr index 598a00c69..04e69e6b4 100644 --- a/spec/json_column_spec.cr +++ b/spec/json_column_spec.cr @@ -63,4 +63,15 @@ describe "JSON Columns" do }), }) end + + describe "serialized", focus: true do + it "saves the raw value" do + SaveBlob.create(metadata_raw: "{}") do |operation, blob| + operation.saved?.should be_true + blob.should_not be_nil + blob.not_nil!.metadata.should be_nil + blob.not_nil!.metadata_raw.should eq("{}") + end + end + end end diff --git a/spec/support/factories/blob_factory.cr b/spec/support/factories/blob_factory.cr index 5ba355775..6cd8cbdc7 100644 --- a/spec/support/factories/blob_factory.cr +++ b/spec/support/factories/blob_factory.cr @@ -1,5 +1,6 @@ class BlobFactory < BaseFactory def initialize doc JSON::Any.new({"foo" => JSON::Any.new("bar")}) + metadata_raw({name: "Test", code: 4}.to_json) end end diff --git a/spec/support/models/blob.cr b/spec/support/models/blob.cr index 597c7fc78..17c53db57 100644 --- a/spec/support/models/blob.cr +++ b/spec/support/models/blob.cr @@ -1,5 +1,13 @@ +class BlobMetadata + include JSON::Serializable + + property name : String + property code : Int32 +end + class Blob < BaseModel table do column doc : JSON::Any? + serialized metadata : BlobMetadata? end end diff --git a/src/avram/insert.cr b/src/avram/insert.cr index 45f5ebc2e..af10aca7f 100644 --- a/src/avram/insert.cr +++ b/src/avram/insert.cr @@ -1,7 +1,7 @@ class Avram::Insert alias Params = Hash(Symbol, String) | Hash(Symbol, String?) | Hash(Symbol, Nil) - def initialize(@table : TableName, @params : Params, @column_names : Array(Symbol) = [] of Symbol) + def initialize(@table : TableName, @params : Params, @column_names : Array(String) = [] of String) end def statement @@ -13,8 +13,7 @@ class Avram::Insert "*" else @column_names - .map { |column| "#{@table}.#{column}" } - .join(", ") + .join(", ") { |column| "#{@table}.#{column}".gsub('"', "") } end end diff --git a/src/avram/model.cr b/src/avram/model.cr index 9201d2fd3..5f1fafddd 100644 --- a/src/avram/model.cr +++ b/src/avram/model.cr @@ -214,7 +214,31 @@ abstract class Avram::Model {% end %} end - macro column(type_declaration, autogenerated = false) + macro serialized(type_declaration) + column {{ type_declaration.var }}_raw : String, serialized: true + + def {{ type_declaration }} + {% if type_declaration.type.is_a?(Union) %} + {% data_type = "#{type_declaration.type.types.first}".id %} + {% nilable = true %} + {% else %} + {% data_type = "#{type_declaration.type}".id %} + {% nilable = false %} + {% end %} + + {% if nilable %} + if {{ type_declaration.var }}_raw.blank? || {{ type_declaration.var }}_raw == "{}" + nil + else + {{ data_type }}.from_json({{ type_declaration.var }}_raw) + end + {% else %} + {{ data_type }}.from_json({{ type_declaration.var }}_raw) + {% end %} + end + end + + macro column(type_declaration, autogenerated = false, serialized = false) {% if type_declaration.type.is_a?(Union) %} {% data_type = "#{type_declaration.type.types.first}".id %} {% nilable = true %} @@ -227,19 +251,20 @@ abstract class Avram::Model {% else %} {% value = nil %} {% end %} - {% COLUMNS << {name: type_declaration.var, type: data_type, nilable: nilable.id, autogenerated: autogenerated, value: value} %} + {% COLUMNS << {name: type_declaration.var, type: data_type, nilable: nilable.id, autogenerated: autogenerated, value: value, serialized: serialized} %} end macro setup_column_info_methods(columns, *args, **named_args) - def self.column_names : Array(Symbol) + def self.column_names : Array(String) columns.map { |column| column[:name] } end - def self.columns : Array({name: Symbol, nilable: Bool, type: String}) + def self.columns : Array({name: String, nilable: Bool, type: String}) [ {% for column in columns %} + {% column_name = column[:serialized] ? "#{column[:name]}::text" : column[:name] %} { - name: {{ column[:name].id.symbolize }}, + name: {{ column_name.stringify }}, nilable: {{ column[:nilable] }}, type: {{ column[:type].id }}.name }, diff --git a/src/avram/query_builder.cr b/src/avram/query_builder.cr index 805e779c9..123ddd05f 100644 --- a/src/avram/query_builder.cr +++ b/src/avram/query_builder.cr @@ -241,8 +241,14 @@ class Avram::QueryBuilder def select(selection : Array(ColumnName)) @selections = selection - .map { |column| "#{@table}.#{column}" } - .join(", ") + .join(", ") do |column| + String.build do |io| + io << "#{@table}.#{column}" + # HACK: cast jsonb columns to String so they can be re-parsed + # https://github.com/will/crystal-pg/issues/125 + io << "::text" if column.to_s.ends_with?("_raw") + end + end self end diff --git a/src/avram/save_operation.cr b/src/avram/save_operation.cr index fada8dd6b..5decc3937 100644 --- a/src/avram/save_operation.cr +++ b/src/avram/save_operation.cr @@ -363,6 +363,9 @@ abstract class Avram::SaveOperation(T) private def insert : T self.created_at.value ||= Time.utc if responds_to?(:created_at) self.updated_at.value ||= Time.utc if responds_to?(:updated_at) + puts "*" * 50 + pp insert_sql.statement + puts "*" * 50 @record = database.query insert_sql.statement, args: insert_sql.args do |rs| @record = T.from_rs(rs).first end From 1daa955e3cc826777c04e8694e353e4c330ac509 Mon Sep 17 00:00:00 2001 From: Jeremy Woertink Date: Mon, 5 Jul 2021 13:06:46 -0700 Subject: [PATCH 2/7] round two reverts some of the previous commit. Now using a column converter. Still needs lots more specs --- .../20210703234151_add_metadata_to_blobs.cr | 8 +-- spec/json_column_spec.cr | 15 ++++-- spec/queryable_spec.cr | 8 +-- spec/support/factories/blob_factory.cr | 3 +- spec/support/models/blob.cr | 13 +++-- src/avram.cr | 7 +-- src/avram/charms/json_decoder_extension.cr | 20 ++++++++ src/avram/charms/json_extensions.cr | 28 ++++++++++ src/avram/insert.cr | 4 +- src/avram/model.cr | 51 +++++++------------ src/avram/query_builder.cr | 10 +--- src/avram/save_operation.cr | 3 -- 12 files changed, 106 insertions(+), 64 deletions(-) create mode 100644 src/avram/charms/json_decoder_extension.cr diff --git a/db/migrations/20210703234151_add_metadata_to_blobs.cr b/db/migrations/20210703234151_add_metadata_to_blobs.cr index f70ca5800..a1df14e8d 100644 --- a/db/migrations/20210703234151_add_metadata_to_blobs.cr +++ b/db/migrations/20210703234151_add_metadata_to_blobs.cr @@ -1,15 +1,15 @@ class AddMetadataToBlobs::V20210703234151 < Avram::Migrator::Migration::V1 def migrate alter :blobs do - # Serialized columns should end in _raw to differentiate between - # the original raw data, and the serialized object - add metadata_raw : JSON::Any, default: JSON::Any.new({} of String => JSON::Any) + add metadata : JSON::Any, default: JSON::Any.new({} of String => JSON::Any) + add media : JSON::Any?, fill_existing_with: :nothing end end def rollback alter :blobs do - remove :metadata_raw + remove :metadata + remove :media end end end diff --git a/spec/json_column_spec.cr b/spec/json_column_spec.cr index 04e69e6b4..45025ffb6 100644 --- a/spec/json_column_spec.cr +++ b/spec/json_column_spec.cr @@ -27,13 +27,17 @@ describe "JSON Columns" do it "should convert scalars and save forms" do form1 = SaveBlob.new form1.set_doc_from_param(42) + form1.set_metadata_from_param(BlobMetadata.from_json("{}")) form1.doc.value.should eq JSON::Any.new(42_i64) + form1.metadata.value.should be_a(BlobMetadata) + form1.save! blob1 = BlobQuery.new.last blob1.doc.should eq JSON::Any.new(42_i64) form2 = SaveBlob.new form2.set_doc_from_param("hey") + form2.set_metadata_from_param(BlobMetadata.from_json("{}")) form2.doc.value.should eq JSON::Any.new("hey") form2.save! blob2 = BlobQuery.new.last @@ -43,6 +47,7 @@ describe "JSON Columns" do it "should convert hashes and arrays and save forms" do form1 = SaveBlob.new form1.set_doc_from_param(%w[a b c]) + form1.set_metadata_from_param(BlobMetadata.from_json("{}")) form1.doc.value.should eq %w[a b c].map { |v| JSON::Any.new(v) } form1.save! blob1 = BlobQuery.new.last @@ -50,6 +55,7 @@ describe "JSON Columns" do form2 = SaveBlob.new form2.set_doc_from_param({"foo" => {"bar" => "baz"}}) + form2.set_metadata_from_param(BlobMetadata.from_json("{}")) form2.doc.value.should eq JSON::Any.new({ "foo" => JSON::Any.new({ "bar" => JSON::Any.new("baz"), @@ -64,13 +70,14 @@ describe "JSON Columns" do }) end - describe "serialized", focus: true do + describe "serialized" do it "saves the raw value" do - SaveBlob.create(metadata_raw: "{}") do |operation, blob| + SaveBlob.create(metadata: BlobMetadata.from_json("{}")) do |operation, blob| operation.saved?.should be_true blob.should_not be_nil - blob.not_nil!.metadata.should be_nil - blob.not_nil!.metadata_raw.should eq("{}") + blob.not_nil!.metadata.should be_a(BlobMetadata) + blob.not_nil!.metadata.name.should be_nil + blob.not_nil!.media.should be_nil end end end diff --git a/spec/queryable_spec.cr b/spec/queryable_spec.cr index 8381bbb92..18637c13f 100644 --- a/spec/queryable_spec.cr +++ b/spec/queryable_spec.cr @@ -988,17 +988,17 @@ describe Avram::Queryable do blob = BlobFactory.new.doc(JSON::Any.new({"foo" => JSON::Any.new("bar")})).create query = JSONQuery.new.static_foo - query.to_sql.should eq ["SELECT blobs.id, blobs.created_at, blobs.updated_at, blobs.doc FROM blobs WHERE blobs.doc = $1", "{\"foo\":\"bar\"}"] + query.to_sql.should eq ["SELECT blobs.id, blobs.created_at, blobs.updated_at, blobs.doc, blobs.metadata, blobs.media FROM blobs WHERE blobs.doc = $1", "{\"foo\":\"bar\"}"] result = query.first result.should eq blob query2 = JSONQuery.new.foo_with_value("bar") - query2.to_sql.should eq ["SELECT blobs.id, blobs.created_at, blobs.updated_at, blobs.doc FROM blobs WHERE blobs.doc = $1", "{\"foo\":\"bar\"}"] + query2.to_sql.should eq ["SELECT blobs.id, blobs.created_at, blobs.updated_at, blobs.doc, blobs.metadata, blobs.media FROM blobs WHERE blobs.doc = $1", "{\"foo\":\"bar\"}"] result = query2.first result.should eq blob query3 = JSONQuery.new.foo_with_value("baz") - query3.to_sql.should eq ["SELECT blobs.id, blobs.created_at, blobs.updated_at, blobs.doc FROM blobs WHERE blobs.doc = $1", "{\"foo\":\"baz\"}"] + query3.to_sql.should eq ["SELECT blobs.id, blobs.created_at, blobs.updated_at, blobs.doc, blobs.metadata, blobs.media FROM blobs WHERE blobs.doc = $1", "{\"foo\":\"baz\"}"] expect_raises(Avram::RecordNotFoundError) do query3.first end @@ -1328,7 +1328,7 @@ describe Avram::Queryable do query.to_prepared_sql.should eq(%{SELECT #{Bucket::COLUMN_SQL} FROM buckets WHERE buckets.names = '{"Larry","Moe","Curly"}' AND buckets.numbers = '{1,2,3}'}) query = Blob::BaseQuery.new.doc(JSON::Any.new({"properties" => JSON::Any.new("sold")})) - query.to_prepared_sql.should eq(%{SELECT blobs.id, blobs.created_at, blobs.updated_at, blobs.doc FROM blobs WHERE blobs.doc = '{"properties":"sold"}'}) + query.to_prepared_sql.should eq(%{SELECT blobs.id, blobs.created_at, blobs.updated_at, blobs.doc, blobs.metadata, blobs.media FROM blobs WHERE blobs.doc = '{"properties":"sold"}'}) query = UserQuery.new.name.in(["Don", "Juan"]).age.gt(30) query.to_prepared_sql.should eq(%{SELECT #{User::COLUMN_SQL} FROM users WHERE users.name = ANY ('{"Don","Juan"}') AND users.age > '30'}) diff --git a/spec/support/factories/blob_factory.cr b/spec/support/factories/blob_factory.cr index 6cd8cbdc7..a86ed8249 100644 --- a/spec/support/factories/blob_factory.cr +++ b/spec/support/factories/blob_factory.cr @@ -1,6 +1,7 @@ class BlobFactory < BaseFactory def initialize doc JSON::Any.new({"foo" => JSON::Any.new("bar")}) - metadata_raw({name: "Test", code: 4}.to_json) + metadata(BlobMetadata.from_json({name: "Test", code: 4}.to_json)) + media nil end end diff --git a/spec/support/models/blob.cr b/spec/support/models/blob.cr index 17c53db57..9fa9599c9 100644 --- a/spec/support/models/blob.cr +++ b/spec/support/models/blob.cr @@ -1,13 +1,20 @@ class BlobMetadata include JSON::Serializable - property name : String - property code : Int32 + property name : String? + property code : Int32? +end + +class MediaMetadata + include JSON::Serializable + + property image : String? end class Blob < BaseModel table do column doc : JSON::Any? - serialized metadata : BlobMetadata? + column metadata : BlobMetadata, serialize: true + column media : MediaMetadata?, serialize: true end end diff --git a/src/avram.cr b/src/avram.cr index 6ccc9e46c..3e91cbb0e 100644 --- a/src/avram.cr +++ b/src/avram.cr @@ -2,6 +2,10 @@ require "dexter" require "wordsmith" require "habitat" require "pulsar" +require "db" +require "pg" +require "uuid" + require "./avram/object_extensions" require "./avram/criteria" require "./avram/type" @@ -11,9 +15,6 @@ require "./avram/charms/**" require "./avram/migrator/**" require "./avram/tasks/**" require "./avram/**" -require "db" -require "pg" -require "uuid" module Avram Habitat.create do diff --git a/src/avram/charms/json_decoder_extension.cr b/src/avram/charms/json_decoder_extension.cr new file mode 100644 index 000000000..10d5deee9 --- /dev/null +++ b/src/avram/charms/json_decoder_extension.cr @@ -0,0 +1,20 @@ +module PG::Decoders + struct JsonDecoder + def decode(io, bytesize, oid) + if oid == JSONB_OID + io.read_byte + bytesize -= 1 + end + + string = String.new(bytesize) do |buffer| + io.read_fully(Slice.new(buffer, bytesize)) + {bytesize, 0} + end + JSON::PullParser.new(string) + end + + def type + JSON::PullParser + end + end +end diff --git a/src/avram/charms/json_extensions.cr b/src/avram/charms/json_extensions.cr index 372f5b885..5485d2107 100644 --- a/src/avram/charms/json_extensions.cr +++ b/src/avram/charms/json_extensions.cr @@ -1,3 +1,31 @@ +module JSON::Serializable + macro included + def self.adapter + Lucky(self) + end + end + + module Lucky(T) + include Avram::Type + + def from_db!(value) + value + end + + def parse(value : JSON::Serializable) + SuccessfulCast(JSON::Serializable).new value + end + + def parse(value) + SuccessfulCast(JSON::Serializable).new T.from_json(value) + end + + def to_db(value) + value.to_json + end + end +end + struct JSON::Any def self.adapter Lucky diff --git a/src/avram/insert.cr b/src/avram/insert.cr index af10aca7f..253664b70 100644 --- a/src/avram/insert.cr +++ b/src/avram/insert.cr @@ -1,7 +1,7 @@ class Avram::Insert alias Params = Hash(Symbol, String) | Hash(Symbol, String?) | Hash(Symbol, Nil) - def initialize(@table : TableName, @params : Params, @column_names : Array(String) = [] of String) + def initialize(@table : TableName, @params : Params, @column_names : Array(Symbol) = [] of Symbol) end def statement @@ -13,7 +13,7 @@ class Avram::Insert "*" else @column_names - .join(", ") { |column| "#{@table}.#{column}".gsub('"', "") } + .join(", ") { |column| "#{@table}.#{column}" } end end diff --git a/src/avram/model.cr b/src/avram/model.cr index 5f1fafddd..9730677e1 100644 --- a/src/avram/model.cr +++ b/src/avram/model.cr @@ -8,6 +8,13 @@ abstract class Avram::Model include Avram::Polymorphic include Avram::SchemaEnforcer + module JSONConverter(T) + def self.from_rs(rs : DB::ResultSet) + value = rs.read(JSON::PullParser?) + T.new(value) if value + end + end + macro inherited COLUMNS = [] of Nil # types are not checked in macros ASSOCIATIONS = [] of Nil # types are not checked in macros @@ -166,12 +173,15 @@ abstract class Avram::Model {{column[:name]}}: { {% if column[:type].id == Array(Float64).id %} type: Array(PG::Numeric), - {% elsif column[:type].is_a?(Generic) %} + {% elsif column[:type].is_a?(Generic) || column[:serialized] %} type: {{column[:type]}}, {% else %} type: {{column[:type]}}::Lucky::ColumnType, {% end %} nilable: {{column[:nilable]}}, + {% if column[:serialized] %} + converter: JSONConverter({{column[:type]}}), + {% end %} }, {% end %} }) @@ -214,36 +224,12 @@ abstract class Avram::Model {% end %} end - macro serialized(type_declaration) - column {{ type_declaration.var }}_raw : String, serialized: true - - def {{ type_declaration }} - {% if type_declaration.type.is_a?(Union) %} - {% data_type = "#{type_declaration.type.types.first}".id %} - {% nilable = true %} - {% else %} - {% data_type = "#{type_declaration.type}".id %} - {% nilable = false %} - {% end %} - - {% if nilable %} - if {{ type_declaration.var }}_raw.blank? || {{ type_declaration.var }}_raw == "{}" - nil - else - {{ data_type }}.from_json({{ type_declaration.var }}_raw) - end - {% else %} - {{ data_type }}.from_json({{ type_declaration.var }}_raw) - {% end %} - end - end - - macro column(type_declaration, autogenerated = false, serialized = false) + macro column(type_declaration, autogenerated = false, serialize is_serialized = false) {% if type_declaration.type.is_a?(Union) %} - {% data_type = "#{type_declaration.type.types.first}".id %} + {% data_type = type_declaration.type.types.first.id %} {% nilable = true %} {% else %} - {% data_type = "#{type_declaration.type}".id %} + {% data_type = type_declaration.type.id %} {% nilable = false %} {% end %} {% if type_declaration.value || type_declaration.value == false %} @@ -251,20 +237,21 @@ abstract class Avram::Model {% else %} {% value = nil %} {% end %} + {% serialized = is_serialized || data_type == "JSON::Any".id %} + {% COLUMNS << {name: type_declaration.var, type: data_type, nilable: nilable.id, autogenerated: autogenerated, value: value, serialized: serialized} %} end macro setup_column_info_methods(columns, *args, **named_args) - def self.column_names : Array(String) + def self.column_names : Array(Symbol) columns.map { |column| column[:name] } end - def self.columns : Array({name: String, nilable: Bool, type: String}) + def self.columns : Array({name: Symbol, nilable: Bool, type: String}) [ {% for column in columns %} - {% column_name = column[:serialized] ? "#{column[:name]}::text" : column[:name] %} { - name: {{ column_name.stringify }}, + name: {{ column[:name].id.symbolize }}, nilable: {{ column[:nilable] }}, type: {{ column[:type].id }}.name }, diff --git a/src/avram/query_builder.cr b/src/avram/query_builder.cr index 123ddd05f..5873bb2c1 100644 --- a/src/avram/query_builder.cr +++ b/src/avram/query_builder.cr @@ -241,14 +241,8 @@ class Avram::QueryBuilder def select(selection : Array(ColumnName)) @selections = selection - .join(", ") do |column| - String.build do |io| - io << "#{@table}.#{column}" - # HACK: cast jsonb columns to String so they can be re-parsed - # https://github.com/will/crystal-pg/issues/125 - io << "::text" if column.to_s.ends_with?("_raw") - end - end + .join(", ") { |column| "#{@table}.#{column}" } + self end diff --git a/src/avram/save_operation.cr b/src/avram/save_operation.cr index 5decc3937..fada8dd6b 100644 --- a/src/avram/save_operation.cr +++ b/src/avram/save_operation.cr @@ -363,9 +363,6 @@ abstract class Avram::SaveOperation(T) private def insert : T self.created_at.value ||= Time.utc if responds_to?(:created_at) self.updated_at.value ||= Time.utc if responds_to?(:updated_at) - puts "*" * 50 - pp insert_sql.statement - puts "*" * 50 @record = database.query insert_sql.statement, args: insert_sql.args do |rs| @record = T.from_rs(rs).first end From dcbe24b56f8b573c00d8c731ef59c8362b074da8 Mon Sep 17 00:00:00 2001 From: Jeremy Woertink Date: Mon, 5 Jul 2021 14:34:53 -0700 Subject: [PATCH 3/7] reverting these changes because they're refactors and not really related to this PR. There's already enough noise. --- src/avram/insert.cr | 3 ++- src/avram/model.cr | 4 ++-- src/avram/query_builder.cr | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/avram/insert.cr b/src/avram/insert.cr index 253664b70..45f5ebc2e 100644 --- a/src/avram/insert.cr +++ b/src/avram/insert.cr @@ -13,7 +13,8 @@ class Avram::Insert "*" else @column_names - .join(", ") { |column| "#{@table}.#{column}" } + .map { |column| "#{@table}.#{column}" } + .join(", ") end end diff --git a/src/avram/model.cr b/src/avram/model.cr index 9730677e1..f75e08a1d 100644 --- a/src/avram/model.cr +++ b/src/avram/model.cr @@ -226,10 +226,10 @@ abstract class Avram::Model macro column(type_declaration, autogenerated = false, serialize is_serialized = false) {% if type_declaration.type.is_a?(Union) %} - {% data_type = type_declaration.type.types.first.id %} + {% data_type = "#{type_declaration.type.types.first}".id %} {% nilable = true %} {% else %} - {% data_type = type_declaration.type.id %} + {% data_type = "#{type_declaration.type}".id %} {% nilable = false %} {% end %} {% if type_declaration.value || type_declaration.value == false %} diff --git a/src/avram/query_builder.cr b/src/avram/query_builder.cr index 5873bb2c1..884bc18c4 100644 --- a/src/avram/query_builder.cr +++ b/src/avram/query_builder.cr @@ -241,7 +241,8 @@ class Avram::QueryBuilder def select(selection : Array(ColumnName)) @selections = selection - .join(", ") { |column| "#{@table}.#{column}" } + .map { |column| "#{@table}.#{column}" } + .join(", ") self end From 29aec80415f7773cfd9fda12b3af8a14e658487e Mon Sep 17 00:00:00 2001 From: Jeremy Woertink Date: Mon, 5 Jul 2021 16:07:54 -0700 Subject: [PATCH 4/7] use separate branch with json changes and we no longer need any patches --- shard.yml | 4 ++-- src/avram/charms/json_decoder_extension.cr | 20 -------------------- src/avram/model.cr | 3 +-- src/avram/query_builder.cr | 1 - 4 files changed, 3 insertions(+), 25 deletions(-) delete mode 100644 src/avram/charms/json_decoder_extension.cr diff --git a/shard.yml b/shard.yml index ce26be1f5..71512a667 100644 --- a/shard.yml +++ b/shard.yml @@ -17,8 +17,8 @@ dependencies: github: luckyframework/lucky_task version: ~> 0.1.0 pg: - github: will/crystal-pg - version: ~> 0.23.0 + github: matthewmcgarvey/crystal-pg + branch: read-json-columns habitat: github: luckyframework/habitat version: ~> 0.4.7 diff --git a/src/avram/charms/json_decoder_extension.cr b/src/avram/charms/json_decoder_extension.cr deleted file mode 100644 index 10d5deee9..000000000 --- a/src/avram/charms/json_decoder_extension.cr +++ /dev/null @@ -1,20 +0,0 @@ -module PG::Decoders - struct JsonDecoder - def decode(io, bytesize, oid) - if oid == JSONB_OID - io.read_byte - bytesize -= 1 - end - - string = String.new(bytesize) do |buffer| - io.read_fully(Slice.new(buffer, bytesize)) - {bytesize, 0} - end - JSON::PullParser.new(string) - end - - def type - JSON::PullParser - end - end -end diff --git a/src/avram/model.cr b/src/avram/model.cr index f75e08a1d..928096d04 100644 --- a/src/avram/model.cr +++ b/src/avram/model.cr @@ -237,9 +237,8 @@ abstract class Avram::Model {% else %} {% value = nil %} {% end %} - {% serialized = is_serialized || data_type == "JSON::Any".id %} - {% COLUMNS << {name: type_declaration.var, type: data_type, nilable: nilable.id, autogenerated: autogenerated, value: value, serialized: serialized} %} + {% COLUMNS << {name: type_declaration.var, type: data_type, nilable: nilable.id, autogenerated: autogenerated, value: value, serialized: is_serialized} %} end macro setup_column_info_methods(columns, *args, **named_args) diff --git a/src/avram/query_builder.cr b/src/avram/query_builder.cr index 884bc18c4..805e779c9 100644 --- a/src/avram/query_builder.cr +++ b/src/avram/query_builder.cr @@ -243,7 +243,6 @@ class Avram::QueryBuilder @selections = selection .map { |column| "#{@table}.#{column}" } .join(", ") - self end From 75fd177b975b59fa41c851496b2875f823a30b7a Mon Sep 17 00:00:00 2001 From: Jeremy Woertink Date: Tue, 6 Jul 2021 18:49:47 -0700 Subject: [PATCH 5/7] Adding the ability to query serializable columns --- spec/json_column_spec.cr | 11 ++++++++++- src/avram/charms/json_extensions.cr | 8 ++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/spec/json_column_spec.cr b/spec/json_column_spec.cr index 45025ffb6..b08c7f6dc 100644 --- a/spec/json_column_spec.cr +++ b/spec/json_column_spec.cr @@ -71,7 +71,7 @@ describe "JSON Columns" do end describe "serialized" do - it "saves the raw value" do + it "saves the serialized value" do SaveBlob.create(metadata: BlobMetadata.from_json("{}")) do |operation, blob| operation.saved?.should be_true blob.should_not be_nil @@ -80,5 +80,14 @@ describe "JSON Columns" do blob.not_nil!.media.should be_nil end end + + it "queries serialized columns" do + one = BlobMetadata.from_json({name: "One", code: 4}.to_json) + two = BlobMetadata.from_json({name: "Two", code: 9}.to_json) + BlobFactory.create &.metadata(one) + BlobFactory.create &.metadata(two) + + BlobQuery.new.metadata(two).select_count.should eq(1) + end end end diff --git a/src/avram/charms/json_extensions.cr b/src/avram/charms/json_extensions.cr index 5485d2107..844e80ce2 100644 --- a/src/avram/charms/json_extensions.cr +++ b/src/avram/charms/json_extensions.cr @@ -6,8 +6,13 @@ module JSON::Serializable end module Lucky(T) + alias ColumnType = JSON::Any include Avram::Type + def self.criteria(query : R, column) forall R + Criteria(R, T).new(query, column) + end + def from_db!(value) value end @@ -23,6 +28,9 @@ module JSON::Serializable def to_db(value) value.to_json end + + class Criteria(T, V) < Avram::Criteria(T, V) + end end end From 761616f3e3e0370e14ea612549f52cd22915c9c7 Mon Sep 17 00:00:00 2001 From: Jeremy Woertink Date: Mon, 12 Jul 2021 08:43:54 -0700 Subject: [PATCH 6/7] This line wasn't needed --- src/avram/charms/json_extensions.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/src/avram/charms/json_extensions.cr b/src/avram/charms/json_extensions.cr index 844e80ce2..d5fb8797a 100644 --- a/src/avram/charms/json_extensions.cr +++ b/src/avram/charms/json_extensions.cr @@ -6,7 +6,6 @@ module JSON::Serializable end module Lucky(T) - alias ColumnType = JSON::Any include Avram::Type def self.criteria(query : R, column) forall R From be1db3cd9308a4844cabda29c5a84ddfe87d1507 Mon Sep 17 00:00:00 2001 From: Jeremy Woertink Date: Mon, 12 Jul 2021 08:47:24 -0700 Subject: [PATCH 7/7] Go back to original PG Shard, but use master now that Matthew's PR was merged --- shard.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shard.yml b/shard.yml index 04c7f9692..7f46aee81 100644 --- a/shard.yml +++ b/shard.yml @@ -17,8 +17,8 @@ dependencies: github: luckyframework/lucky_task version: ~> 0.1.0 pg: - github: matthewmcgarvey/crystal-pg - branch: read-json-columns + github: will/crystal-pg + branch: master habitat: github: luckyframework/habitat version: ~> 0.4.7