Performance problems with NITFBylineViewlet when having many users (specially when using LDAP)

[idgserpro]

When rendering NITF default view, this code is called:

    def getMemberInfoByName(self, fullname):
        membership = api.portal.get_tool('portal_membership')
        members = membership.searchForMembers(name=fullname)
        if members:
            member = members[0].getUserId()  # we care only about the first
            return membership.getMemberInfo(member)

searchForMembers in MembershipTool calls listMembers, listing all users and iterating through all of them. This causes a huge performance problem when using LDAP.

I want to know the motivation of this snippet so we can try to give a solution. Is this snippet to guarantee that a name inserted in "byline" field has a related userid in acl_users? Why we can't ust accept the string that was inserted in the field, is this from NITF standards?

members[0] isn't a good idea as well. You can have similar names and it validates just the first one found.

[hvelarde]

the problem was a little bit different: getMemberInfoByName was called 3 times per view rendering; I refactored the code a little bit and added a memoizer to avoid that.

please review the fix and merge it in case your test results are satisfactory.

BTW, I was reviewing searchForMembers and it doesn't call listMembers at all as you can see on it's code.

[idgserpro]

Thanks for the refactoring.

What exactly is the motivation behing this validation? Why not just accept the string in the field and render it on the template?

[hvelarde]

@idgserpro because the byline of a news article could be different from its creator; the byline field defines the name of the person who wrote the piece; the creator field is set with the id of the person who upload it into the CMS.

if there is a user matching the name in the byline, then we assume that person is the same and we show the link to her personal page as usually; if not, we just show the name without link.

please accept your membership in the Collective to be able to merge the pull request.

[idgserpro]

Sorry about the whole confusion. We have lots of collective.nitf customizations in brasil.gov.portal, and in there the behavior isn't to create a link to the user if it exists in acl_users, it just hides the byline. It makes sense to create a link to the author in the site like you're doing now.

I'll take a look to make the merge as soon as possible.

I have a small question though about a minor issue: suppose I add "John Doe" in byline and "John Doe Bond" in control panel. The name shown on NITF content view is going to be "John Doe" and not "John Doe Bond". That's the intended behavior, right?

[hvelarde]

yes, John Doe, but you can add a test for it, if you want :beer:

one of the main enhancements of version 2.0 is that we completely removed dependency on five.grok; so we have no more crazy magic to override the template.

just plain ZCML, clean and painless.

[idgserpro]

I still think members[0] isn't the best idea but I don't have a better solution. :(

You can have

john.doe (John Doe) john (John) john.bond (John Doe Bond)

And, in Author, just John, and in this scenario I can't be sure which of of the authors is the correct one but I just get the first one found.

I know this is a really strange and improbable scenario, but it can happen nonetheless. But if we can't find a better solution we can just close this issue after the pull is merged.

[hvelarde]

let's not solve nonexistent problems; I'll change the caching so we can move on with this.