|
[
Permlink
| « Hide
]
Charles Hedrick added a comment - 15-Aug-2008 10:40
Are you sure about this? StructuredArtifactFInder ends up calling contenthostingservice.findResources(null, null, null). That calls getFlatResources on each site that the user has access to. When called with nulls, that just access checks the files. getFlatResources has already done that, so we have 2 access checks. (That can be fixed by calling filterArtifacts with the large argument false.) But as far as I can see it does not check every form of that type on the system, only in sites that the user has access to.
getResourcesOfType doesn't seem to be called. Instead findByOwnerAndType is used. By using Owner, the code is able to restrict access to sites to which the owner has access. I believe this is a result of patches which you may not have.
Portfolio performance issues when selecting items for templates that have many forms as item defs http://jira.sakaiproject.org/jira/browse/SAK-13791 Slow performance in wizards and viewing portfolios that use matrices http://jira.sakaiproject.org/jira/browse/SAK-13804 I now believe that the right thing to do is to show only forms and files in the user's worksite. This isn't just an efficiency issue. Rather, if you let users choose things from other sites, their portfolio will break if the other sites are removed or changed. We expect portfolios to have a long life: we will encourage freshmen to start them, and may support them for alumni in some disciplines. Course and other sites tend to have much shorter lifetimes. Thus if a user wants to include a form or file from another site, we'd like them to copy it to their worksite and include it from there. I have removed the code from 14165, going back to the version in
During today's call, we discussed Rutgers solution to
The patch developed by Rutgers restricts this search for potential content to MyWorkspace if a sakai.property is set. Its reasonable to assume that most content would be created in the process of completed a matrix or wizard and would end up in the "Portfolio Interaction" folder in the student's MyWorkspace. However, the UI still allows for the creation of "form items" right through Resources (assuming that the form is not hidden). I recommended that an additional sakai.property that turned off the ability to author form items in sites other than MyWorkspace would be a reasonable compliment to this feature (preventing students form authoring content that they would never be able to use). Another approach would be to use the Rutgers approach as an initial page load to populate the pick lists with content from MyWorkspace, with an option to search for "all" content (as it does right now) for those that need it. The attached patches resolve the majority of the performance issue. They do not change the behavior in that only items owned by the current user within accessible sites are listed in the portfolio assembly screen. Both the content and metaobj patches are required, as there is a signature/implementation change in ContentHostingService.
These patches are against a lightly modified 2.5.2. They should apply quite cleanly to 2.5.x trees. Without significant refactoring of how resources are stored, located, and checked for permissions, this patch is approximately as efficient as possible. It requires a small change to the API of ContentHostingService and came too late, so it will not be included for Kernel 1 / 2.6. It is, however, easy to apply the changes to a custom post-K1 build.
Re-opening until the attached patch is applied to subversion
I'm attaching an upgraded patch. It avoids multiple round-trips in getResourcesOfType, which previously happened for each "item definition" in presentation templates. This should yield significant improvements when editing the contents of a templated portfolio.
I've checked in changes that uses the intent of both patches, but also leverages the new ContentHostingService.getContextResourcesOfType() method which should be a lot faster than what currently exists. This new CHS method will retrieve forms only in sites the user is a member -- and this logic is executed within the SQL Select statement (not in the java code). It is likely that the only forms returned will be those in the user's My Workspace, given current usage patterns. This smaller list is then filtered to include only those forms of the requested type and owner.
QA Instructions: Modified code can be exercised in two places: 1) Portfolio -> Add/Edit Content (displaying this page, as long as there are forms to select from, will exercise the new CHS method) 2) Forms -> Edit -> Select Schema File -> Select -> Continue -> Save Changes (this exercises related re-factored code) Michigan will be testing this in production in the Fall -- once verified & closed, this should be merged to 2.6.x
That looks right. I'm concerned about any search that is proportional to either the number of users or of sites. We need something that is roughly constant in the size of the system.
FYI: Rutgers is installing the June 10, 2009 patch. We'll try them for the next month and decide whether to keep them in the fall.
Michigan has been running this in production 2.6.x for a month with no problems. Performance is significantly improved.
Looks like something is missing in the r63496 commit for a merge into 2.6.x:
[INFO] Compilation failure /home/leveque/sakai/source/sakai_2-6-x_full_build/metaobj/metaobj-impl/api-impl/src/java/org/sakaiproject/metaobj/shared/mgt/impl/WrappedStructuredArtifactFinder.java:[96,33] cannot find symbol symbol : method getContextResourcesOfType(java.lang.String,java.util.Set) location: interface org.sakaiproject.content.api.ContentHostingService /home/leveque/sakai/source/sakai_2-6-x_full_build/metaobj/metaobj-impl/api-impl/src/java/org/sakaiproject/metaobj/shared/mgt/impl/WrappedStructuredArtifactFinder.java:[96,33] cannot find symbol symbol : method getContextResourcesOfType(java.lang.String,java.util.Set) location: interface org.sakaiproject.content.api.ContentHostingService This issue cannot be merged to 2.6.x without a new kernel release that includes
I currently marked the merge as a "Won't fix" as it depends on the merge of the kernel issue, and the fix version for
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||