Spring Roo
  1. Spring Roo
  2. ROO-2514

Controllers shouldn't call findAll on each request

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 1.2.0.RC1
    • Fix Version/s: 1.2.0.RELEASE
    • Component/s: WEB MVC
    • Labels:
      None

      Description

      As explained in the forum thread, and using the Pet entity as an example, the controller code generated by Roo calls findAllPets() upon every request, thanks to the presence of this method:

      PetController_Roo_Controller.aj
      @ModelAttribute("pets")
      public Collection<Pet> PetController.populatePets() {
          return Pet.findAllPets();
      }

      This is wasteful given that:

      • Some of the controller's request handling methods (e.g. "show" and "delete") don't even use the returned list of Pets.
      • The "list" method obtains its own list of Pets (usually via findPetEntries()).
      • The "create*" and "update*" methods only need the list of all Pets if the Pet class has a Pet (or Pets) field.

      We can make the scaffolded controllers more performant by only obtaining the list of all Pets when it's actually required.

      In the meantime, the workaround is as follows:

      1. Push in the populatePets method and have it return null (saves time in the "list" method)
      2. If the entity has a field of its own type (or a collection thereof), push in the "create*" and "update*" methods and modify them to explicitly put the "pets" list into the model, for example (showing the create* methods):
      PetController.java
      @RequestMapping(params = "form", method = RequestMethod.GET)
      public String createForm(Model uiModel) {
          uiModel.addAttribute("pets", Pet.findAllPets());
          uiModel.addAttribute("pet", new Pet());
          return "pets/create";
      }
      
      @RequestMapping(method = RequestMethod.POST)
      public String create(@Valid Pet pet, BindingResult bindingResult, Model uiModel, HttpServletRequest httpServletRequest) {
          if (bindingResult.hasErrors()) {
              uiModel.addAttribute("pets", Pet.findAllPets());
              uiModel.addAttribute("pet", pet);
              return "pets/create";
          }
          uiModel.asMap().clear();
          pet.persist();
          return "redirect:/pets/" + encodeUrlPathSegment(pet.getId().toString(), httpServletRequest);
      }

        Activity

        We have experienced serious performance problems on enterprise databases with tables of 100,000 records.

        This solution solves the problem for show, delete and list. However, to create and update the problem will remain.

        A more general solution would be to use a rich select (eg dijit.form.FilteringSelect) obtaining related entity list by JSON filtered in some way to avoid the initial charge of all records.

        Show
        Mario Martínez Sánchez - gvNIX - DiSiD Technologies S.L. added a comment - We have experienced serious performance problems on enterprise databases with tables of 100,000 records. This solution solves the problem for show, delete and list. However, to create and update the problem will remain. A more general solution would be to use a rich select (eg dijit.form.FilteringSelect) obtaining related entity list by JSON filtered in some way to avoid the initial charge of all records.
        Hide
        Michael Oliver added a comment -

        Some sort of flag in the request is needed to trigger model attributes. A combo box with a dynamic list of options for example would only need that model attribute set for the create and update forms. A control option in Roo for a combo box might set the flag so the model attribute is set when that form with that combo box control is displayed and no place else.

        Show
        Michael Oliver added a comment - Some sort of flag in the request is needed to trigger model attributes. A combo box with a dynamic list of options for example would only need that model attribute set for the create and update forms. A control option in Roo for a combo box might set the flag so the model attribute is set when that form with that combo box control is displayed and no place else.
        Hide
        VIDREQUIN added a comment -

        We also have the same problem here, whith thousands of SQL request when just printing on drop down list ...

        Show
        VIDREQUIN added a comment - We also have the same problem here, whith thousands of SQL request when just printing on drop down list ...
        Hide
        Andrew Swan added a comment - - edited

        Git commit 42c04adf8f84552a77c8507420c77aff46504500 changes the "populateXxx" methods to return null for any request that is not a "show form" request. In other words, these CRUD operations will no longer cause a findAll query for any domain type:

        • delete
        • show
        • submit create form
        • submit update form
        Show
        Andrew Swan added a comment - - edited Git commit 42c04adf8f84552a77c8507420c77aff46504500 changes the "populateXxx" methods to return null for any request that is not a "show form" request. In other words, these CRUD operations will no longer cause a findAll query for any domain type: delete show submit create form submit update form
        Hide
        Andrew Swan added a comment -

        Git commit 62e38aa90069ec2cd07d949822d92e1cd17ef664 further reduces the number of "findAll" queries by suppressing the creation of "populateXxx" methods for domain types that are not persistent fields of the form backing type (either as a single value or a collection).

        This completes the changes for 1.2.0. If further improvements are required (e.g. retrieving pages of related entities on demand, as suggested above), please log a new ticket that can be milestoned for a later release.

        Show
        Andrew Swan added a comment - Git commit 62e38aa90069ec2cd07d949822d92e1cd17ef664 further reduces the number of " findAll " queries by suppressing the creation of " populateXxx " methods for domain types that are not persistent fields of the form backing type (either as a single value or a collection). This completes the changes for 1.2.0. If further improvements are required (e.g. retrieving pages of related entities on demand, as suggested above), please log a new ticket that can be milestoned for a later release.
        Hide
        Andrew Swan added a comment -

        Reopening because further testing reveals that the related objects are not put into the model when the form is redisplayed following a validation error.

        Show
        Andrew Swan added a comment - Reopening because further testing reveals that the related objects are not put into the model when the form is redisplayed following a validation error.
        Hide
        Andrew Swan added a comment -

        Fixed the above problem in Git commit 95443d459cca1a6ae80f0288691067b23c669436. The controller ITDs no longer have methods annotated with @ModelAttribute, as these don't provide enough control over when they are invoked (e.g. for a POST/PUT request, only if there is a validation error). Instead, the controllers now have a populateEditForm method that apart from addressing the original purpose of this ticket also eliminates the duplication that previously existed between the scaffolded create, createForm, update, and updateForm methods, specifically in how they populated the UI model. The main benefit of removing this duplication is that if the user wants to customise the UI model (e.g. how the related objects are retrieved), they only need to push in the populateEditForm method instead of the four form-related methods listed above.

        Show
        Andrew Swan added a comment - Fixed the above problem in Git commit 95443d459cca1a6ae80f0288691067b23c669436 . The controller ITDs no longer have methods annotated with @ModelAttribute , as these don't provide enough control over when they are invoked (e.g. for a POST/PUT request, only if there is a validation error). Instead, the controllers now have a populateEditForm method that apart from addressing the original purpose of this ticket also eliminates the duplication that previously existed between the scaffolded create , createForm , update , and updateForm methods, specifically in how they populated the UI model. The main benefit of removing this duplication is that if the user wants to customise the UI model (e.g. how the related objects are retrieved), they only need to push in the populateEditForm method instead of the four form-related methods listed above.

          People

          • Assignee:
            Andrew Swan
            Reporter:
            Andrew Swan
          • Votes:
            14 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: