612051 - Migrate update.php to zamboni

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect, P1)

Description

[Wil Clouser [:clouserw]]

update.php[1] is what Firefox asks to see if there are add-on updates available.  We need to migrate it to python.  It's in this milestone because we're getting rid of the sandbox, so we either need to migrate this script or tweak on the PHP.

Our new statuses are in attachment 487064 [details].  As I understand it, any status except for INCOMPLETE or DISABLED should get updates (ccing fligtar for confirmation).  If a user is on a beta channel, they should only get updates to a new version on that beta channel.

The code in [1] should answer questions about how the update system works right now.  It's not tough, only a few hundred lines of PHP, but there are some caveats:

- This is the highest traffic script on AMO by orders of magnitude.  Code here needs to be fast and the output needs to be cachable.
- This needs to have lots of tests - overlapping coverage is good.  If we update people to the wrong versions, that can break things more permanently than we want.
- The stuff about $detect_installed can be ignored.  We used that when we had a facebook app, but don't need it anymore.
- The $debug stuff isn't important to me either, unless it helps you write it

As usual, ask questions if you have them.  I'm happy to help if I can.

[1] http://viewvc.svn.mozilla.org/vc/addons/trunk/site/app/webroot/services/update.php?view=markup

Comment 1

[Justin Scott [:fligtar]]

Couple notes from me:
* All of the Facebook cookie stuff can be removed. That includes anything that mentions AMOfbUser and lines 281 - 300

* Right now we return the latest version of the add-on compatible with the application give, regardless of version compatibility. We used to check version compatibility but removed it due to problems sorting versions with SQL. However, we can now add this back as last year we added a way to properly sort version numbers via SQL by converting them to an integer format. We should use this to return the latest add-on version compatible with the application version passed to us.

Comment 2

[Andy McKay]

We'll need to keep the URL the same I take it, should we be doing that in the
Apache side or should I make the URL the same in Django?

Comment 3

[Wil Clouser [:clouserw]]

(In reply to comment #2)
> We'll need to keep the URL the same I take it, should we be doing that in the
> Apache side or should I make the URL the same in Django?

We can use /services/update in Django and 301 there in apache.

Comment 4

[Justin Scott [:fligtar]]

update.php isn't actually called itself and is instead a redirect from the previous name of the file, VersionCheck.php.

Here's what Firefox hits:

https://versioncheck.addons.mozilla.org/update/VersionCheck.php?reqVersion=%REQ_VERSION%&id=%ITEM_ID%&version=%ITEM_VERSION%&maxAppVersion=%ITEM_MAXAPPVERSION%&status=%ITEM_STATUS%&appID=%APP_ID%&appVersion=%APP_VERSION%&appOS=%APP_OS%&appABI=%APP_ABI%&locale=%APP_LOCALE%&currentAppVersion=%CURRENT_APP_VERSION%&updateType=%UPDATE_TYPE%

Comment 5

[Andy McKay]

The version integer isn't in SQL, it's calculated in Python. We can either a)
add this into the database (migration and management command) or b) do this by
grabbing the relevant versions then sorting in Python.

b) is easier, but could be slower with a large number of versions.

Comment 6

[Wil Clouser [:clouserw]]

A lot of versions is not unusual and will only grow over time.  Do what is best!

mysql> select addon_id, count(*) as c from versions group by addon_id order by c desc limit 50;
+----------+-----+
| addon_id | c   |
+----------+-----+
|      722 | 261 | 
|      220 | 157 | 
|     4925 | 129 | 
|     5890 | 100 | 
|     4536 |  87 | 
|    13491 |  87 | 
|     4490 |  87 | 
|     3576 |  87 | 
|     1379 |  86 | 
|     2324 |  84 | 
|     4908 |  82 | 
|      166 |  80 | 
|    10828 |  80 | 
|      123 |  79 | 
|    14920 |  79 | 
|      122 |  78 | 
|     8680 |  78 | 
|      307 |  75 | 
|      421 |  73 | 
|     9409 |  71 | 
|     5373 |  70 | 
|     1730 |  65 | 
|      427 |  65 | 
|     5276 |  65 | 
|     8446 |  65 | 
|     4129 |  64 | 
|     1833 |  64 | 
|     3209 |  63 | 
|      743 |  63 | 
|    11164 |  63 | 
|      446 |  61 | 
|    12105 |  61 | 
|     2464 |  61 | 
|     9825 |  60 | 
|      424 |  60 | 
|    12022 |  60 | 
|     7004 |  60 | 
|     5791 |  60 | 
|      725 |  60 | 
|     3863 |  59 | 
|     1368 |  58 | 
|      816 |  58 | 
|      189 |  58 | 
|     5593 |  58 | 
|    11783 |  58 | 
|    10229 |  57 | 
|     3006 |  57 | 
|     3036 |  57 | 
|     6357 |  57 | 
|    49815 |  56 | 
+----------+-----+
50 rows in set (0.03 sec)

I can help load test if you need it.

Comment 7

[Andy McKay]

Just need to use version_int that's on appversions to complete this. That's already there.

Comment 8

[Andy McKay]

There's feedback that I'd like to integrate, and I'm sure you'll find issues which will need more tests.

It's at: /update

For example: /update?appVersion=3.0pre&version=2.0.58&id={2fa4ed95-0317-4c6a-a74c-5f3e3912c1f9}&reqVersion=1&appID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}

https://github.com/jbalogh/zamboni/commit/9c5b3716640c1b41948895c1687c94620c1bdc24