From cb28dde2be563935944d7a75a334845e1e4ff456 Mon Sep 17 00:00:00 2001 From: Jason Hill Date: Thu, 19 Feb 2026 16:52:29 -0500 Subject: [PATCH 1/6] Fix ES index create race in tests --- core/lib/workarea/elasticsearch/index.rb | 39 ++++++++++++----- .../workarea/elasticsearch/document_test.rb | 42 +++++++++++++++++++ 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/core/lib/workarea/elasticsearch/index.rb b/core/lib/workarea/elasticsearch/index.rb index 36787b657..b0375dfe1 100644 --- a/core/lib/workarea/elasticsearch/index.rb +++ b/core/lib/workarea/elasticsearch/index.rb @@ -18,23 +18,40 @@ def exists? Workarea.elasticsearch.indices.exists?(index: name) end + # Create the index, optionally deleting it first when +force+ is true. + # + # The previous implementation used an +unless exists?+ guard which + # introduced a TOCTOU (time-of-check/time-of-use) race condition: the + # cluster state visible to +exists?+ could differ from the state seen by + # the subsequent +create+ call, causing intermittent + # +resource_already_exists_exception+ (400) errors in CI and during rapid + # sequential test runs. When +create!+ raised that exception the index + # remained deleted (because +delete!+ had already run), which caused the + # cascade +index_not_found_exception+ (404) seen in later test operations. + # + # The fix removes the guard and instead rescues the 400 BadRequest that + # Elasticsearch raises when the index already exists. This makes the + # operation atomic from the caller's perspective: if the index was already + # present we simply skip creation, just as the old +unless exists?+ path + # intended – but without the window for a concurrent creation to slip in. + # def create!(force: false) delete! if force - unless exists? - Workarea.elasticsearch.indices.create( - index: name, - body: { - settings: Search::Settings.current.elasticsearch_settings, - mappings: mappings, - aliases: aliases - } - ) - end + Workarea.elasticsearch.indices.create( + index: name, + body: { + settings: Search::Settings.current.elasticsearch_settings, + mappings: mappings, + aliases: aliases + } + ) + rescue ::Elasticsearch::Transport::Transport::Errors::BadRequest => e + raise unless e.message.include?('resource_already_exists_exception') end def delete! - Workarea.elasticsearch.indices.delete(index: name, ignore: 404) + Workarea.elasticsearch.indices.delete(index: name, ignore: [404]) end def while_closed diff --git a/core/test/lib/workarea/elasticsearch/document_test.rb b/core/test/lib/workarea/elasticsearch/document_test.rb index dcd862f57..214642ef4 100644 --- a/core/test/lib/workarea/elasticsearch/document_test.rb +++ b/core/test/lib/workarea/elasticsearch/document_test.rb @@ -143,6 +143,48 @@ def test_scroll ensure Foo.clear_scroll(results['_scroll_id']) end + + # Regression: Index#create! must be idempotent. + # + # The old implementation used `unless exists?` before calling + # `indices.create`. This introduced a TOCTOU race: between the `exists?` + # check and the actual `create` call another request could have created + # (or re-created) the same index, causing a + # `resource_already_exists_exception` (HTTP 400). When that exception + # propagated the index was left in a deleted state (because `delete!` had + # already run), which then caused `index_not_found_exception` (404) for + # any search performed by the same test. + # + # The fix removes the guard and rescues the 400 BadRequest instead, making + # the operation atomic. + def test_create_idempotent_when_index_already_exists + index = Foo.current_index + + # First call – index exists from setup_indexes; should not raise. + assert_nothing_raised { index.create! } + end + + def test_create_force_recreates_index_and_remains_searchable + Foo.save(id: 'before') + Foo.current_index.wait_for_health + + # force: true must delete-and-recreate; previously saved documents are gone + Foo.reset_indexes! + + assert_equal(0, Foo.count, 'expected empty index after force-recreate') + + Foo.save(id: 'after') + assert_equal(1, Foo.count, 'expected new document after force-recreate') + end + + def test_create_force_idempotent_called_twice + # Calling reset_indexes! back-to-back must not raise + # resource_already_exists_exception on the second call. + assert_nothing_raised do + Foo.reset_indexes! + Foo.reset_indexes! + end + end end end end From 7a6633f63f664c5c8a3d68b4bae877195a889006 Mon Sep 17 00:00:00 2001 From: Jason Hill Date: Thu, 19 Feb 2026 17:00:13 -0500 Subject: [PATCH 2/6] WA-NEW-002: guard callback workers + truncate all Mongoid clients in tests --- .../workers/workarea/bust_navigation_cache.rb | 2 ++ .../workers/workarea/generate_promo_codes.rb | 2 ++ .../workers/workarea/index_categorization.rb | 2 ++ .../workers/workarea/index_product_rule.rb | 2 ++ .../workarea/mark_discounts_as_redeemed.rb | 2 ++ .../workers/workarea/save_order_metrics.rb | 2 ++ .../workarea/save_user_order_details.rb | 2 ++ .../app/workers/workarea/send_refund_email.rb | 2 ++ .../workarea/synchronize_user_metrics.rb | 5 ++++ testing/lib/workarea/test_case.rb | 27 ++++++++++++++++++- 10 files changed, 47 insertions(+), 1 deletion(-) diff --git a/core/app/workers/workarea/bust_navigation_cache.rb b/core/app/workers/workarea/bust_navigation_cache.rb index 79eb60d39..d3ed20fe3 100644 --- a/core/app/workers/workarea/bust_navigation_cache.rb +++ b/core/app/workers/workarea/bust_navigation_cache.rb @@ -13,6 +13,8 @@ def perform(id) taxon = Navigation::Taxon.find(id) taxon.ancestors.each(&:touch) taxon.descendants.each(&:touch) + rescue Mongoid::Errors::DocumentNotFound + nil end end end diff --git a/core/app/workers/workarea/generate_promo_codes.rb b/core/app/workers/workarea/generate_promo_codes.rb index 8095b6738..dc6765eae 100644 --- a/core/app/workers/workarea/generate_promo_codes.rb +++ b/core/app/workers/workarea/generate_promo_codes.rb @@ -7,6 +7,8 @@ class GeneratePromoCodes def perform(id) Pricing::Discount::CodeList.find(id).generate_promo_codes! + rescue Mongoid::Errors::DocumentNotFound + nil end end end diff --git a/core/app/workers/workarea/index_categorization.rb b/core/app/workers/workarea/index_categorization.rb index 9aa6aac63..4e960e56d 100644 --- a/core/app/workers/workarea/index_categorization.rb +++ b/core/app/workers/workarea/index_categorization.rb @@ -15,6 +15,8 @@ def self.perform(category) def perform(id) category = Catalog::Category.find(id) self.class.perform(category) + rescue Mongoid::Errors::DocumentNotFound + nil end end end diff --git a/core/app/workers/workarea/index_product_rule.rb b/core/app/workers/workarea/index_product_rule.rb index c5799197e..59813db5b 100644 --- a/core/app/workers/workarea/index_product_rule.rb +++ b/core/app/workers/workarea/index_product_rule.rb @@ -17,6 +17,8 @@ class IndexProductRule def perform(id) product_list = Catalog::Category.find(id) IndexCategorization.perform(product_list) + rescue Mongoid::Errors::DocumentNotFound + nil end end end diff --git a/core/app/workers/workarea/mark_discounts_as_redeemed.rb b/core/app/workers/workarea/mark_discounts_as_redeemed.rb index f6ba9e55a..b76231937 100644 --- a/core/app/workers/workarea/mark_discounts_as_redeemed.rb +++ b/core/app/workers/workarea/mark_discounts_as_redeemed.rb @@ -9,6 +9,8 @@ def perform(order_id) order = Order.find(order_id) shippings = Shipping.where(order_id: order_id).to_a mark_redeemed(order, shippings) + rescue Mongoid::Errors::DocumentNotFound + nil end def mark_redeemed(order, shippings) diff --git a/core/app/workers/workarea/save_order_metrics.rb b/core/app/workers/workarea/save_order_metrics.rb index ad6f95c5c..cfb667e53 100644 --- a/core/app/workers/workarea/save_order_metrics.rb +++ b/core/app/workers/workarea/save_order_metrics.rb @@ -36,6 +36,8 @@ def save_user(metrics) def perform(order_id) self.class.perform(Order.find(order_id)) + rescue Mongoid::Errors::DocumentNotFound + nil end end end diff --git a/core/app/workers/workarea/save_user_order_details.rb b/core/app/workers/workarea/save_user_order_details.rb index bc6d77950..f6ab9dfa4 100644 --- a/core/app/workers/workarea/save_user_order_details.rb +++ b/core/app/workers/workarea/save_user_order_details.rb @@ -13,6 +13,8 @@ def perform(order_id) save_payment_details(order, user) save_shipping_details(order, user) + rescue Mongoid::Errors::DocumentNotFound + nil end def save_payment_details(order, user) diff --git a/core/app/workers/workarea/send_refund_email.rb b/core/app/workers/workarea/send_refund_email.rb index 89e4d86d4..38ea53409 100644 --- a/core/app/workers/workarea/send_refund_email.rb +++ b/core/app/workers/workarea/send_refund_email.rb @@ -12,6 +12,8 @@ def perform(id) refund = Payment::Refund.find(id) return if refund.total.zero? Storefront::PaymentMailer.refunded(id.to_s).deliver_now + rescue Mongoid::Errors::DocumentNotFound + nil end end end diff --git a/core/app/workers/workarea/synchronize_user_metrics.rb b/core/app/workers/workarea/synchronize_user_metrics.rb index 105d82ebb..e9952c1e8 100644 --- a/core/app/workers/workarea/synchronize_user_metrics.rb +++ b/core/app/workers/workarea/synchronize_user_metrics.rb @@ -31,6 +31,11 @@ def perform(id) }, upsert: true ) + rescue Mongoid::Errors::DocumentNotFound + # Callback workers may execute after the underlying document has been + # removed (e.g. test cleanup / truncation). Missing documents should be + # treated as a no-op. + nil end end end diff --git a/testing/lib/workarea/test_case.rb b/testing/lib/workarea/test_case.rb index 2d073bfd6..8a7e7c93f 100644 --- a/testing/lib/workarea/test_case.rb +++ b/testing/lib/workarea/test_case.rb @@ -71,10 +71,23 @@ module Workers module SearchIndexing extend ActiveSupport::Concern + def wait_for_elasticsearch!(timeout: 30) + start = Time.now + + loop do + Workarea.elasticsearch.cluster.health(wait_for_status: "yellow") + break + rescue Faraday::ConnectionFailed, ::Elasticsearch::Transport::Transport::Errors::ServiceUnavailable, ::Elasticsearch::Transport::Transport::Errors::BadGateway + raise if (Time.now - start) > timeout + sleep 0.5 + end + end + included do setup do Workarea.config.auto_refresh_search = true WebMock.disable_net_connect!(allow_localhost: true) + wait_for_elasticsearch! Workarea::Elasticsearch::Document.all.each(&:reset_indexes!) Workarea::Search::Storefront.ensure_dynamic_mappings end @@ -241,9 +254,21 @@ module Setup extend ActiveSupport::Concern include ActiveJob::TestHelper + def truncate_all_mongoid_clients! + # Mongoid.truncate! only truncates the global (default) client. + # Workarea uses additional clients (e.g. :metrics), so ensure we clear + # data for all configured clients to avoid cross-test pollution. + Mongoid::Clients.clients.values.each do |client| + client.database.collections.each do |collection| + next if collection.name.start_with?('system.') + collection.find.delete_many + end + end + end + included do setup do - Mongoid.truncate! + truncate_all_mongoid_clients! Workarea.redis.flushdb WebMock.disable_net_connect!(allow_localhost: true) ActionMailer::Base.deliveries.clear From 5f5921317991d9f86774d50f116ec3d3772a3f04 Mon Sep 17 00:00:00 2001 From: Jason Hill Date: Thu, 19 Feb 2026 20:47:44 -0500 Subject: [PATCH 3/6] Fix DataFile password randomization tests to be deterministic --- core/test/models/workarea/data_file/csv_test.rb | 17 ++++++++++++----- .../test/models/workarea/data_file/json_test.rb | 4 ++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/core/test/models/workarea/data_file/csv_test.rb b/core/test/models/workarea/data_file/csv_test.rb index e86ed6dc0..2eb17927d 100644 --- a/core/test/models/workarea/data_file/csv_test.rb +++ b/core/test/models/workarea/data_file/csv_test.rb @@ -369,14 +369,21 @@ def test_randomize_password_when_missing_and_new_record file_type: 'csv' ) + # Deterministic: ensure we always assign a randomized password + # when the password column is missing for a new user. + SecureRandom.stubs(:hex).returns('0123456789abcdef0123') + assert_difference -> { User.count } do Csv.new(import).import! - user = User.find_by_email('missing@example.com') - - assert user.present?, 'user not imported' - refute user.authenticate('password1'), - "password authenticated when it shouldn't have" end + + user = User.find_by_email('missing@example.com') + + assert user.present?, 'user not imported' + assert user.authenticate('0123456789abcdef0123_aA1'), + 'random password not assigned as expected' + refute user.authenticate('password1'), + "password authenticated when it shouldn't have" end def test_ignore_password_column_when_user_exists diff --git a/core/test/models/workarea/data_file/json_test.rb b/core/test/models/workarea/data_file/json_test.rb index 1d795dd1c..df425f677 100644 --- a/core/test/models/workarea/data_file/json_test.rb +++ b/core/test/models/workarea/data_file/json_test.rb @@ -60,6 +60,8 @@ def test_randomize_password_when_missing_and_new_record file_type: 'json' ) + SecureRandom.stubs(:hex).returns('0123456789abcdef0123') + assert_difference -> { User.count } do Json.new(data).import! end @@ -67,6 +69,8 @@ def test_randomize_password_when_missing_and_new_record user = User.find_by_email('test@example.com') assert user.present?, 'user not imported' + assert user.authenticate('0123456789abcdef0123_aA1'), + 'random password not assigned as expected' refute user.authenticate(password), "password authenticated when it shouldn't have" end From d8dcd912b5fdae74e912ae3a7cfc3dc77a26b365 Mon Sep 17 00:00:00 2001 From: Jason Hill Date: Thu, 19 Feb 2026 20:58:27 -0500 Subject: [PATCH 4/6] Fix metrics week calculations across DST --- core/app/models/workarea/metrics/by_week.rb | 9 ++++- core/app/models/workarea/metrics/scoring.rb | 8 +++- .../workarea/metrics/product_by_week_test.rb | 38 ++++++++++++++----- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/core/app/models/workarea/metrics/by_week.rb b/core/app/models/workarea/metrics/by_week.rb index 6ad099909..a69ea04cd 100644 --- a/core/app/models/workarea/metrics/by_week.rb +++ b/core/app/models/workarea/metrics/by_week.rb @@ -13,9 +13,14 @@ module ByWeek index({ reporting_on: 1 }, { expire_after_seconds: 2.years.seconds.to_i }) scope :last_week, -> do + # Use date math to avoid DST-related boundary shifts. + # We want the previous calendar week, not "now minus 7 days". + start_of_this_week = Time.current.to_date.beginning_of_week + start_of_last_week = start_of_this_week - 1.week + where( - :reporting_on.gte => Time.current.last_week, - :reporting_on.lt => Time.current.last_week.end_of_week + :reporting_on.gte => start_of_last_week.in_time_zone, + :reporting_on.lt => start_of_this_week.in_time_zone ) end end diff --git a/core/app/models/workarea/metrics/scoring.rb b/core/app/models/workarea/metrics/scoring.rb index fd2bdb7d8..baf2eb6ff 100644 --- a/core/app/models/workarea/metrics/scoring.rb +++ b/core/app/models/workarea/metrics/scoring.rb @@ -22,8 +22,12 @@ def score(field) end def weeks_ago - difference = Time.current.beginning_of_week - reporting_on.beginning_of_week - difference / 1.week + # Use date math instead of second-based math to avoid DST boundary issues. + # (e.g. a "week" containing a DST shift is not always 7 * 24 hours) + current_week = Time.current.to_date.beginning_of_week + reporting_week = reporting_on.to_date.beginning_of_week + + ((current_week - reporting_week) / 7).to_i end end end diff --git a/core/test/models/workarea/metrics/product_by_week_test.rb b/core/test/models/workarea/metrics/product_by_week_test.rb index 9696809b3..bbe5e028c 100644 --- a/core/test/models/workarea/metrics/product_by_week_test.rb +++ b/core/test/models/workarea/metrics/product_by_week_test.rb @@ -3,6 +3,10 @@ module Workarea module Metrics class ProductByWeekTest < TestCase + setup do + Metrics::ProductByWeek.delete_all + Metrics::ProductForLastWeek.delete_all + end def test_last_week two_weeks_ago = create_product_by_week(reporting_on: Time.zone.local(2018, 11, 25)) last_week = create_product_by_week(reporting_on: Time.zone.local(2018, 12, 2)) @@ -117,20 +121,34 @@ def test_score def test_weeks_ago model = create_product_by_week(reporting_on: Time.zone.local(2019, 1, 25)) - travel_to Time.zone.local(2019, 1, 25) - assert_equal(0, model.weeks_ago) + travel_to Time.zone.local(2019, 1, 25) do + assert_equal(0, model.weeks_ago) + end + + travel_to Time.zone.local(2019, 1, 27) do + assert_equal(0, model.weeks_ago) + end + + travel_to Time.zone.local(2019, 2, 8) do + assert_equal(2, model.weeks_ago) + end - travel_to Time.zone.local(2019, 1, 27) - assert_equal(0, model.weeks_ago) + travel_to Time.zone.local(2019, 2, 9) do + assert_equal(2, model.weeks_ago) + end - travel_to Time.zone.local(2019, 2, 8) - assert_equal(2, model.weeks_ago) + travel_to Time.zone.local(2019, 2, 11) do + assert_equal(3, model.weeks_ago) + end - travel_to Time.zone.local(2019, 2, 9) - assert_equal(2, model.weeks_ago) + # Regression: DST boundaries should not affect week math. + Time.use_zone('Eastern Time (US & Canada)') do + dst_model = create_product_by_week(reporting_on: Time.zone.local(2019, 3, 4, 0, 0, 0)) - travel_to Time.zone.local(2019, 2, 11) - assert_equal(3, model.weeks_ago) + travel_to Time.zone.local(2019, 3, 11, 0, 0, 0) do + assert_equal(1, dst_model.weeks_ago) + end + end end end end From ba0ec8a9bb2f3918c918c4285305567e15f2d18d Mon Sep 17 00:00:00 2001 From: Jason Hill Date: Thu, 19 Feb 2026 21:14:12 -0500 Subject: [PATCH 5/6] WA-NEW-005: Remove unsupported use_dis_max from query_string queries (ES7) The use_dis_max parameter was removed from the Elasticsearch query_string query in ES7. It caused a 400 parsing_exception: [query_string] query does not support [use_dis_max] In ES7 the equivalent behavior is achieved via tie_breaker (default 0.0 gives pure dis_max scoring). Where tie_breaker was already present (product search) only use_dis_max was removed; where it was absent (admin index search, help search) removing the flag restores the ES7 default (tie_breaker=0). Files changed: - core/app/queries/workarea/search/admin_index_search.rb - core/app/queries/workarea/search/help_search.rb - core/app/queries/workarea/search/product_search.rb --- core/app/queries/workarea/search/admin_index_search.rb | 3 +-- core/app/queries/workarea/search/help_search.rb | 3 +-- core/app/queries/workarea/search/product_search.rb | 1 - 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/core/app/queries/workarea/search/admin_index_search.rb b/core/app/queries/workarea/search/admin_index_search.rb index ac5562b10..59c6f5c52 100644 --- a/core/app/queries/workarea/search/admin_index_search.rb +++ b/core/app/queries/workarea/search/admin_index_search.rb @@ -49,8 +49,7 @@ def query { query_string: { query: sanitized_query, - fields: fields, - use_dis_max: true + fields: fields } }, { diff --git a/core/app/queries/workarea/search/help_search.rb b/core/app/queries/workarea/search/help_search.rb index c059f3978..7d396af99 100644 --- a/core/app/queries/workarea/search/help_search.rb +++ b/core/app/queries/workarea/search/help_search.rb @@ -28,8 +28,7 @@ def query { query_string: { query: sanitized_query, - fields: %w(name^1.5 facets^0.75 body), - use_dis_max: true + fields: %w(name^1.5 facets^0.75 body) } } end diff --git a/core/app/queries/workarea/search/product_search.rb b/core/app/queries/workarea/search/product_search.rb index e60a5ffd0..b3c617991 100644 --- a/core/app/queries/workarea/search/product_search.rb +++ b/core/app/queries/workarea/search/product_search.rb @@ -170,7 +170,6 @@ def general_search_clause query_string: { query: customization.rewrite, fields: boosted_fields, - use_dis_max: true, default_operator: default_operator, tie_breaker: Workarea.config.search_dismax_tie_breaker } From 31b767f552ac2d60f743b940daf375ded3442c11 Mon Sep 17 00:00:00 2001 From: Jason Hill Date: Thu, 19 Feb 2026 21:54:01 -0500 Subject: [PATCH 6/6] WA-NEW-006: Fix Payment#complete? to reject zero-amount credit card authorizations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two interrelated fixes: 1. Pricing::Request#save_order — Mongoid 7 as_document returns nested BSON::Document values (e.g. Money fields) that update_attributes! cannot properly deserialize. Using as_json performs a deep, recursive JSON-safe conversion so Money totals are correctly persisted after pricing runs. Also switch reverse_merge to use string key 'items' (not symbol) to avoid creating duplicate symbol/string key pairs. 2. Checkout::Steps::Payment#complete? — add credit_card_authorizable? guard: if a credit card tender is present its amount must be positive. A zero-amount credit card results in a no-op authorization and must not allow an order to proceed to placement. This is a defensive check on top of the purchasable? amount comparison. Fixes: payment_test.rb test_complete? refute at line 95 Verified: payment_test, checkout_test, pricing/request_test all green --- .../models/workarea/checkout/steps/payment.rb | 18 +++++++++++++++++- core/app/models/workarea/pricing/request.rb | 13 +++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/core/app/models/workarea/checkout/steps/payment.rb b/core/app/models/workarea/checkout/steps/payment.rb index 5948cde29..7d8f80b3f 100644 --- a/core/app/models/workarea/checkout/steps/payment.rb +++ b/core/app/models/workarea/checkout/steps/payment.rb @@ -30,15 +30,31 @@ def update(params = {}) # Requires: # * order to be purchasable # * payment purchasable for order total_price + # * credit card, if present, must have a positive authorization amount + # (a zero-amount credit card indicates no real authorization will occur) # # @return [Boolean] # def complete? - order.purchasable? && payment.purchasable?(order.total_price) + order.purchasable? && + payment.purchasable?(order.total_price) && + credit_card_authorizable? end private + # Ensure a credit card tender, if one is set, will actually be + # authorized (i.e. has a positive amount). A zero-amount credit card + # results in a no-op authorization and should not be treated as a + # complete payment. + # + # @return [Boolean] + # + def credit_card_authorizable? + return true unless payment.credit_card? + payment.credit_card.amount > 0.to_m + end + def set_payment_profile payment.set(profile_id: payment_profile.try(:id)) end diff --git a/core/app/models/workarea/pricing/request.rb b/core/app/models/workarea/pricing/request.rb index af9f6247f..74145dc09 100644 --- a/core/app/models/workarea/pricing/request.rb +++ b/core/app/models/workarea/pricing/request.rb @@ -136,8 +136,17 @@ def all_skus def save_order # as_document won't contain items in the hash if there isn't any items left. - # ensure the items get cleared out when this happens - @persisted_order.update_attributes!(order.as_document.reverse_merge(items: [])) + # ensure the items get cleared out when this happens. + # + # Use as_json for a deep-JSON-safe conversion of the document — Mongoid 7's + # as_document returns nested BSON::Document instances for embedded fields + # (e.g. Money hashes) which update_attributes! cannot properly deserialize, + # causing pricing totals to silently remain at zero. as_json recursively + # converts all values to JSON-safe plain Ruby types that Mongoid can cast. + # Using a string key for "items" avoids creating both symbol and string keys + # which can cause unpredictable attribute-merge behavior. + attrs = order.as_json.reverse_merge("items" => []) + @persisted_order.update_attributes!(attrs) cache_key.order = @persisted_order @persisted_order.set(pricing_cache_key: cache_key.to_s) end