Denormalize release info a bit and store latest and recommended releases in {project_release_supported_versions}
| Project: | Project |
| Version: | 6.x-1.x-dev |
| Component: | Releases |
| Category: | task |
| Priority: | critical |
| Assigned: | dww |
| Status: | closed |
Instead of trying to compute this crap all the time with big nasty queries, especially since it only possibly changes when releases are edited, added or deleted, we should just compute it when releases change, and save the nids of the "latest" (of all releases) and "recommended" (highest releases without "extra") for every given branch (API compatibility tid + major version pair).
Not only is this going to significantly easy the DB load on d.o, it's going to make some really nice views support for releases possible. For example, we'll be able to construct the download table via a view, which is *way* better for customization, (and will let us rip a lot of nasty code out of project_release.module). Plus, it's going to let us fix #176776-41: GHOP #55: Make release summary table UI look more like update(_status)? report...
Patch coming in a moment...

#1
Initial patch that does the denormalization, including schema update and changes to the code to keep these values accurate. This doesn't yet touch the views support, nor replace the download table (I think I'd like to handle those in a separate issue/patch), but it's an absolute prerequisite to any of that. Tested lightly on my local install. Needs more testing for edge cases, and the DB update should be tested on d6.d.o to see how it handles a huge amount of data.
#2
After some extensive testing, found a rather nasty bug in the previous patch. I'm testing all this in an OG + Project environment. Whenever project_release_check_supported_versions() was called by my packaging script, it was finding no releases, even if it had just published them. I realized this was do to db_rewrite_sql() and the fact that my packaging script is effectively running as anonymous, and therefore, had no access to any projects. ;)
So, there's now a flag to project_release_find_latest_releases() to specify if you want access checks or not. As I wrote in the PHPDoc about it:
* Optional boolean to indicate if node access checks should be enforced.
* Defaults to FALSE since the caller might not actually have access to all
* the releases or projects. However, this function usually has to compute
* the accurate values regardless of access, and consumers of this data are
* responsible for ensuring access.
Since we're populating a site-wide table, {project_release_supported_versions}, and there really is only ever one latest and recommended release, I figure it's better for this code to always operate without access checks (though still checking if the nodes are published). Anyone consuming this data (views, etc) needs to ensure that a given user has permission to view the nodes in question. However, generally, if a user has permission to access any releases for a project, they're going to have access to all of them, so I don't think this is a fundamental flaw in the design. Worst case is you get to create a really expensive view that recomputes all this crap each time if you really need access checking for each different user.
I also slightly renamed some functions and added some more PHPDoc to hopefully make things a bit more clear...
I've now tested this heavily, and I'm sure it's working. The only thing remaining for this issue would be giving the DB update a spin on d6.d.o to make sure it doesn't blow up, but I'm pretty confident everything's going to be fine there.
#3
Based on testing this on d6.d.o, I found a few issues with the previous. In case of faulty data, the update could hang forever, since it'd keep finding the same row with latest_release as 0, and keep trying that indefinitely. So now, project_release_check_supported_versions() returns a bool to indicate if it did anything. The DB update starts out with these two new columns as NULL. The query to find the next row to work on checks if
latest_release IS NULL. If project_release_check_supported_versions() returns FALSE, we update that row to set latest_release and recommended_release to 0 (just so we know we visited it). That way, we ensure the update completes, even if it hits branches without any published releases, etc.This final patch ran fine (a few times) on d6.d.o, which is certainly the largest and nastiest project* dataset on Earth. Pretty happy with this. Any final objections from anyone before I commit?
#4
Assuming no one's really going to review this, and since I tested heavily locally and on d6.d.o, I just committed to HEAD. Marking needs work for d.o deployment, since we'll need to make sure we run the DB update when we update the code...
#5
There was a minor bug in the previous patch. Fixed: http://drupal.org/cvs?commit=247392
#6
Deployed.
#7
Automatically closed -- issue fixed for 2 weeks with no activity.