This commit replaces the `full_name_required` setting with a new `full_name_requirement` setting to allow more flexibility with the name field in the signup form. The new setting has 2 options, "Required at signup" and "Optional at signup", which are equivalent to the true/false possibilities of the old setting, and a third option "Hidden at signup" that hides the name field from the signup form, making it effectively optional too.
New sites will have the "Hidden at signup" option as the default option, and existing site will continue to use the option that maps to their current configuration.
Internal topic: t/136746.
This commit adds the `add_request_rate_limiter` plugin API which allows plugins to add custom rate limiters on top of the default rate limiters which requests by a user's id or the request's IP address.
Example to add a rate limiter that rate limits all requests from Googlebot under the same rate limit bucket:
```
add_request_rate_limiter(
identifier: :country,
key: ->(request) { "country/#{DiscourseIpInfo.get(request.ip)[:country]}" },
activate_when: ->(request) { DiscourseIpInfo.get(request.ip)[:country].present? },
)
```
* Split `shutdown` into two separate methods for better control:
- `shutdown` - signals threads to stop accepting new work
- `wait_for_termination` - waits for threads to finish (with optional timeout)
* Add tracking of busy threads via `@busy_threads` Set
* Make idle_time parameter optional with 30-second default
* Improve thread spawning logic:
- Spawn initial thread immediately when work is posted
- Spawn additional threads when all threads are busy and work is queued
* Fix race condition in work distribution
* Add busy thread count to stats output
* Add test coverage for zero min_threads configuration
This commit makes the ThreadPool more reliable, easier to use, and adds
better visibility into its internal state.
---------
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This commit introduces a new ThreadPool class that provides efficient worker
thread management for background tasks. Key features include:
- Dynamic scaling from min to max threads based on workload
- Proper database connection management in multisite setup
- Graceful shutdown with task completion
- Robust error handling and logging
- FIFO task processing with a managed queue
- Configurable idle timeout for worker threads
The implementation is thoroughly tested, including stress tests, error
scenarios, and multisite compatibility.
There’s currently a bug when using a dedicated class as a policy in
services: if that class delegates its `#call` method (to an underlying
strategy object for example), then an error will be raised saying steps
aren’t allowed to provide default parameters.
This should not happen, and this patch fixes that issue.
This commit reimplements how we monitor Sidekiq processes that are
forked from the Unicorn master process. Prior to this change, we rely on
`Jobs::Heartbeat` to enqueue a `Jobs::RunHeartbeat` job every 3 minutes.
The `Jobs::RunHeartbeat` job then sets a Redis key with a timestamp. In
the Unicorn master process, we then fetch the timestamp that has been set
by the job from Redis every 30 minutes. If the timestamp has not been
updated for more than 30 minutes, we restart the Sidekiq process. The
fundamental flaw with this approach is that it fails to consider
deployments with multiple hosts and multiple Sidekiq processes. A
sidekiq process on a host may be in a bad state but the heartbeat check
will not restart the process because the `Jobs::RunHeartbeat` job is
still being executed by the working Sidekiq processes on other hosts.
In order to properly ensure that stuck Sidekiq processs are restarted,
we now rely on the [Sidekiq::ProcessSet](https://github.com/sidekiq/sidekiq/wiki/API#processes)
API that is supported by Sidekiq. The API provides us with "near real-time (updated every 5 sec)
info about the current set of Sidekiq processes running". The API
provides useful information like the hostname, pid and also when Sidekiq
last did its own heartbeat check. With that information, we can easily
determine if a Sidekiq process needs to be restarted from the Unicorn
master process.
`new_in_category` was using `first` instead of `limit`
This meant it gets an array and that means that you can not operate on it easily in a modifier.
This ensures we always give the modifier a relation, with the notable exception of suggested topics.
We're changing the default of hide_email_address_taken to true. This is a trade-off we want to make, as it prevents account enumeration with minimal impact on legitimate users. If you forget you have an account and try to sign up again with the same e-mail you'll receive an e-mail letting you know.
Previously, theme hbr files were compiled to an IIFE, which would be executed before the app is booted. That is causing silenced deprecations to be printed, because the deprecation-workflow isn't set up when the IIFE is run.
This commit updates the theme compiler so that it matches the ember-cli-based raw-hbs compiler. Templates are output to normal modules, which will then be loaded by the existing `eager-load-raw-templates` initializer. This runs after the app has started booting.
Currently, there are two ways (kind of) for accessing `params` inside a
service:
- when there is no contract or it hasn’t been reached yet, `params` is
just the hash that was provided to the service. To access a key, you
have to use the bracket notation `params[:my_key]`.
- when there is a contract and it has been executed successfully,
`params` now references the contract and the attributes are accessible
using methods (`params.my_key`).
This patch unifies how `params` exposes its attributes. Now, even if
there is no contract at all in a service, `params` will expose its
attributes through methods, that way things are more consistent.
This patch also makes sure there is always a `params` object available
even when no `params` key is provided to the service (this allows a
contract to fail because its attributes are blank instead of having the
service raising an error because it doesn’t find `params` in its context).
This patch aims to improve the steps inspector output:
- The service class name is displayed at the top.
- Next to each step is displayed the time it took to run said step.
- Steps that didn’t run are hidden.
- `#inspect` automatically outputs the error when it is present.
It doesn't make much sense to have the content of a `<details>` in an excerpt so I replaced them with "▶ summary" instead.
That way, they can't be (ab)used in user cards for example.
Reference - https://meta.discourse.org/t/335094
The new name may be too long for the bookmarks.name column and raise an
exception. This changes allows the remapper to truncate the new value to
fit (truncates to 100 characters).
We've seen in some communities abuse of user profile where bios and other fields are used in malicious ways, such as malware distribution. A common pattern between all the abuse cases we've seen is that the malicious actors tend to have 0 posts and have a low trust level.
To eliminate this abuse vector, or at least make it much less effective, we're making the following changes to user profiles:
1. Anonymous, TL0 and TL1 users cannot see any user profiles for users with 0 posts except for staff users
2. Anonymous and TL0 users can only see profiles of TL1 users and above
Users can always see their own profile, and they can still hide their profiles via the "Hide my public profile" preference. Staff can always see any user's profile.
Internal topic: t/142853.
Currently only system flags are translated. When we send message to the user that their post was deleted because of custom flag, we should default to custom flag name.
Spec was flaky cause work could still be in pipeline after the defer
length is 0. Our length denotes the backlog, not the in progress
count.
This adds a mechanism for gracefully stopping the queue and avoids
wait_for callse
Sometimes changes to "What's new?" feed items are made or the feed items are
removed altogether, and the polling interval to check for new features is 1 day.
This is quite long, so this commit introduces a "Check for updates"
button for admins to click on the "What's new?" page which will bust
the cache for the feed and check again at the new features endpoint.
This is limited to 5 times per minute to avoid rapid sending of
requests.
This fix handles the case where an In-Reply-To mail header
can contain multiple Message-IDs. We use this header to
try look up an EmailLog record to find the post to reply
to in the group email inbox flow.
Since the case where multiple In-Reply-To Message-IDs is
rare (we've only seen a couple of instances of this causing
errors in the wild), we are just going to use the first one
in the array.
Also, Discourse does not support replying to multiple posts
at once, so it doesn't really make sense to use multiple
In-Reply-To Message-IDs anyway.
* DEV: Gracefully handle `regex_replace` violations of column length constraints
This is a follow-up to the `remap` [refactor](9b0cfa99c5).
Similar to `remap`, the entire `regex_replace` operation fails if the new content exceeds the column’s max length.
This change introduces an optional mode, controlled by the new `skip_max_length_violations` param
to skip records eligible for `regex_replace` where the new content violates the max column length constraint.
It also includes updates to the exception message raised when `regex_replace` fails to include more details
* DEV: Remove string escapes in heredoc text
This patch adds a new step to services named `try`.
It’s useful to rescue exceptions that some steps could raise. That way,
if an exception is caught, the service will stop its execution and can
be inspected like with any other steps.
Just wrap the steps that can raise with a `try` block:
```ruby
try do
step :step_that_can_raise
step :another_step_that_can_raise
end
```
By default, `try` will catch any exception inheriting from
`StandardError`, but we can specify what exceptions to catch:
```ruby
try(ArgumentError, RuntimeError) do
step :will_raise
end
```
An outcome matcher has been added: `on_exceptions`. By default it will
be executed for any exception caught by the `try` step.
Here also, we can specify what exceptions to catch:
```ruby
on_exceptions(ArgumentError, RuntimeError) do |exception|
…
end
```
Finally, an RSpec matcher has been added:
```ruby
it { is_expected.to fail_with_exception }
# or
it { is_expected.to fail_with_exception(ArgumentError) }
```
* DEV: Gracefully handle remaps which violate DB column constraints
This change implements length constraint enforcement to skip remaps
which exceed column max lengths
* DEV: Only perform skipped column stats lookup when verbose is true
* DEV: Tidy up specs
* DEV: Make skipping violating remap behaviour opt-in
This change introduces a new `skip_max_length_violations` param for
`remap`, set to `false` by default to ensure we still continue to fail
hard when max lenth constraints are violated.
To aid in quick resolution when remaps fail, this change also
adds more context to the exception message to include the offending table
and column information
* Apply suggestions from code review
Co-authored-by: Gerhard Schlager <gerhard.schlager@discourse.org>
* FIX: Various fixes
- Linter errors
- Remap status "logger" early return condition
---------
Co-authored-by: Gerhard Schlager <gerhard.schlager@discourse.org>
It splits the hide_profile_and_presence user option and the default_hide_profile_and_presence site setting for more granular control. It keeps the option to hide the profile under /u/username/preferences/interface and adds the presence toggle in the quick user menu.
Co-authored-by: Régis Hanol <regis@hanol.fr>
This reverts commit 5a00a041f1a9a00fb31b18956769262af6f11037.
Implementation is currently not correct. Multiple uploads can share the
same etag but have different paths in the S3 bucket.
A "bad upload" in this context is a upload with a mismatched URL. This can happen when changing the S3 bucket used for uploads and the upload records in the database have not been remapped correctly.
When we added direct S3 uploads to Discourse, which use
presigned URLs, we never took into account the dualstack
endpoints for IPv6 on S3.
This commit fixes the issue by using the dualstack endpoints
for presigned URLs and requests, which are used in the
get-presigned-put and batch-presign-urls endpoints used when
directly uploading to S3.
It also makes regular S3 requests for `put` and so on use
dualstack URLs. It doesn't seem like there is a downside to
doing this, but a bunch of specs needed to be updated to reflect this.
The normalize_emails setting makes it so that only canonical e-mails are considered for validation purposes. This means disallowing "plus addressing". For example, with this enabled, bob@discourse.org and bob+foo@discourse.org are considered the same address, and you can only sign up with one of them.
Currently this is disabled by default, leading to a lot of spam sign-ups. It's healthier to consider this an opt-out setting.
This commit contains two changes to how our site setting
keyword system works:
1. Crowdin, our translation provider, does not support YAML lists,
so we are changing site setting keywords in server.en.yml to
be pipe-separated (|)
2. It's unclear to translators what they are supposed to do with
aliases of site settings where the name has changed, e.g.
min_trust_level_for_here_mention. Instead of getting these as
keywords from the yml file, we can discern these from
SiteSettings::DeprecatedSettings automatically, and still use
them for client-side search
These changes should help improve the situation for translators.
This commit removes the feature flag for the new /about page, enabling it for all sites, and removes the code for old the /about page.
Internal topic: t/140413.
This patch replaces the parameters provided to a service through
`params` by the contract object.
That way, it allows better consistency when accessing input params. For
example, if you have a service without a contract, to access a
parameter, you need to use `params[:my_parameter]`. But with a contract,
you do this through `contract.my_parameter`. Now, with this patch,
you’ll be able to access it through `params.my_parameter` or
`params[:my_parameter]`.
Some methods have been added to the contract object to better mimic a
Hash. That way, when accessing/using `params`, you don’t have to think
too much about it:
- `params.my_key` is also accessible through `params[:my_key]`.
- `params.my_key = value` can also be done through `params[:my_key] =
value`.
- `#slice` and `#merge` are available.
- `#to_hash` has been implemented, so the contract object will be
automatically cast as a hash by Ruby depending on the context. For
example, with an AR model, you can do this: `user.update(**params)`.
Currently in services, we don’t make a distinction between input
parameters, options and dependencies.
This can lead to user input modifying the service behavior, whereas it
was not the developer intention.
This patch addresses the issue by changing how data is provided to
services:
- `params` is now used to hold all data coming from outside (typically
user input from a controller) and a contract will take its values from
`params`.
- `options` is a new key to provide options to a service. This typically
allows changing a service behavior at runtime. It is, of course,
totally optional.
- `dependencies` is actually anything else provided to the service (like
`guardian`) and available directly from the context object.
The `service_params` helper in controllers has been updated to reflect
those changes, so most of the existing services didn’t need specific
changes.
The options block has the same DSL as contracts, as it’s also based on
`ActiveModel`. There aren’t any validations, though. Here’s an example:
```ruby
options do
attribute :allow_changing_hidden, :boolean, default: false
end
```
And here’s an example of how to call a service with the new keys:
```ruby
MyService.call(params: { key1: value1, … }, options: { my_option: true }, guardian:, …)
```
