From 4d85efd90ff53e8893df0c83eccbd1360e1caae9 Mon Sep 17 00:00:00 2001 From: Marek Mytkowski <101755133+mytkom@users.noreply.github.com> Date: Wed, 28 Jun 2023 13:35:53 +0200 Subject: [PATCH 1/6] ODA: coverage badge workflow (#19) * ODA: new test job and try to fetch cov percentage * ODA: fix syntax error * ODA: add Gemfile.lock... * ODA: check where i am * ODA: print file * ODA: check if parse properly * ODA: other syntax try * ODA: try to generate badge and upload as artifact * ODA: syntax error fix * ODA: change directory to root * ODA: some echoes * ODA: add id to step * ODA: maybe * ODA: fix colors * ODA: little clean up * ODA: temporarly run on pull_request * ODA: delete unnecessary linter * ODA: try * ODA: permissions setting * ODA: make dir * ODA: finally :) * ODA: naming improvement * ODA: it seems to look better --- .github/workflows/coverage.yml | 96 ++++++++++++++++++++++++++++++ .github/workflows/pull_request.yml | 2 +- Gemfile.lock | 8 +++ README.md | 1 + orderable.gemspec | 1 + spec/spec_helper.rb | 5 ++ 6 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/coverage.yml diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml new file mode 100644 index 0000000..35b57b4 --- /dev/null +++ b/.github/workflows/coverage.yml @@ -0,0 +1,96 @@ +name: Code test coverage badge +on: + push: + branches: + - master + +permissions: + contents: read + pages: write + id-token: write + +jobs: + coverage: + name: Test suite for coverage + outputs: + coverage: ${{ steps.coverage.outputs.coverage }} + percentage_coverage: ${{ steps.coverage.outputs.percentage_coverage }} + runs-on: ubuntu-latest + services: + postgres: + image: postgres:14.7 + env: + POSTGRES_PASSWORD: postgres + ports: + - "5432:5432" + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + env: + RAILS_ENV: test + POSTGRES_USERNAME: postgres + POSTGRES_PASSWORD: postgres + SCHEMA: structure.sql + steps: + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Setup Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Run RSpec + run: bundle exec rspec -fd + + - name: Save code coverage + id: coverage + run: | + COVERAGE="$( cat ./coverage/.last_run.json | grep "line" | grep -oE "[0-9]+\.[0-9]+" )" + echo "##[set-output name=coverage;]${COVERAGE}" + echo "##[set-output name=percentage_coverage;]${COVERAGE}%" + + badge: + name: Generate badge image and deply to GitHub Pages + needs: coverage + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Create directory for badge + run: | + mkdir ./badge + + - name: Generate the badge SVG image + uses: emibcn/badge-action@v2.0.2 + id: badge + with: + label: 'Coverage' + status: ${{ needs.coverage.outputs.percentage_coverage }} + color: ${{ + needs.coverage.outputs.coverage > 90 && 'green' || + needs.coverage.outputs.coverage > 80 && 'yellow,green' || + needs.coverage.outputs.coverage > 70 && 'yellow' || + needs.coverage.outputs.coverage > 60 && 'orange,yellow' || + needs.coverage.outputs.coverage > 50 && 'orange' || + needs.coverage.outputs.coverage > 40 && 'red,orange' || + needs.coverage.outputs.coverage > 30 && 'red,red,orange' || + needs.coverage.outputs.coverage > 20 && 'red,red,red,orange' || + 'red' }} + path: ./badge/test-coverage.svg + + - name: Setup Pages + uses: actions/configure-pages@v1 + + - name: Upload artifact + uses: actions/upload-pages-artifact@v1 + with: + path: ./badge + + - name: Deploy to GitHub Pages + id: deployment + uses: actions/deploy-pages@main + + diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index b533d84..f522fae 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -20,7 +20,7 @@ jobs: run: bundle exec rubocop test: - name: Test suite and coverage + name: Test suite runs-on: ubuntu-latest strategy: fail-fast: false diff --git a/Gemfile.lock b/Gemfile.lock index 8d6eb8a..1a7dd15 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -50,6 +50,7 @@ GEM activerecord database_cleaner (~> 1.99.0) diff-lcs (1.5.0) + docile (1.4.0) erubi (1.12.0) factory_bot (4.11.1) activesupport (>= 3.0.0) @@ -126,6 +127,12 @@ GEM ruby-progressbar (1.13.0) shoulda-matchers (4.1.2) activesupport (>= 4.2.0) + simplecov (0.22.0) + docile (~> 1.1) + simplecov-html (~> 0.11) + simplecov_json_formatter (~> 0.1) + simplecov-html (0.12.3) + simplecov_json_formatter (0.1.4) standalone_migrations (7.1.1) activerecord (>= 4.2.7, < 7.1.0, != 5.2.3.rc1, != 5.2.3) railties (>= 4.2.7, < 7.1.0, != 5.2.3.rc1, != 5.2.3) @@ -150,6 +157,7 @@ DEPENDENCIES rspec (~> 3.0) rubocop (~> 0.80) shoulda-matchers (~> 4.1.0) + simplecov (~> 0.22.0) standalone_migrations (~> 7.1.1) BUNDLED WITH diff --git a/README.md b/README.md index 96c8db1..34eb82e 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,5 @@ # Orderable +![Test Coverage](https://ventcode.github.io/orderable/test-coverage.svg) A gem that makes it easy to change the default order of PostgreSQL database rows by the addition of a modifiable integer column. diff --git a/orderable.gemspec b/orderable.gemspec index 9ec24e7..99bb412 100644 --- a/orderable.gemspec +++ b/orderable.gemspec @@ -40,6 +40,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency "rspec", "~> 3.0" spec.add_development_dependency "rubocop", "~> 0.80" spec.add_development_dependency "shoulda-matchers", "~> 4.1.0" + spec.add_development_dependency "simplecov", "~> 0.22.0" spec.add_development_dependency "standalone_migrations", "~> 7.1.1" spec.metadata["rubygems_mfa_required"] = "true" end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c6f2e04..f0b5112 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "simplecov" require "byebug" require "fixtures" require "orderable" @@ -10,6 +11,10 @@ require_relative "./factories" Dir["./support/*.rb"].sort.each { |file| require file } +SimpleCov.start do + formatter SimpleCov::Formatter::SimpleFormatter +end + RSpec.configure do |config| # Enable flags like --only-failures and --next-failure config.example_status_persistence_file_path = ".rspec_status" From 124774fd77686d265bd82e654400f381fc38f2b4 Mon Sep 17 00:00:00 2001 From: Marek Mytkowski <101755133+mytkom@users.noreply.github.com> Date: Wed, 28 Jun 2023 14:05:25 +0200 Subject: [PATCH 2/6] ODA: add 2 new badges and fix coverage one (#22) --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 34eb82e..838deb0 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,8 @@ # Orderable -![Test Coverage](https://ventcode.github.io/orderable/test-coverage.svg) +[![Test Coverage](https://ventcode.github.io/orderable/test-coverage.svg)](#) +[![Ruby Style Guide](https://img.shields.io/badge/code_style-rubocop-brightgreen.svg)](https://github.com/rubocop/rubocop) +[![Test and Coverage Workflow](https://github.com/ventcode/orderable/actions/workflows/coverage.yml/badge.svg)](.github/workflows/coverage.yml) + A gem that makes it easy to change the default order of PostgreSQL database rows by the addition of a modifiable integer column. From bff266c712f896b39d72b788aeebdfccb9d87361 Mon Sep 17 00:00:00 2001 From: Marek Mytkowski <101755133+mytkom@users.noreply.github.com> Date: Wed, 28 Jun 2023 15:58:14 +0200 Subject: [PATCH 3/6] ODA: stop using deprecated github workflow output (#23) * ODA: stop using deprecated github workflow output * ODA: just checking if works * ODA: ehh snake case * ODA: clean up --- .github/workflows/coverage.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 35b57b4..0111cd2 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -49,8 +49,8 @@ jobs: id: coverage run: | COVERAGE="$( cat ./coverage/.last_run.json | grep "line" | grep -oE "[0-9]+\.[0-9]+" )" - echo "##[set-output name=coverage;]${COVERAGE}" - echo "##[set-output name=percentage_coverage;]${COVERAGE}%" + echo "{coverage}={${COVERAGE}}" >> $GITHUB_OUTPUT + echo "{percentage_coverage}={${COVERAGE}%}" >> $GITHUB_OUTPUT badge: name: Generate badge image and deply to GitHub Pages @@ -65,7 +65,6 @@ jobs: - name: Generate the badge SVG image uses: emibcn/badge-action@v2.0.2 - id: badge with: label: 'Coverage' status: ${{ needs.coverage.outputs.percentage_coverage }} From ad3ba6a295c639efed86c381a253ba059ce17789 Mon Sep 17 00:00:00 2001 From: Marek Mytkowski <101755133+mytkom@users.noreply.github.com> Date: Wed, 28 Jun 2023 17:38:06 +0200 Subject: [PATCH 4/6] ODA: fix badge (#24) * ODA: fix badge * ODA: switch to master --- .github/workflows/coverage.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 0111cd2..85a977f 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -49,8 +49,8 @@ jobs: id: coverage run: | COVERAGE="$( cat ./coverage/.last_run.json | grep "line" | grep -oE "[0-9]+\.[0-9]+" )" - echo "{coverage}={${COVERAGE}}" >> $GITHUB_OUTPUT - echo "{percentage_coverage}={${COVERAGE}%}" >> $GITHUB_OUTPUT + echo "coverage=${COVERAGE}" >> $GITHUB_OUTPUT + echo "percentage_coverage=${COVERAGE}%" >> $GITHUB_OUTPUT badge: name: Generate badge image and deply to GitHub Pages From 444f35577dc1155e2503b32ee5b3e73a3a1ccea0 Mon Sep 17 00:00:00 2001 From: Marek Mytkowski <101755133+mytkom@users.noreply.github.com> Date: Wed, 28 Jun 2023 17:43:23 +0200 Subject: [PATCH 5/6] ODA: intuitive default ordered direction + README (#21) * ODA: intuitive default ordered direction + README * ODA: CR * ODA: CR --- README.md | 104 +++++++++++++-------- lib/orderable/config.rb | 4 - lib/orderable/model_extension.rb | 2 +- spec/callbacks/on_create_spec.rb | 20 ++-- spec/callbacks/on_destroy_spec.rb | 24 ++--- spec/callbacks/on_update_spec.rb | 132 +++++++++++++-------------- spec/features/from_spec.rb | 6 +- spec/features/order_spec.rb | 4 +- spec/features/reset_function_spec.rb | 98 ++++++++++++++------ 9 files changed, 231 insertions(+), 163 deletions(-) diff --git a/README.md b/README.md index 838deb0..07819c3 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ A gem that makes it easy to change the default order of PostgreSQL database rows * [Decremental sequence](#decremental-sequence) * [License](#license) ### Basic usage -Let's consider the AR **image** model that implements the `orderable` method. Its position field name is set as `position` and it has only 2 properties - `id` and `name`. **Images** table content is presented below. +Let's consider the AR **image** model that implements the `orderable` method. Its positioning field name is set as `position` and it has only 2 properties - `id` and `label`. **Images** table content is presented below. | id | name | position | |----|-----|----------| @@ -39,21 +39,33 @@ class Image < ApplicationRecord orderable :position end -Image.pluck(:name, :position) # => [["A", 1], ["B", 2], ["C", 0]] -Image.ordered.pluck(:name) # => ["B", "A", "C"] +Image.pluck(:label, :position) # => [["A", 1], ["B", 2], ["C", 0]] +Image.ordered.pluck(:label) # => ["C", "A", "B"] # on create -image = Image.create(name: "D") +image = Image.create(label: "D") image.position # => 3 -Image.ordered.pluck(:name) # => ["D", B", "A", "C"] +Image.ordered.pluck(:label) # => ["C", "A", "B", "D"] # on update image.update(position: 2) -Image.ordered.pluck(:name) # => ["B", "D", "A", "C"] +Image.ordered.pluck(:label) # => ["C", "A", "D", "B"] # on destroy image.destroy() -Image.ordered.pluck(:name) # => ["B", "A", "C"] +Image.ordered.pluck(:label, :position) # => [["C", 0], ["A", 1], ["B", 2]] +``` + +Notice that you can pass direction `:asc`/`:desc` to `ordered` scope as parameter: +```ruby +Image.pluck(:label, :position) # => [["A", 1], ["B", 2], ["C", 0]] + +# :asc by default +Image.ordered.pluck(:label) # => ["C", "A", "B"] +Image.ordered(:asc).pluck(:label) # => ["C", "A", "B"] + +# :desc +Image.ordered(:desc).pluck(:label) # => ["B", "A", "C"] ``` ### Installation @@ -107,39 +119,49 @@ orderable :orderable_field_name | Attribute | Value | Default | Description | | - | - | - | - | | `scope` | array of symbols | `[]` | scope same as in unique index (uniqueness of this fields combination would be ensured) | -| `validate` | boolean | `true` | if `true`, it validates numericality of positioning field, as well as being in range `<0, M>`, where `M` stands for the biggest positioning field value | -| `auto_set` | boolean | `true` | if `true`, it sets a new record in front of other records unless position field is passed directly | -|`from`| integer | 0 | base value from which positions are set | +| `auto_set` | boolean | `true` | if `true` and positioning field value is not specified, it inserts a new record on the bottom for decremental sequence or on the top for incremental sequence on create | | `sequence` | `:incremental` or `:decremental` | `:incremental` | value used to determine positioning sequence | +| `validate` | boolean | `true` | if `true`, it validates numericality of positioning field value, as well as being in range `<0, M>`, where `M` stands for the biggest positioning field value | +|`from`| integer | 0 | base value from which sequence starts | ### Usage Examples #### Model with a scope +Let's say a user has few cover and profile photos. Using *orderable* with scope will allow user to customize their order separately. ```ruby -class Image < ActiveRecord::Base - orderable :position, scope: :group +class Photo < ActiveRecord::Base + orderable :position, scope: :type + + scope :profile, -> { where(type: 'profile') } + scope :cover, -> { where(type: 'cover') } end -Image.pluck(:name, :position, :group) # => [["A", 0, "G_1"], ["E", 1, "G_2"], ["C", 2, "G_1"], ["B", 1, "G_1"], ["D", 0, "G_2"]] -Image.ordered.pluck(:name) # => [["C", 2, "G_1"], ["B", 1, "G_1"], ["A", 0, "G_1"], ["E", 1, "G_2"], ["D", 0, "G_2"]] +Photo.pluck(:label, :position, :type) # => [["A", 0, "profile"], ["E", 1, "cover"], ["C", 2, "profile"], ["B", 1, "profile"], ["D", 0, "cover"]] +Photo.ordered.pluck(:label) # => ["A", "B", "C", "D", "E"] +Photo.profile.ordered.pluck(:label, :position) # => [["A", 0], ["B", 1], ["C", 2]] +Photo.cover.ordered.pluck(:label, :position) # => [["D", 0], ["E", 1]] # on create -image = Image.create(name: "F", group: "G_1") -image.position # => 3 -Image.ordered.pluck(:name, :position, :group) # => [["F", 3, "G_1"], ["C", 2, "G_1"], ["B", 1, "G_1"], ["A", 0, "G_1"], ["E", 1, "G_2"], ["D", 0, "G_2"]] +photo = Photo.create(label: "F", type: "profile") +photo.position # => 3 +Photo.profile.ordered.pluck(:label, :position) # => [["A", 0], ["B", 1], ["C", 2], ["F", 3]] +Photo.cover.ordered.pluck(:label, :position) # => [["D", 0], ["E", 1]] # on update -image.update(group: "G_2") -image.position # => 2 -Image.ordered.pluck(:name, :position, :group) # => [["C", 2, "G_1"], ["B", 1, "G_1"], ["A", 0, "G_1"], ["F", 2, "G_2"], ["E", 1, "G_2"], ["D", 0, "G_2"]] +photo.update(type: "cover") +photo.position # => 2 +Photo.profile.ordered.pluck(:label, :position) # => [["A", 0], ["B", 1], ["C", 2]] +Photo.cover.ordered.pluck(:label, :position) # => [["D", 0], ["E", 1], ["F", 2]] -image.update(position: 1) -Image.ordered.pluck(:name, :position, :group) # => [["C", 2, "G_1"], ["B", 1, "G_1"], ["A", 0, "G_1"], ["E", 2, "G_2"], ["F", 1, "G_2"], ["D", 0, "G_2"]] +photo.update(position: 1) +Photo.profile.ordered.pluck(:label, :position) # => [["A", 0], ["B", 1], ["C", 2]] +Photo.cover.ordered.pluck(:label, :position) # => [["D", 0], ["F", 1], ["E", 2]] # on destroy -image.destroy() -Image.ordered.pluck(:name, :position, :group) # => [["C", 2, "G_1"], ["B", 1, "G_1"], ["A", 0, "G_1"], ["E", 1, "G_2"], ["D", 0, "G_2"]] +photo.destroy() +Photo.profile.ordered.pluck(:label, :position) # => [["A", 0], ["B", 1], ["C", 2]] +Photo.cover.ordered.pluck(:label, :position) # => [["D", 0], ["E", 1]] ``` #### Default push front @@ -148,17 +170,20 @@ class Image < ActiveRecord::Base orderable :position, auto_set: true # by default end +image = Image.create(label: "A") # => OK +image.position # => 0 +image = Image.create(label: "B") # => OK +Image.ordered.pluck(:label, :position) # => [["A", 0], ["B", 1]] + + class Post < ActiveRecord::Base orderable :position, auto_set: false end -image= Image.create(name: "A") # => OK -image.position # => 0 - Post.create(title: "A") # => validation error (position is not specified) Post.create(title: "A", position: 0) # => OK Post.create(title: "B", position: 0) # => OK -Post.ordered.pluck(:title, :position) # => [["A", 1], ["B", 0]] +Post.ordered.pluck(:title, :position) # => [["B", 0], ["A", 1]] ``` #### Disabling validation @@ -167,15 +192,16 @@ class Image < ActiveRecord::Base orderable :position, validation: true # by default end +Image.count # => 0 +Image.create(label: "A", position: -1) # => validation error (cannot be negative) +Image.create(label: "A", position: 1) # => validation error (no image with position 0) + + class Post < ActiveRecord::Base orderable :position, validation: false end -Image.count # => 0 Post.count # => 0 - -Image.create(name: "A", position: -1) # => validation error (cannot be negative) -Image.create(name: "A", position: 1) # => validation error (no image with position 0) Post.create(title: "A title", position: -1) # => OK ``` #### Setting from value @@ -185,9 +211,9 @@ class Image < ActiveRecord::Base orderable :position, from: 10 end -Image.create(name: "A") -Image.create(name: "B") -Image.ordered.pluck(:name, :position) # => [["B", 11], ["A", 10]] +Image.create(label: "A") +Image.create(label: "B") +Image.ordered.pluck(:label, :position) # => [["A", 10], ["B", 11]] ``` ### Decremental sequence @@ -197,10 +223,10 @@ class Image < ActiveRecord::Base orderable :position, from: 10, sequence: :decremental end -Image.create(name: "A") -Image.create(name: "B") -Image.create(name: "C") -Image.ordered.pluck(:name, :position) # => [["C", 8], ["B", 9], ["A", 10]] +Image.create(label: "A") +Image.create(label: "B") +Image.create(label: "C") +Image.ordered.pluck(:label, :position) # => [["C", 8], ["B", 9], ["A", 10]] ``` ## License diff --git a/lib/orderable/config.rb b/lib/orderable/config.rb index 26ed13e..186591a 100644 --- a/lib/orderable/config.rb +++ b/lib/orderable/config.rb @@ -22,10 +22,6 @@ def initialize(**options) super(DEFAULTS.merge(normalized_options)) end - def order_direction - SEQUENCES.fetch(sequence) - end - private def normalize_options!(options) diff --git a/lib/orderable/model_extension.rb b/lib/orderable/model_extension.rb index 8beaf56..c9fbf20 100644 --- a/lib/orderable/model_extension.rb +++ b/lib/orderable/model_extension.rb @@ -18,7 +18,7 @@ def orderable(field, **options) validate { executor.validate_record_position(self) } end - scope :ordered, ->(direction = config.order_direction) { order(field => direction) } + scope :ordered, ->(direction = :asc) { order(field => direction) } define_singleton_method(:reorder) { executor.reset } end end diff --git a/spec/callbacks/on_create_spec.rb b/spec/callbacks/on_create_spec.rb index c6db719..ceead89 100644 --- a/spec/callbacks/on_create_spec.rb +++ b/spec/callbacks/on_create_spec.rb @@ -72,17 +72,17 @@ .to change { ModelWithManyScopes.ordered.pluck(:name, :position, :group) } .from( [ - ["c", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["c", 2, "first"] ] ) .to( [ - ["d", 3, "first"], - ["c", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["c", 2, "first"], + ["d", 3, "first"] ] ) end @@ -96,17 +96,17 @@ .to change { ModelWithManyScopes.ordered.pluck(:name, :position, :group) } .from( [ - ["c", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["c", 2, "first"] ] ) .to( [ - ["c", 2, "first"], - ["b", 1, "first"], ["a", 0, "first"], - ["d", 0, "second"] + ["d", 0, "second"], + ["b", 1, "first"], + ["c", 2, "first"] ] ) end diff --git a/spec/callbacks/on_destroy_spec.rb b/spec/callbacks/on_destroy_spec.rb index 3d3ccce..3409b2a 100644 --- a/spec/callbacks/on_destroy_spec.rb +++ b/spec/callbacks/on_destroy_spec.rb @@ -152,17 +152,17 @@ .to change { ModelWithManyScopes.ordered.pluck(:name, :position, :group) } .from( [ - ["c", 2, "first"], - ["b", 1, "first"], ["a", 0, "first"], - ["d", 0, "second"] + ["d", 0, "second"], + ["b", 1, "first"], + ["c", 2, "first"] ] ) .to( [ - ["c", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["c", 2, "first"] ] ) end @@ -180,19 +180,19 @@ .to change { ModelWithManyScopes.ordered.pluck(:name, :position, :group) } .from( [ - ["c", 3, "first"], - ["e", 2, "first"], - ["b", 1, "first"], ["a", 0, "first"], - ["d", 0, "second"] + ["d", 0, "second"], + ["b", 1, "first"], + ["e", 2, "first"], + ["c", 3, "first"] ] ) .to( [ - ["c", 2, "first"], - ["b", 1, "first"], ["a", 0, "first"], - ["d", 0, "second"] + ["d", 0, "second"], + ["b", 1, "first"], + ["c", 2, "first"] ] ) end diff --git a/spec/callbacks/on_update_spec.rb b/spec/callbacks/on_update_spec.rb index 60e6cdf..c72f999 100644 --- a/spec/callbacks/on_update_spec.rb +++ b/spec/callbacks/on_update_spec.rb @@ -92,18 +92,18 @@ .to change { ModelWithManyScopes.ordered.pluck(:name, :position, :group) } .from( [ - ["c", 2, "first"], - ["b", 1, "first"], ["a", 0, "first"], - ["d", 0, "second"] + ["d", 0, "second"], + ["b", 1, "first"], + ["c", 2, "first"] ] ) .to( [ - ["c", 3, "first"], - ["b", 2, "first"], + ["a", 0, "first"], ["d", 1, "first"], # updated record - ["a", 0, "first"] + ["b", 2, "first"], + ["c", 3, "first"] ] ) end @@ -116,18 +116,18 @@ .to change { ModelWithManyScopes.ordered.pluck(:name, :position, :group) } .from( [ - ["c", 2, "first"], - ["b", 1, "first"], ["a", 0, "first"], - ["d", 0, "second"] + ["d", 0, "second"], + ["b", 1, "first"], + ["c", 2, "first"] ] ) .to( [ - ["c", 3, "first"], - ["d", 2, "first"], # updated record + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["d", 2, "first"], # updated record + ["c", 3, "first"] ] ) end @@ -143,18 +143,18 @@ .to change { ModelWithManyScopes.ordered.pluck(:name, :position, :group) } .from( [ - ["c", 2, "first"], - ["b", 1, "first"], ["a", 0, "first"], - ["d", 0, "second"] + ["d", 0, "second"], + ["b", 1, "first"], + ["c", 2, "first"] ] ) .to( [ - ["d", 3, "first"], # updated record - ["c", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["c", 2, "first"], + ["d", 3, "first"] # updated record ] ) end @@ -169,18 +169,18 @@ .to change { ModelWithManyScopes.ordered.pluck(:name, :position, :group) } .from( [ - ["d", 3, "first"], - ["c", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["c", 2, "first"], + ["d", 3, "first"] ] ) .to( [ - ["new name", 3, "first"], - ["c", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["c", 2, "first"], + ["new name", 3, "first"] ] ) end @@ -195,18 +195,18 @@ .to change { ModelWithManyScopes.ordered.pluck(:name, :position, :group) } .from( [ - ["c", 3, "first"], - ["d", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["d", 2, "first"], + ["c", 3, "first"] ] ) .to( [ - ["c", 2, "first"], - ["b", 1, "first"], ["a", 0, "first"], - ["d", 0, "second"] # updated record + ["d", 0, "second"], # updated record + ["b", 1, "first"], + ["c", 2, "first"] ] ) end @@ -408,23 +408,23 @@ .to change { NoValidationModelWithOneScope.ordered.pluck(:name, :position, :kind) } .from( [ - ["f", 9, "first"], - ["e", 8, "first"], - ["d", 7, "first"], - ["c", 2, "first"], - ["b", 1, "first"], + ["g", 0, "second"], ["a", 0, "first"], - ["g", 0, "second"] + ["b", 1, "first"], + ["c", 2, "first"], + ["d", 7, "first"], + ["e", 8, "first"], + ["f", 9, "first"] ] ).to( [ - ["g", 10, "first"], - ["f", 9, "first"], - ["e", 8, "first"], - ["d", 7, "first"], - ["c", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["c", 2, "first"], + ["d", 7, "first"], + ["e", 8, "first"], + ["f", 9, "first"], + ["g", 10, "first"] ] ) end @@ -438,23 +438,23 @@ .to change { NoValidationModelWithOneScope.ordered.pluck(:name, :position, :kind) } .from( [ - ["f", 9, "first"], - ["e", 8, "first"], - ["d", 7, "first"], - ["c", 2, "first"], - ["b", 1, "first"], + ["g", 0, "second"], ["a", 0, "first"], - ["g", 0, "second"] + ["b", 1, "first"], + ["c", 2, "first"], + ["d", 7, "first"], + ["e", 8, "first"], + ["f", 9, "first"] ] ).to( [ - ["f", 10, "first"], - ["e", 9, "first"], - ["d", 8, "first"], - ["c", 3, "first"], - ["b", 2, "first"], + ["a", 0, "first"], ["g", 1, "first"], - ["a", 0, "first"] + ["b", 2, "first"], + ["c", 3, "first"], + ["d", 8, "first"], + ["e", 9, "first"], + ["f", 10, "first"] ] ) end @@ -472,23 +472,23 @@ .to change { NoValidationModelWithOneScope.ordered.pluck(:name, :position, :kind) } .from( [ - ["g", 10, "first"], - ["f", 9, "first"], - ["e", 8, "first"], - ["d", 7, "first"], - ["c", 2, "first"], + ["a", 0, "first"], ["b", 1, "first"], - ["a", 0, "first"] + ["c", 2, "first"], + ["d", 7, "first"], + ["e", 8, "first"], + ["f", 9, "first"], + ["g", 10, "first"] ] ).to( [ - ["f", 10, "first"], - ["e", 9, "first"], - ["d", 8, "first"], - ["c", 3, "first"], - ["b", 2, "first"], + ["a", 0, "first"], ["g", 1, "first"], - ["a", 0, "first"] + ["b", 2, "first"], + ["c", 3, "first"], + ["d", 8, "first"], + ["e", 9, "first"], + ["f", 10, "first"] ] ) end diff --git a/spec/features/from_spec.rb b/spec/features/from_spec.rb index c2d6426..a462445 100644 --- a/spec/features/from_spec.rb +++ b/spec/features/from_spec.rb @@ -11,7 +11,7 @@ it "creates a new record with the highest position" do expect(subject.position).to eq(FromModel.maximum(:position)) - expect(FromModel.ordered.pluck(:position)).to eq([102, 101, 100]) + expect(FromModel.ordered.pluck(:position)).to eq([100, 101, 102]) end context "when position attribute given" do @@ -19,7 +19,7 @@ it "creates a new record with a correct position" do expect(subject.position).to eq(101) - expect(FromModel.ordered.pluck(:position)).to eq([102, 101, 100]) + expect(FromModel.ordered.pluck(:position)).to eq([100, 101, 102]) end end end @@ -57,7 +57,7 @@ it "removes the record and shifts others correctly" do expect(subject.destroy).to be_present expect { subject.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect(FromModel.ordered.pluck(:position)).to eq([101, 100]) + expect(FromModel.ordered.pluck(:position)).to eq([100, 101]) end end end diff --git a/spec/features/order_spec.rb b/spec/features/order_spec.rb index cef88a1..a20a02e 100644 --- a/spec/features/order_spec.rb +++ b/spec/features/order_spec.rb @@ -5,13 +5,13 @@ context "when ascending order set" do it "returns records proper order" do - expect(BasicModel.ordered(:asc).pluck(:position)).to eq([0, 1, 2]) + expect(BasicModel.ordered.pluck(:position)).to eq([0, 1, 2]) end end context "when order not specified in scope" do it "returns records in descending order" do - expect(BasicModel.ordered.pluck(:position)).to eq([2, 1, 0]) + expect(BasicModel.ordered(:desc).pluck(:position)).to eq([2, 1, 0]) end end end diff --git a/spec/features/reset_function_spec.rb b/spec/features/reset_function_spec.rb index 3e0c1f8..4efff81 100644 --- a/spec/features/reset_function_spec.rb +++ b/spec/features/reset_function_spec.rb @@ -21,47 +21,93 @@ before { ModelWithManyScopes.reorder } it "reset the positions and keeps the sequential order" do - expect(alpha_positions).to eq((0..2).to_a.reverse) - expect(beta_positions).to eq((0..2).to_a.reverse) + expect(alpha_positions).to eq((0..2).to_a) + expect(beta_positions).to eq((0..2).to_a) end end context "without validation" do context "with incremental sequence" do - before do - create_list(:no_validation_model_with_many_scopes, 3, :random_position) - create_list(:no_validation_model_with_many_scopes, 3, :random_position, kind: "beta") + context "with random positions" do + before do + create_list(:no_validation_model_with_many_scopes, 3, :random_position) + create_list(:no_validation_model_with_many_scopes, 3, :random_position, kind: "beta") + end + + let(:alpha_positions) { NoValidationModelWithManyScopes.ordered.where(kind: "alpha").pluck(:position) } + let(:beta_positions) { NoValidationModelWithManyScopes.ordered.where(kind: "beta").pluck(:position) } + + before { NoValidationModelWithManyScopes.reorder } + + it "reset the positions and keeps the sequential order" do + expect(alpha_positions).to eq((0..2).to_a) + expect(beta_positions).to eq((0..2).to_a) + end end - let(:alpha_positions) { NoValidationModelWithManyScopes.ordered.where(kind: "alpha").pluck(:position) } - let(:beta_positions) { NoValidationModelWithManyScopes.ordered.where(kind: "beta").pluck(:position) } + context "with preset positions" do + before do + [1, 4, 8].each { |position| create(:no_validation_model_with_many_scopes, position: position) } + [2, 5, 9].each { |position| create(:no_validation_model_with_many_scopes, position: position, kind: "beta") } + end + + let(:alpha_positions) { NoValidationModelWithManyScopes.ordered.where(kind: "alpha").pluck(:position) } + let(:beta_positions) { NoValidationModelWithManyScopes.ordered.where(kind: "beta").pluck(:position) } - before { NoValidationModelWithManyScopes.reorder } + before { NoValidationModelWithManyScopes.reorder } - it "reset the positions and keeps the sequential order" do - expect(alpha_positions).to eq((0..2).to_a.reverse) - expect(beta_positions).to eq((0..2).to_a.reverse) + it "reset the positions and keeps the sequential order" do + expect(alpha_positions).to eq((0..2).to_a) + expect(beta_positions).to eq((0..2).to_a) + end end end context "with decremental sequence" do - before do - create_list(:decremental_sequence_no_validation_model_with_many_scopes, 3, :random_position) - create_list(:decremental_sequence_no_validation_model_with_many_scopes, 3, :random_position, kind: "beta") + context "with random positions" do + before do + create_list(:decremental_sequence_no_validation_model_with_many_scopes, 3, :random_position) + create_list(:decremental_sequence_no_validation_model_with_many_scopes, 3, :random_position, kind: "beta") + end + + let(:alpha_positions) do + DecrementalSequenceNoValidationModelWithManyScopes.ordered.where(kind: "alpha").pluck(:position) + end + let(:beta_positions) do + DecrementalSequenceNoValidationModelWithManyScopes.ordered.where(kind: "beta").pluck(:position) + end + + before { DecrementalSequenceNoValidationModelWithManyScopes.reorder } + + it "reset the positions and keeps the sequential order" do + expect(alpha_positions).to eq((8..10).to_a) + expect(beta_positions).to eq((8..10).to_a) + end end - let(:alpha_positions) do - DecrementalSequenceNoValidationModelWithManyScopes.ordered.where(kind: "alpha").pluck(:position) - end - let(:beta_positions) do - DecrementalSequenceNoValidationModelWithManyScopes.ordered.where(kind: "beta").pluck(:position) - end - - before { DecrementalSequenceNoValidationModelWithManyScopes.reorder } - - it "reset the positions and keeps the sequential order" do - expect(alpha_positions).to eq((8..10).to_a) - expect(beta_positions).to eq((8..10).to_a) + context "with preset positions" do + before do + [8, 4, -2].each do |position| + create(:decremental_sequence_no_validation_model_with_many_scopes, position: position) + end + [7, 2, -6].each do |position| + create(:decremental_sequence_no_validation_model_with_many_scopes, position: position, kind: "beta") + end + end + + let(:alpha_positions) do + DecrementalSequenceNoValidationModelWithManyScopes.ordered.where(kind: "alpha").pluck(:position) + end + let(:beta_positions) do + DecrementalSequenceNoValidationModelWithManyScopes.ordered.where(kind: "beta").pluck(:position) + end + + before { DecrementalSequenceNoValidationModelWithManyScopes.reorder } + + it "reset the positions and keeps the sequential order" do + expect(alpha_positions).to eq((8..10).to_a) + expect(beta_positions).to eq((8..10).to_a) + end end end end From dad8987752371afc34693d6509312b79447acbd7 Mon Sep 17 00:00:00 2001 From: Mateusz Lata Date: Thu, 29 Jun 2023 12:36:15 +0200 Subject: [PATCH 6/6] Add test covering model without a scope. (#20) * Add test covering model without a scope. * ODA: Cover with test example performance of basic model with and without index. * ODA: Exclude performance test on GH actions. Clean up spec helper. * ODA: Don not run GH actions on drafts * ODA: fix typo * ODA: Implement suggestions after code review. --- .github/workflows/coverage.yml | 1 + .github/workflows/pull_request.yml | 1 + README.md | 4 +- spec/performance/index_spec.rb | 111 ++++++++++++++++++++++++++ spec/performance/no_index_spec.rb | 120 +++++++++++++++++++++++++++++ spec/spec_helper.rb | 8 ++ 6 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 spec/performance/index_spec.rb create mode 100644 spec/performance/no_index_spec.rb diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 85a977f..a6361ac 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -33,6 +33,7 @@ jobs: POSTGRES_USERNAME: postgres POSTGRES_PASSWORD: postgres SCHEMA: structure.sql + PERFORMANCE_TESTS_DISABLED: 1 steps: - name: Checkout repository uses: actions/checkout@v3 diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index f522fae..6712f67 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -58,6 +58,7 @@ jobs: POSTGRES_PASSWORD: postgres BUNDLE_GEMFILE: ${{ matrix.gemfile }} SPEC_DISABLE_DROP_DATABASE: 1 + PERFORMANCE_TESTS_DISABLED: 1 SCHEMA: structure.sql steps: - name: Checkout repository diff --git a/README.md b/README.md index 07819c3..1f44b14 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,7 @@ photo.destroy() Photo.profile.ordered.pluck(:label, :position) # => [["A", 0], ["B", 1], ["C", 2]] Photo.cover.ordered.pluck(:label, :position) # => [["D", 0], ["E", 1]] ``` -#### Default push front +#### Auto set ```ruby class Image < ActiveRecord::Base @@ -216,7 +216,7 @@ Image.create(label: "B") Image.ordered.pluck(:label, :position) # => [["A", 10], ["B", 11]] ``` -### Decremental sequence +#### Decremental sequence ```ruby class Image < ActiveRecord::Base diff --git a/spec/performance/index_spec.rb b/spec/performance/index_spec.rb new file mode 100644 index 0000000..e730c0d --- /dev/null +++ b/spec/performance/index_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +RSpec.describe "Performance with index" do + before(:all) do + attrs = (0...100_000).map do |position| + "('#{SecureRandom.alphanumeric(8)}', #{position})" + end + + BasicModel.connection.execute(<<-SQL) + INSERT INTO basic_models (name, position) + VALUES + #{attrs.join(', ')}; + SQL + end + + after(:all) { BasicModel.delete_all } + + describe "model without scope" do + context "on #update" do + subject { create(:basic_model).update!(position: position) } + + context "with position set as first value" do + let(:position) { 0 } + + it "takes less than 1.25s" do + expect(elapsed_time { subject }).to be < 1.25 + end + end + + context "with position set as middle value" do + let(:position) { 50_000 } + + it "takes less than 0.75s" do + expect(elapsed_time { subject }).to be < 0.75 + end + end + + context "with position set as last value" do + let(:position) { 99_999 } + + it "takes less than 0.15s" do + expect(elapsed_time { subject }).to be < 0.15 + end + end + end + + context "on #create" do + subject { create(:basic_model, **attrs) } + let(:attrs) { {} } + + context "without position specified" do + it "takes less than 0.07s" do + expect(elapsed_time { subject }).to be < 0.07 + end + end + + context "with postion set as 3/4 of total count" do + let(:attrs) { { position: 75_000 } } + + it "takes less than 0.35s" do + expect(elapsed_time { subject }).to be < 0.35 + end + end + + context "with position set as the middle value" do + let(:attrs) { { position: 50_000 } } + + it "takes less than 0.8s" do + expect(elapsed_time { subject }).to be < 0.8 + end + end + + context "with position set as first value" do + let(:attrs) { { position: 0 } } + + it "takes less than 1.5s" do + expect(elapsed_time { subject }).to be < 1.5 + end + end + end + + context "on #destroy" do + subject { record.destroy! } + let!(:record) { create(:basic_model, **attrs) } + + context "with position set as last value" do + let(:attrs) { { position: 100_000 } } + + it "takes less than 0.01s" do + expect(elapsed_time { subject }).to be < 0.01 + end + end + + context "with position set as middle value" do + let(:attrs) { { position: 50_000 } } + + it "takes less than 0.85s" do + expect(elapsed_time { subject }).to be < 0.85 + end + end + + context "with position set as first value" do + let(:attrs) { { position: 0 } } + + it "takes less than 1.5s" do + expect(elapsed_time { subject }).to be < 1.5 + end + end + end + end +end diff --git a/spec/performance/no_index_spec.rb b/spec/performance/no_index_spec.rb new file mode 100644 index 0000000..c7c777f --- /dev/null +++ b/spec/performance/no_index_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +RSpec.describe "Performance no index" do + before(:all) do + attrs = (0...100_000).map do |position| + "('#{SecureRandom.alphanumeric(8)}', #{position})" + end + + BasicModel.connection.execute(<<-SQL) + ALTER TABLE basic_models + DROP CONSTRAINT basic_models_position_key; + + INSERT INTO basic_models (name, position) + VALUES + #{attrs.join(', ')}; + SQL + end + + after(:all) do + BasicModel.delete_all + BasicModel.connection.execute(<<-SQL) + ALTER TABLE basic_models + ADD UNIQUE(position) DEFERRABLE INITIALLY DEFERRED; + SQL + end + + describe "model without scope" do + context "on #update" do + subject { create(:basic_model).update!(position: position) } + + context "with position set as first value" do + let(:position) { 0 } + + it "takes less than 0.9s" do + expect(elapsed_time { subject }).to be < 0.9 + end + end + + context "with position set as middle value" do + let(:position) { 50_000 } + + it "takes less than 0.5s" do + expect(elapsed_time { subject }).to be < 0.5 + end + end + + context "with position set as last value" do + let(:position) { 99_999 } + + it "takes less than 0.09s" do + expect(elapsed_time { subject }).to be < 0.09 + end + end + end + + context "on #create" do + subject { create(:basic_model, **attrs) } + let(:attrs) { {} } + + context "without position specified" do + it "takes less than 0.04s" do + expect(elapsed_time { subject }).to be < 0.04 + end + end + + context "with postion set as 3/4 of total count" do + let(:attrs) { { position: 75_000 } } + + it "takes less than 0.25s" do + expect(elapsed_time { subject }).to be < 0.25 + end + end + + context "with position set as the middle value" do + let(:attrs) { { position: 50_000 } } + + it "takes less than 0.5s" do + expect(elapsed_time { subject }).to be < 0.5 + end + end + + context "with position set as first value" do + let(:attrs) { { position: 0 } } + + it "takes less than 0.9s" do + expect(elapsed_time { subject }).to be < 0.9 + end + end + end + + context "on #destroy" do + subject { record.destroy! } + let!(:record) { create(:basic_model, **attrs) } + + context "with position set as last value" do + let(:attrs) { { position: 100_000 } } + + it "takes less than 0.025s" do + expect(elapsed_time { subject }).to be < 0.025 + end + end + + context "with position set as middle value" do + let(:attrs) { { position: 50_000 } } + + it "takes less than 0.6s" do + expect(elapsed_time { subject }).to be < 0.6 + end + end + + context "with position set as first value" do + let(:attrs) { { position: 0 } } + + it "takes less than 1.15s" do + expect(elapsed_time { subject }).to be < 1.15 + end + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f0b5112..9c38a0d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -36,6 +36,8 @@ end end + config.exclude_pattern = "spec/performance/*.rb" if ENV["PERFORMANCE_TESTS_DISABLED"] == "1" + config.before(:suite) do Rake::Task["db:create"].invoke Rake::Task["db:migrate"].invoke @@ -62,3 +64,9 @@ with.library :active_model end end + +def elapsed_time(&block) + Benchmark.measure do + block.call + end.real +end