Merge pull request Aspen-Discovery#2239 from LeoStoyanovByWater/DIS-2…

…50-fix-user-usage-v2 DIS-250: Fixed User Usage Checks & Added Atomic SQL Updates
bywatersolutions · Feb 13, 2025 · e82bff6 · e82bff6
2 parents 73a3f28 + d2813c1
commit e82bff6
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 12 deletions.
diff --git a/code/web/bootstrap.php b/code/web/bootstrap.php
@@ -64,6 +64,9 @@
 
 initMemcache();
 initDatabase();
+$timer->logTime("Initialized Database");
+requireSystemLibraries();
+initLocale();
 
 if ($aspenUsage->getInstance() != 'aspen_internal') {
 	$isValidServerName = true;
@@ -146,19 +149,35 @@
 	$foundExisting = $usageByUserAgent->find(true);
 
 	if ($userAgent->blockAccess) {
-		$usageByUserAgent->numBlockedRequests++;
-		if ($usageByUserAgent->update() == 0){
-			$logger->log("Could not update user agent usage", Logger::LOG_ERROR);
-			$logger->log($usageByUserAgent->getLastError(), Logger::LOG_ERROR);
+		if ($foundExisting) {
+			$updateResult = $usageByUserAgent->incrementNumBlockedRequests();
+			if ($updateResult === false) {
+				$logger->log("Could not update blocked user agent usage", Logger::LOG_ERROR);
+				$logger->log($usageByUserAgent->getLastError(), Logger::LOG_ERROR);
+			}
+		} else {
+			$usageByUserAgent->numBlockedRequests = 1;
+			if (!$usageByUserAgent->insert()) {
+				$logger->log("Could not insert blocked user agent usage", Logger::LOG_ERROR);
+				$logger->log($usageByUserAgent->getLastError(), Logger::LOG_ERROR);
+			}
 		}
 		http_response_code(403);
 		echo("<h1>Forbidden</h1><p><strong>We are unable to handle your request.</strong></p>");
 		die();
 	}else{
-		$usageByUserAgent->numRequests++;
-		if ($usageByUserAgent->update() == 0){
-			$logger->log("Could not update user agent usage", Logger::LOG_ERROR);
-			$logger->log($usageByUserAgent->getLastError(), Logger::LOG_ERROR);
+		if ($foundExisting) {
+			$updateResult = $usageByUserAgent->incrementNumRequests();
+			if ($updateResult === false) {
+				$logger->log("Could not update user agent usage", Logger::LOG_ERROR);
+				$logger->log($usageByUserAgent->getLastError(), Logger::LOG_ERROR);
+			}
+		} else {
+			$usageByUserAgent->numRequests = 1;
+			if (!$usageByUserAgent->insert()) {
+				$logger->log("Could not insert user agent usage", Logger::LOG_ERROR);
+				$logger->log($usageByUserAgent->getLastError(), Logger::LOG_ERROR);
+			}
 		}
 	}
 }catch (Exception $e) {
@@ -181,10 +200,6 @@
 $usageByIPAddress->lastRequest = time();
 $usageByIPAddress->numRequests++;
 
-$timer->logTime("Initialized Database");
-requireSystemLibraries();
-initLocale();
-
 //Check to see if we should be blocking based on the IP address
 if (IPAddress::isClientIpBlocked()) {
 	$aspenUsage->blockedRequests++;

diff --git a/code/web/release_notes/25.02.00.MD b/code/web/release_notes/25.02.00.MD
@@ -221,6 +221,8 @@
 ### Usage Tables Updates
 - Added a unique composite index on `(userAgentId, year, month, instance)` for `usage_by_user_agent`. (DIS-250) (*LS*)
 - Added update call in `bootstrap.php` to update IP Address request counter. (DIS-250) (*LS*)
+- Moved the initialization of local time in `bootstrap.php` to occur before any usage table operations. (DIS-250) (*LS*)
+- Changed update calls to user usage table to be atomic SQL queries. (DIS-250) (*LS*)
 
 ### User List Updates
 - Modified `UserList::getListEntries()` to do SQL-level pagination rather than PHP array slicing of all list entries. (DIS-305) (*LS*)

diff --git a/code/web/sys/SystemLogging/UsageByUserAgent.php b/code/web/sys/SystemLogging/UsageByUserAgent.php
@@ -15,4 +15,12 @@ class UsageByUserAgent extends DataObject {
 	function objectHistoryEnabled() : bool {
 		return false;
 	}
+
+	public function incrementNumRequests(): bool {
+		return $this->query("UPDATE usage_by_user_agent SET numRequests = numRequests + 1 WHERE id = {$this->id}");
+	}
+
+	public function incrementNumBlockedRequests(): bool {
+		return $this->query("UPDATE usage_by_user_agent SET numBlockedRequests = numBlockedRequests + 1 WHERE id = {$this->id}");
+	}
 }