Constraint wasn’t an issue.
A race condition is not based on likelihood. It is in fact simultaneous requests for the same thing which absolutely can and does happen. Sure likelihood of it happening in a low traffic app is small, but it can still happen. Do you really want to advocate building in an application bug based on likelihood?
The specific race condition is inserting a value that someone else is also trying to insert. We are not talking about a dirty vs clean record. A race condition of someone entering a value and validating it does not have presence is not an large issue. Many apps use AJAX to validate a username before it is submitted, it’s the same concept. Same thing when you want to buy a domain name. They don’t submit the name then wait to see if it exists first. They check to see if it is available, and if it is they allow the process.
You are advocating an anti pattern and I am not. Simple as that. The part where you say the anti pattern is more correct is wrong in itself. But, we can agree to disagree.
Large issue or not depends on the scale of your application. At worst it is less then a favorable user experience. The point is, checking first builds in the capability for a race condition. The constraint stops dirty data of course, so it relates to the user experience.
Sure, many apps do the username check first by whatever method. Many apps also use MySQL_* and put variables in the queries. That doesn’t make it ok.
In any of those cases, a potential race condition affecting the user experience is built into the app. There is no way it cannot be. There can only be one person with a particular username or domain name no matter how many people passed the availability check. The first request to make it to the server gets the name. In PHP, an Exception is thrown all by itself without a programmer throwing it. How it is handled is something else. Without seeing the code I couldn’t say, such as the case of domain names.
Unless I am mistaken, what you are talking about as anti-pattern or Exceptions for program flow doesn’t apply. That is the programmer throwing the exceptions. That is not the case here.
A duplicate constraint violation is an Exception in PHP all by itself so your previous statement about “actually aren’t exceptions” is incorrect.
Let’s just take it to code, as it always says what is what. We already agree on the duplicate constraint in place. Lets take 100 people that want the username “BOB” which is not currently in the DB. A username check will tell ALL 100 people they can have the name “BOB”. They all submit at the same time. The race is on. Only one of them (race winner) gets the username registered to them, the other 99 have a failure EVEN THOUGH the app said the name was available.
Tell me, how would you, @astonecipher solve this problem?
$sql = "SELECT COUNT(*) FROM Member WHERE email = ?";
$stmt = $this->_pdo->prepare($sql);
$stmt->execute([$data['email']]);
if($stmt->rowCount() == 1)
return -1;
$sql = "INSERT INTO Member (first_name, last_name, email) VALUES (?,?,?)";
$stmt = $this->_pdo->prepare($sql);
$stmt->execute([$data['first_name'],
$data['last_name'],
$data['email']]
);
return $this->_pdo->lastInsertId();
I don’t see a race condition being an issue for this. And it is the same practice used in high availability systems with 1m + users a week that I have worked on, though not in PHP.
There are two different types of used name checks. One is the kind is like what the Ajax and Domain name checks do. They check the name and give the user an Available/Not Available response without attempting the insert. (A double process to complete, 1.check username & respond to user. 2. Attempt insert on final submit) Easily a candidate for a race condition.
The other is like what your posted code is doing, which is a “single process” doing everything in one submit. It is still vulnerable to a race condition.
Lets be clear, I am talking about the UX (User Experience). Even with your code concurrent requests can still pass the username check (first query) and fail the second query if another request for the same username wins the race to the insert query. There are no locks on the DB when the first query runs so it wouldn’t be a problem for another request to get between the two query’s.
You didn’t show what you do with the -1 as far as the UX, nevertheless, the second query still fails for all concurrent requests (users) but one. One and only one insert can happen per unique username even though the first query says it is available. It basically tells a half truth. The first query says “The name is available as of the moment I asked the DB”, but it has no idea if another request has already got to the insert query before your request.
Additionally, especially on a high load DB, I know you would agree that one query is always better than two.
Take your code and do a concurrent request load test and see what happens. If I get time, I may do one in case this comes up again sometime. Code doesn’t lie.
I was thinking of doing so, but don’t have the ability from work. If I have the inclination, I will do so later.
FYI: A couple methods for concurrency testing are curl_multi_init, HttpRequestPool and ApacheBench.
I am sure there are probably plenty more options as well.