Move USING_AJAX into the Session class

Defining `USING_AJAX` as a global constant made it hard to test, since it cannot be modified once set. Moving it into the Session class as the `ajax` property makes it easier to reset, and also much more clear when it will be defined. In general, we should avoid global constants when they are not configuration parameters.
smrealms · Nov 22, 2022 · 6cda6a9 · 6cda6a9
1 parent d048135
commit 6cda6a9
Show file tree

Hide file tree

Showing 10 changed files with 34 additions and 22 deletions.
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -27,7 +27,6 @@ parameters:
         - RECAPTCHA_PRIVATE
         - SMTP_HOSTNAME
         - TWITTER_CONSUMER_KEY
-        - USING_AJAX
         # We code in protection against a value of 0
         - SmrMines::TOTAL_ENEMY_MINES_MODIFIER
 

diff --git a/src/bootstrap.php b/src/bootstrap.php
@@ -27,6 +27,8 @@ function logException(Throwable $e): void {
 			$message .= $delim;
 		}
 
+		$message .= 'ajax: ' . var_export($session->ajax, true) . "\n";
+
 		$var = $session->hasCurrentVar() ? $session->getCurrentVar() : null;
 		$message .= '$var: ' . print_r($var, true) . $delim;
 	}
@@ -41,7 +43,6 @@ function logException(Throwable $e): void {
 	$message .=
 		'User IP: ' . getIpAddress() . "\n" .
 		'User Agent: ' . ($_SERVER['HTTP_USER_AGENT'] ?? 'undefined') . "\n" .
-		'USING_AJAX: ' . (defined('USING_AJAX') ? var_export(USING_AJAX, true) : 'undefined') . "\n" .
 		'URL: ' . (defined('URL') ? URL : 'undefined');
 
 	// Try to release lock so they can carry on normally
@@ -122,7 +123,7 @@ function handleException(Throwable $e): void {
 
 	// If this is an ajax update, we don't really have a way to redirect
 	// to an error page at this time.
-	if (!ENABLE_DEBUG && (!defined('USING_AJAX') || !USING_AJAX)) {
+	if (!ENABLE_DEBUG) {
 		header('location: /error.php?msg=' . urlencode($errorType));
 	}
 }

diff --git a/src/config.php b/src/config.php
@@ -471,5 +471,3 @@
 
 const AJAX_DEFAULT_REFRESH_TIME = 1500;
 const AJAX_UNPROTECTED_REFRESH_TIME = 800;
-
-define('USING_AJAX', isset($_REQUEST['ajax']) && $_REQUEST['ajax'] == 1);
diff --git a/src/engine/Default/message_view.php b/src/engine/Default/message_view.php
@@ -90,7 +90,8 @@
 	}
 }
 
