Session: modify how pages are handled internally #1473
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The purpose of this is to make the management of valid page links more straightforward and to better separate the concepts of page links and the current page. It should also fix the issue of the number of links in the session growing linearly with the number of pages loaded during the session (in some common cases, such as refreshing Current Sector). There are still some edge cases that may not work as intended yet. For example, if we click an ajax link (e.g. updating sector connections) and that SN uses the same page data as the current page, then it will be incorrectly identified by the Session as an ajax auto-refresh, and won't actually load the desired page. In the future, we may want to change the query parameters so that we can explicitly identify when an auto-refresh request is made. Three main aspects of this change: 1. Replace CommonID hashing with direct page equality comparison. We use loose object equality comparison with the page being added. This should be plenty fast, because in most cases any two pages will share no common properties and will return early from the equality check. It also avoids the need to constantly recompute the common ID of each page, which may even be more expensive (it has to serialize and hash every Page object and thus has no early return mechanism). 2. Save the current page separately from the stored page links. (This allows us to reuse it if we detect an auto-refresh.) Since we won't modify links associated with an SN when the current page is modified (e.g. by adding a request variable to the page via Session::getRequestVar), we ensure that when we try to add reusable pages that were already added, they won't get a new SN. It also fixes a memory leak of sorts, because clicking on certain reusable pages would *always* create a new SN and so the session var would just get larger and larger (`current_sector.php` was the most prominent page affected by this). 3. Convert remaining page loads from a dynamic int to a constant bool. (We were basically using the int as a bool, and having it be a constant allows us not to care when a page was created.) Left panel links should all be reusable, so include them in the "always available" list. The following concepts were tested manually for proper behavior: * Assigning GP Editor * Plot to Nearest * Custom display range in Rankings * Making multiple chess moves * Editing sector links * Toggling ship weapon display
Move the database query to mark messages as read into the SmrPlayer method `setMessagesRead`. This is helpful because it consolidates the queries that should not be performed on ajax updates.
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.
Codecov ReportBase: 26.63% // Head: 26.67% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1473 +/- ##
============================================
+ Coverage 26.63% 26.67% +0.03%
+ Complexity 4019 4017 -2
============================================
Files 120 120
Lines 11690 11682 -8
============================================
+ Hits 3114 3116 +2
+ Misses 8576 8566 -10
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.