diff --git a/src/alumna-sqlite.cr b/src/alumna-sqlite.cr index c98d9cb..bbd6098 100644 --- a/src/alumna-sqlite.cr +++ b/src/alumna-sqlite.cr @@ -22,7 +22,9 @@ module Alumna @fetch_col_fds : Array(FieldDescriptor?) # Pre-compiled SQL strings and keys for uniqueness checks - @unique_fields : Array(FieldDescriptor) + @unique_fields : Array({String, FieldDescriptor}) + @indexed_fields : Array({String, FieldDescriptor}) + @unique_sqls_no_skip : Array(String) @unique_sqls_with_skip : Array(String) @@ -66,12 +68,14 @@ module Alumna end @insert_sql = "INSERT INTO #{@table_name} (#{@columns_to_update.join(", ")}) VALUES (#{placeholders})" - @unique_fields = @sch.fields.select(&.unique) + @unique_fields = @sch.unique_fields + @indexed_fields = @sch.indexed_fields + @unique_sqls_no_skip = Array(String).new(initial_capacity: @unique_fields.size) @unique_sqls_with_skip = Array(String).new(initial_capacity: @unique_fields.size) - @unique_fields.each do |fd| - db_col = extract_db_col(fd.name) + @unique_fields.each do |(path, fd)| + db_col = extract_db_col(path) @unique_sqls_no_skip << "SELECT 1 FROM #{@table_name} WHERE #{db_col} = ? LIMIT 1" @unique_sqls_with_skip << "SELECT 1 FROM #{@table_name} WHERE #{db_col} = ? AND id != ? LIMIT 1" end @@ -109,11 +113,10 @@ module Alumna # ------------------------------------------------------------------------- def create_indexes! : Nil - @sch.fields.each do |fd| - next unless fd.unique || fd.indexed - idx_name = "idx_#{@table_name}_#{fd.name.gsub(SAFE_NAME_RE, '_')}" + @indexed_fields.each do |(path, fd)| + idx_name = "idx_#{@table_name}_#{path.gsub(SAFE_NAME_RE, '_')}" unique_str = fd.unique ? "UNIQUE " : "" - db_col = extract_db_col(fd.name) + db_col = extract_db_col(path) @db.exec("CREATE #{unique_str}INDEX IF NOT EXISTS #{idx_name} ON #{@table_name}(#{db_col})") end @@ -174,7 +177,8 @@ module Alumna end when .time? case val - when Time then val + when Time then val + # Prevent unhandled 500s from corrupt or legacy dates in the database when String then Time::Format::RFC_3339.parse(val) rescue nil else nil end @@ -289,9 +293,9 @@ module Alumna # ------------------------------------------------------------------------- private def check_unique(record : Hash(String, AnyData), skip_id : String? = nil) : ServiceError? - # 1. Single-field fast path - @unique_fields.each_with_index do |fd, i| - val = extract_value(record, fd.name) + # 1. Single-field fast path (now natively includes nested hashes) + @unique_fields.each_with_index do |(path, fd), i| + val = extract_value(record, path) next if val.nil? duplicate = if skip_id @@ -301,7 +305,7 @@ module Alumna end if duplicate - return ServiceError.unprocessable("Unique constraint violation", {fd.name => "already exists"} of String => AnyData) + return ServiceError.unprocessable("Unique constraint violation", {path => "already exists"} of String => AnyData) end end @@ -316,7 +320,7 @@ module Alumna idx.fields.each do |f| v = extract_value(record, f) if v.nil? - # LCOV_EXCL_START - kcov wrongly reports on this line, while reporting coverage for the `break` right after + # LCOV_EXCL_START - kcov wrongly reports this assignment as uncovered while reporting `break` as covered missing_val = true # LCOV_EXCL_STOP break @@ -381,15 +385,15 @@ module Alumna filters = q.typed_filters(@sch) return filters if filters.is_a?(ServiceError) - args = Array(DB::Any).new(initial_capacity: filters.size * 4) + # Increased heuristic to accommodate typical $in conditions safely + args = Array(DB::Any).new(initial_capacity: filters.size * 8) - # Declared before String.build so the q.select branch can update them - # via closure capture; map_row reads the final values after the block. col_names = @fetch_col_names col_fds = @fetch_col_fds id_idx = 0 - query = String.build(capacity: 256) do |sql| + # Increased capacity to 512 to prevent buffer doubling on medium-to-large queries + query = String.build(capacity: 512) do |sql| sql << "SELECT " if fields = q.select @@ -406,11 +410,14 @@ module Alumna built_names << "id" built_fds << nil sql << "id" - elsif fd = @sch.find_field(c) + elsif @columns_to_update.includes?(c) + fd = @sch.find_field(c) sql << ", " if built_names.size > 0 built_names << c built_fds << fd sql << c + else + return ServiceError.bad_request("Invalid select field: #{c}") end end @@ -435,7 +442,9 @@ module Alumna filters.each do |field, conditions| next if conditions.empty? fd = conditions.first.fd - next unless fd + + # Strictly prevent silently dropping filters for unknown fields + return ServiceError.bad_request("Unknown query field: #{field}") unless fd dot_idx = field.index('.') is_array = fd.type.array? @@ -486,18 +495,18 @@ module Alumna first_sort = true sort.each do |(f, dir)| dot_idx = f.index('.') - valid_col = dot_idx ? !@sch.find_field(f).nil? : (f == "id" || !@sch.find_field(f).nil?) - - if valid_col - sql << (first_sort ? " ORDER BY " : ", ") - if dot_idx - write_db_col(sql, f, dot_idx) - else - sql << f - end - sql << (dir == 1 ? " ASC" : " DESC") - first_sort = false + valid_col = dot_idx ? !@sch.find_field(f).nil? : (f == "id" || @columns_to_update.includes?(f)) + + return ServiceError.bad_request("Unknown sort field: #{f}") unless valid_col + + sql << (first_sort ? " ORDER BY " : ", ") + if dot_idx + write_db_col(sql, f, dot_idx) + else + sql << f end + sql << (dir == 1 ? " ASC" : " DESC") + first_sort = false end end @@ -558,7 +567,9 @@ module Alumna columns = Array(String).new(initial_capacity: ctx.data.size) ctx.data.each do |k, v| next if k == "id" - return ServiceError.bad_request("Unknown column: #{k}") unless @sch.find_field(k) + # Validate solely against top-level columns without triggering dot-notation scans + return ServiceError.bad_request("Unknown column: #{k}") unless @columns_to_update.includes?(k) + existing[k] = v columns << k end @@ -571,6 +582,8 @@ module Alumna args = Array(DB::Any).new(initial_capacity: columns.size + 1) + # If the column count matches, we can safely reuse the pre-compiled full UPDATE statement + # because we already verified all columns exist in @columns_to_update if columns.size == @columns_to_update.size query = @update_sql @columns_to_update.each { |c| args << cast_to_db(existing[c]) }