-if ($page == 0 && !USING_AJAX) {
+// This should really be part of a (pre)processing page
+if ($page == 0 && !$session->ajax) {
 	$player->setMessagesRead($folderID);
 }
 

diff --git a/src/engine/Default/toggle_processing.php b/src/engine/Default/toggle_processing.php
@@ -5,7 +5,7 @@
 
 $player->setDisplayWeapons(!$player->isDisplayWeapons());
 // If this is called by ajax, we don't want to do any forwarding
-if (USING_AJAX) {
+if ($session->ajax) {
 	exit;
 }
 

diff --git a/src/htdocs/loader.php b/src/htdocs/loader.php
@@ -19,8 +19,6 @@
 	// *
 	// ********************************
 
-	//echo '<pre>';echo_r($session);echo'</pre>';
-	//exit;
 	// do we have a session?
 	$session = Smr\Session::getInstance();
 	if (!$session->hasAccount()) {
@@ -30,7 +28,7 @@
 
 	// check if we got a sn number with our url
 	if (empty($session->getSN())) {
-		if (!USING_AJAX) {
+		if (!$session->ajax) {
 			create_error('Your browser lost the SN. Try to reload the page!');
 		} else {
 			exit;
@@ -39,7 +37,7 @@
 
 	// do we have such a container object in the db?
 	if ($session->hasCurrentVar() === false) {
-		if (!USING_AJAX) {
+		if (!$session->ajax) {
 			create_error('Please avoid using the back button!');
 		} else {
 			exit;

diff --git a/src/lib/Default/AbstractSmrPlayer.php b/src/lib/Default/AbstractSmrPlayer.php
@@ -2256,7 +2256,7 @@ public function removeUnderAttack(): bool {
 			return $var['UnderAttack'];
 		}
 		$underAttack = $this->isUnderAttack();
-		if ($underAttack && !USING_AJAX) {
+		if ($underAttack && !$session->ajax) {
 			$var['UnderAttack'] = $underAttack; //Remember we are under attack for AJAX
 		}
 		$this->setUnderAttack(false);

diff --git a/src/lib/Default/smr.inc.php b/src/lib/Default/smr.inc.php
@@ -202,7 +202,8 @@ function handleUserError(string $message): never {
 	// Template from the original page.
 	DiContainer::getContainer()->reset(Template::class);
 
-	if (Session::getInstance()->hasGame()) {
+	$session = Session::getInstance();
+	if ($session->hasGame()) {
 		$container = Page::create('current_sector.php');
 		$errorMsg = '<span class="red bold">ERROR: </span>' . $message;
 		$container['errorMsg'] = $errorMsg;
@@ -211,7 +212,7 @@ function handleUserError(string $message): never {
 		$container['message'] = $message;
 	}
 
-	if (USING_AJAX) {
+	if ($session->ajax) {
 		// To avoid the page just not refreshing when an error is encountered
 		// during ajax updates, use javascript to auto-redirect to the
 		// appropriate error page.
@@ -308,7 +309,7 @@ function do_voodoo(): never {
 		define('AJAX_CONTAINER', isset($var['AJAX']) && $var['AJAX'] === true);
 	}
 
-	if (!AJAX_CONTAINER && USING_AJAX && $session->hasChangedSN()) {
+	if (!AJAX_CONTAINER && $session->ajax && $session->hasChangedSN()) {
 		exit;
 	}
 	//ob_clean();
@@ -322,7 +323,7 @@ function do_voodoo(): never {
 		$player = $session->getPlayer();
 		$sectorID = $player->getSectorID();
 
-		if (!USING_AJAX //AJAX should never do anything that requires a lock.
+		if (!$session->ajax //AJAX should never do anything that requires a lock.
 			//&& ($var->file == 'current_sector.php' || $var->file == 'map_local.php') //Neither should CS or LM and they gets loaded a lot so should reduce lag issues with big groups.
 		) {
 			// We skip locking if we've already failed to display error page
@@ -358,7 +359,7 @@ function do_voodoo(): never {
 		$player->updateTurns();
 
 		// Check if we need to redirect to a different page
-		if (!$var->skipRedirect && !USING_AJAX) {
+		if (!$var->skipRedirect && !$session->ajax) {
 			if ($player->getGame()->hasEnded()) {
 				Page::create('game_leave_processing.php', ['forward_to' => 'game_play.php', 'errorMsg' => 'The game has ended.'], skipRedirect: true)->go();
 			}
@@ -408,7 +409,7 @@ function do_voodoo(): never {
 	}
 	$template->assign('AJAX_ENABLE_REFRESH', $ajaxRefresh);
 
-	$template->display('skeleton.php', USING_AJAX || AJAX_CONTAINER);
+	$template->display('skeleton.php', $session->ajax || AJAX_CONTAINER);
 
 	$session->update();
 

diff --git a/src/lib/Smr/Session.php b/src/lib/Smr/Session.php
@@ -33,6 +33,7 @@ class Session {
 	private array $links = [];
 	private ?Page $currentPage = null;
 	private bool $generate;
+	public readonly bool $ajax;
 	private string $SN;
 	private string $lastSN;
 	private int $accountID;
@@ -82,10 +83,11 @@ public function __construct() {
 		$this->db->write('DELETE FROM active_session WHERE last_accessed < ' . $this->db->escapeNumber(time() - self::TIME_BEFORE_EXPIRY));
 
 		// try to get current session
+		$this->ajax = Request::getInt('ajax', 0) === 1;
 		$this->SN = Request::get('sn', '');
 		$this->fetchVarInfo();
 
-		if (!USING_AJAX && $this->hasCurrentVar()) {
+		if (!$this->ajax && $this->hasCurrentVar()) {
 			$file = $this->getCurrentVar()->file;
 			$loadDelay = self::URL_LOAD_DELAY[$file] ?? 0;
 			$timeBetweenLoads = microtime(true) - $this->lastAccessed;
@@ -119,7 +121,7 @@ public function fetchVarInfo(): void {
 
 			[$this->links, $lastPage] = $dbRecord->getObject('session_var', true);
 
-			$ajaxRefresh = USING_AJAX && !$this->hasChangedSN();
+			$ajaxRefresh = $this->ajax && !$this->hasChangedSN();
 			if ($ajaxRefresh) {
 				$this->currentPage = $lastPage;
 			} elseif (isset($this->links[$this->SN])) {
@@ -146,9 +148,9 @@ public function fetchVarInfo(): void {
 	public function update(): void {
 		$sessionVar = [$this->links, $this->currentPage];
 		if (!$this->generate) {
-			$this->db->write('UPDATE active_session SET account_id=' . $this->db->escapeNumber($this->accountID) . ',game_id=' . $this->db->escapeNumber($this->gameID) . (!USING_AJAX ? ',last_accessed=' . $this->db->escapeNumber(Epoch::microtime()) : '') . ',session_var=' . $this->db->escapeObject($sessionVar, true) .
+			$this->db->write('UPDATE active_session SET account_id=' . $this->db->escapeNumber($this->accountID) . ',game_id=' . $this->db->escapeNumber($this->gameID) . (!$this->ajax ? ',last_accessed=' . $this->db->escapeNumber(Epoch::microtime()) : '') . ',session_var=' . $this->db->escapeObject($sessionVar, true) .
 					',last_sn=' . $this->db->escapeString($this->SN) .
-					' WHERE session_id=' . $this->db->escapeString($this->sessionID) . (USING_AJAX ? ' AND last_sn=' . $this->db->escapeString($this->lastSN) : ''));
+					' WHERE session_id=' . $this->db->escapeString($this->sessionID) . ($this->ajax ? ' AND last_sn=' . $this->db->escapeString($this->lastSN) : ''));
 		} else {
 			$this->db->write('DELETE FROM active_session WHERE account_id = ' . $this->db->escapeNumber($this->accountID) . ' AND game_id = ' . $this->db->escapeNumber($this->gameID));
 			$this->db->insert('active_session', [

diff --git a/test/SmrTest/lib/DefaultGame/SessionIntegrationTest.php b/test/SmrTest/lib/DefaultGame/SessionIntegrationTest.php
@@ -89,6 +89,17 @@ public function test_getSessionID(): void {
 		self::assertNotEquals($sessionID, $session->getSessionID());
 	}
 
+	public function test_ajax(): void {
+		// If $_REQUEST is empty, ajax is false
+		self::assertFalse($this->session->ajax);
+
+		// Test other values in $_REQUEST
+		$_REQUEST['ajax'] = 1;
+		self::assertTrue(DiContainer::make(Session::class)->ajax);
+		$_REQUEST['ajax'] = 'anything other than 1';
+		self::assertFalse(DiContainer::make(Session::class)->ajax);
+	}
+
 	public function test_current_var(): void {
 		// With an empty session, there should be no current var
 		self::assertFalse($this->session->hasCurrentVar());
@@ -125,6 +136,7 @@ public function test_current_var(): void {
 		// we should still get the updated var, even though it wasn't
 		// the one originally associated with this SN.
 		$session->update();
+		$_REQUEST['ajax'] = 1;
 		$session = DiContainer::make(Session::class);
 		self::assertEquals($var2, $session->getCurrentVar());