Games gallery makes a request for each game

Bug #787403 reported by Matt Giuca
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MUGLE
Triaged
Low
Unassigned

Bug Description

Unfortunately, as of fixing bug #787368, the games gallery necessarily needs to make a server request for each game -- it needs the game's devteam to construct the URL. This is pretty bad; find a way around it.

Possibly set a custom field in Game (by getGames) which contains the full DevTeam object. Or a method in DevTeamService which takes a list of games and gets their devteams.

Tags: performance
Revision history for this message
Prageeth Silva (prageethsilva) wrote :

The easiest way I can think of is, simply like how ActiveVersion is populated in the GameServiceImpl, the DevTeam can also be populated in a custom field in the wrapper. When adding a new field to a wrapper class (ModelClientClass), there are NO rules so you don't have to worry about breaking the ModelWrapper.

However, I can do it when I get home in a couple of hours, if you want.

Revision history for this message
Matt Giuca (mgiuca) wrote :

This bug also applies to the MyGames view.

Revision history for this message
Matt Giuca (mgiuca) wrote :

Prageeth: Note that the above approach for the activeVersion caused a lot of trouble and needs to be refactored (see bug #788075). Please don't do the same for devTeam (all the code that assumes 'devTeam' is a key will break, because it will now be an object). You can do something similar, but make a SEPARATE field for "devTeam as an object" and leave the current field devTeam as the key.

Revision history for this message
Prageeth Silva (prageethsilva) wrote : Re: [Bug 787403] Re: Games gallery makes a request for each game

On Thu, May 26, 2011 at 11:01 AM, Matt Giuca <email address hidden>wrote:

> Prageeth: Note that the above approach for the activeVersion caused a
> lot of trouble and needs to be refactored (see bug #788075). Please
> don't do the same for devTeam (all the code that assumes 'devTeam' is a
> key will break, because it will now be an object). You can do something
> similar, but make a SEPARATE field for "devTeam as an object" and leave
> the current field devTeam as the key.

OK, but I think the reason we switched from ObjectType to Keys were because
Persistent Objects were not allowed to have multiple parents.
I'm not sure if the same approach would apply for devTeams. But I'll have a
look when I get a chance during the weekend.

--
*Prageeth Silva*

Revision history for this message
Matt Giuca (mgiuca) wrote :

No.. I'm not saying we shouldn't have keys instead of objects. I'm not talking about the datastore classes at all; just the client classes.

Basically, here's what we did for the Game (client) class. It was:
class Game
{
    Long activeVersion;
}

it is now:
class Game
{
    GameVersion activeVersion;
}

(Note that the database class is still a Key.)

It *should have been*:

class Game
{
    Long activeVersion;
    GameVersion activeVersionObject;
}

That would solve the many problems where things expect an activeVersion to be a key but it isn't. For example the fact that it is set to null when it is read, and needs to be non-null when it is written -- again, see bug #788075.

I am saying a) we need to fix this, and b) do not make the same mistake with devTeam. DO NOT DO this:

class Game
{
    DevTeam devTeam;
}

Instead, do this:

class Game
{
    Long devTeam;
    DevTeam devTeamObject;
}

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.