Details
-
Improvement
-
Status: Open
-
Minor
-
Resolution: Unresolved
-
1.1.3.RELEASE, 1.1.4.RELEASE
-
None
-
None
Description
When having a @OneToMany field with GOOGLE_APP_ENGINE, Spring ROO generates code that does N queries instead of just 1. Something like this should only be done for a @ManyToMany relationship or an unowned @OneToMany
// Spring Roo 1.1.4.BUILD-SNAPSHOT [rev c6e48eb] log opened at 2011-04-15 18:13:20 project --topLevelPackage com.test persistence setup --provider DATANUCLEUS --database GOOGLE_APP_ENGINE entity --class ~.User --serializable --permitReservedWords field string --fieldName name --notNull json all controller scaffold --class ~.web.UserController --entity ~.User entity --class ~.UserRole --serializable field string --fieldName name --notNull field reference --fieldName user --type ~.User --cardinality MANY_TO_ONE --permitReservedWords controller scaffold --class ~.web.UserRoleController focus --class ~.User field set --fieldName roles --type ~.UserRole --cardinality ONE_TO_MANY --mappedBy user quit // Spring Roo 1.1.4.BUILD-SNAPSHOT [rev c6e48eb] log closed at 2011-04-15 18:14:49
Generates a User class:
@RooJavaBean @RooToString @RooEntity @RooSerializable @RooJson public class User { @NotNull private String name; @OneToMany(cascade = CascadeType.ALL, mappedBy = "user") private Set<UserRole> roles = new HashSet<UserRole>(); }
which generates User_Roo_JavaBean.aj that contains the getter and setter for UserRoles:
public Set<UserRole> User.getRoles() { Set<com.test.UserRole> localRoles = new HashSet<com.test.UserRole>(); for (Key key : rolesKeys) { com.test.UserRole entity = UserRole.findUserRole(key.getId()); if (entity != null) { localRoles.add(entity); } } this.roles = localRoles; return localRoles; } public void User.setRoles(Set<UserRole> roles) { Set<com.test.UserRole> localRoles = new HashSet<com.test.UserRole>(); List<Long> longIds = new ArrayList<Long>(); for (Key key : rolesKeys) { if (!longIds.contains(key.getId())) { longIds.add(key.getId()); } } for (UserRole entity : roles) { if (!longIds.contains(entity.getId())) { longIds.add(entity.getId()); rolesKeys.add(KeyFactory.createKey(UserRole.class.getName(), entity.getId())); } localRoles.add(entity); } this.roles = localRoles; }
Since we are talking about a @OneToMany relationship here and every UserRole class has a reference to the User class. Why not just generate a findByUser finder on the UserRole class and user that? That way we are doing 1 query instead of N.
Naturally you will need to check to make sure that the @ManyToOne reference exists prior to optimizing this code.
Furthermore, the above code could be optimized such that if the getter is called more than once, you don't get M * N queries. Since we are maintaining a transient set, it should be utilized as a cache.
Otherwise, I don't understand what the point of this.roles = localRoles; is in both methods. You are setting the transient field, but it is NEVER utilized.
