Skip to content

Commit

Permalink
fix: [CO-1249] Better handling of GalGroup type matches in FullAutoCo…
Browse files Browse the repository at this point in the history 
…mplete API (#532)

* fix: [CO-1249] better handling of GalGroup type matches

by adding null checks on returned AutoCompleteMatch objects + null check on requestedAccount before accessing it

* chore: improve builder

* chore: add email attribute to supported entries
  • Loading branch information
@keshavbhatt
keshavbhatt authored Jun 28, 2024
1 parent 895a073 commit 818b57d
Show file tree
Hide file tree
Showing 2 changed files with 192 additions and 58 deletions.
140 changes: 82 additions & 58 deletions store/src/main/java/com/zimbra/cs/service/mail/FullAutoComplete.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@
*/
public class FullAutoComplete extends MailDocumentHandler {

/**
* Helper method that returns if the {@link AutoCompleteMatch} is a Contact group
*
* @return true if the {@link AutoCompleteMatch} is a Contact group false otherwise
*/
private static Boolean isContactGroup(AutoCompleteMatch match) {
return match != null
&& Boolean.TRUE.equals(match.getGroup())
&& ContactEntryType.CONTACT.name().equalsIgnoreCase(match.getMatchType());
}

@Override
public Element handle(Element request, Map<String, Object> context) throws ServiceException {
final var zsc = getZimbraSoapContext(context);
Expand Down Expand Up @@ -118,77 +129,45 @@ private ArrayList<Element> getElementsForMatches(List<AutoCompleteMatch> fullAut
ZimbraSoapContext zsc) {

return fullAutoCompleteMatches.stream().map(match -> {
var matchElement = zsc.createElement(MailConstants.E_MATCH);
matchElement.addAttribute(MailConstants.A_RANKING, Integer.toString(match.getRanking()));
matchElement.addAttribute(MailConstants.A_MATCH_TYPE, match.getMatchType());
matchElement.addAttribute(MailConstants.A_IS_GROUP, match.getGroup());

// for contact group, emails of members will be expanded separately on user request
if (Boolean.FALSE.equals(match.getGroup())) {
matchElement.addAttribute(MailConstants.A_EMAIL, match.getEmail());
}
var matchElementBuilder = new AutoCompleteMatchElementBuilder(zsc);

if (match.getGroup() && match.getCanExpandGroupMembers()) {
matchElement.addAttribute(MailConstants.A_EXP, true);
}
matchElementBuilder.addIntegerAttribute(MailConstants.A_RANKING, match.getRanking(), 0)
.addStringAttribute(MailConstants.A_MATCH_TYPE, match.getMatchType())
.addBooleanAttribute(MailConstants.A_IS_GROUP, match.getGroup(), Boolean.FALSE);

final String id = match.getId();
if (id != null) {
matchElement.addAttribute(MailConstants.A_ID, id);
if (Boolean.FALSE.equals(isContactGroup(match))) {
matchElementBuilder.addStringAttribute(MailConstants.A_EMAIL, match.getEmail());
}

final String folder = match.getFolder();
if (folder != null) {
matchElement.addAttribute(MailConstants.A_FOLDER, folder);
if (Boolean.TRUE.equals(match.getGroup())) {
matchElementBuilder.addBooleanAttribute(MailConstants.A_EXP, match.getCanExpandGroupMembers(), Boolean.FALSE);
}

matchElementBuilder.addStringAttribute(MailConstants.A_ID, match.getId())
.addStringAttribute(MailConstants.A_FOLDER, match.getFolder());

if (Boolean.TRUE.equals(match.getGroup()) || !Objects.equals(match.getMatchType(),
ContactEntryType.GAL.getName())) {
matchElement.addAttribute(MailConstants.A_DISPLAYNAME, match.getDisplayName());
}

final String firstName = match.getFirstName();
if (firstName != null) {
matchElement.addAttribute(MailConstants.A_FIRSTNAME, firstName);
matchElementBuilder.addStringAttribute(MailConstants.A_DISPLAYNAME, match.getDisplayName());
}

final String middleName = match.getMiddleName();
if (middleName != null) {
matchElement.addAttribute(MailConstants.A_MIDDLENAME, middleName);
}

final String lastName = match.getLastName();
if (lastName != null) {
matchElement.addAttribute(MailConstants.A_LASTNAME, lastName);
}

final String fullName = match.getFullName();
if (fullName != null) {
matchElement.addAttribute(MailConstants.A_FULLNAME, fullName);
}
matchElementBuilder.addStringAttribute(MailConstants.A_FIRSTNAME, match.getFirstName())
.addStringAttribute(MailConstants.A_MIDDLENAME, match.getMiddleName())
.addStringAttribute(MailConstants.A_LASTNAME, match.getLastName())
.addStringAttribute(MailConstants.A_FULLNAME, match.getFullName())
.addStringAttribute(MailConstants.A_NICKNAME, match.getNickname())
.addStringAttribute(MailConstants.A_COMPANY, match.getCompany())
.addStringAttribute(MailConstants.A_FILEAS, match.getFileAs());

final String nickname = match.getNickname();
if (nickname != null) {
matchElement.addAttribute(MailConstants.A_NICKNAME, nickname);
}

final String company = match.getCompany();
if (company != null) {
matchElement.addAttribute(MailConstants.A_COMPANY, company);
}

final String fileAs = match.getFileAs();
if (fileAs != null) {
matchElement.addAttribute(MailConstants.A_FILEAS, fileAs);
}
return matchElement;
return matchElementBuilder.build();
}).collect(Collectors.toCollection(ArrayList::new));
}

/**
* Gets AutoComplete matches for passed account.
* <p>If the account is local execute the handler otherwise makes http call to the remote server to retrieve the response
* for SOAP AutoComplete.</p>
* <p>If the account is local execute the handler otherwise makes http call to the remote server to retrieve the
* response for SOAP AutoComplete.</p>
* <p>If the requested account does not exist, an empty AutoComplete response is returned.
*
* @param authenticatedAccount The Authenticated account
* @param requestedAccountId The account ID for which the {@link AutoComplete} matches will be returned
Expand All @@ -204,16 +183,16 @@ private AutoCompleteResponse doAutoCompleteOnAccount(Account authenticatedAccoun
new AutoComplete().handle(JaxbUtil.jaxbToElement(autoCompleteRequest), context));
} else {
var requestedAccount = Provisioning.getInstance().getAccountById(requestedAccountId);
if (requestedAccount.getServer().isLocalServer()) {
if (requestedAccount != null && requestedAccount.getServer().isLocalServer()) {
var zimbraSoapContext = getZimbraSoapContext(context);
var operationContext = getOperationContext(zimbraSoapContext, context);
operationContext.setmRequestedAccountId(requestedAccountId);
return JaxbUtil.elementToJaxb(
new AutoComplete().handle(JaxbUtil.jaxbToElement(autoCompleteRequest), requestedAccount,
operationContext, zimbraSoapContext));
}
return proxyRequestInternal(requestedAccountId, autoCompleteRequest, context);
}
return proxyRequestInternal(requestedAccountId, autoCompleteRequest, context);
} catch (ServiceException e) {
ZimbraLog.misc.warn(e.getMessage());
return new AutoCompleteResponse();
Expand All @@ -226,7 +205,6 @@ AutoCompleteResponse proxyRequestInternal(String requestedAccountId, AutoComplet
requestedAccountId));
}


/**
* Parses {@link com.zimbra.common.soap.MailConstants#E_ORDERED_ACCOUNT_IDS} into a {@link Tuple2} object containing
* the "preferred account" and "other preferred accounts". Duplicates account IDs are omitted.
Expand Down Expand Up @@ -257,4 +235,50 @@ Tuple2<String, LinkedHashSet<String>> parsePreferredAccountsFrom(String preferre

return new Tuple2<>(preferredAccount, otherAccounts);
}

static class AutoCompleteMatchElementBuilder {

private final Element element;

public AutoCompleteMatchElementBuilder(ZimbraSoapContext zsc) {
this.element = zsc.createElement(MailConstants.E_MATCH);
}

public AutoCompleteMatchElementBuilder addStringAttribute(String name, String value) {
addStringAttributeWithDefault(name, value, null);
return this;
}

public AutoCompleteMatchElementBuilder addStringAttributeWithDefault(String name, String value,
String defaultValue) {
if (value != null) {
element.addAttribute(name, value);
} else if (defaultValue != null) {
element.addAttribute(name, defaultValue);
}
return this;
}

public AutoCompleteMatchElementBuilder addBooleanAttribute(String name, Boolean value, Boolean defaultValue) {
if (value != null) {
element.addAttribute(name, value);
} else if (defaultValue != null) {
element.addAttribute(name, defaultValue);
}
return this;
}

public AutoCompleteMatchElementBuilder addIntegerAttribute(String name, Integer value, Integer defaultValue) {
if (value != null) {
element.addAttribute(name, value);
} else if (defaultValue != null) {
element.addAttribute(name, defaultValue);
}
return this;
}

public Element build() {
return element;
}
}
}
110 changes: 110 additions & 0 deletions store/src/test/java/com/zimbra/cs/service/mail/FullAutoCompleteTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.zextras.mailbox.soap.SoapTestSuite;
import com.zextras.mailbox.util.MailboxTestUtil.AccountAction;
Expand All @@ -25,6 +28,7 @@
import com.zimbra.cs.mailbox.Mailbox;
import com.zimbra.cs.mailbox.MailboxManager;
import com.zimbra.cs.mailbox.OperationContext;
import com.zimbra.cs.service.mail.FullAutoComplete.AutoCompleteMatchElementBuilder;
import com.zimbra.soap.JaxbUtil;
import com.zimbra.soap.mail.message.AutoCompleteRequest;
import com.zimbra.soap.mail.message.CreateContactRequest;
Expand Down Expand Up @@ -340,6 +344,45 @@ void should_return_matches_from_authenticated_account_when_request_misses_Ordere
}
}

@Test
void should_return_matches_from_other_accounts_when_requested_account_not_found()
throws Exception {
String searchTerm = "fac";
String domain = UUID.randomUUID() + "something.com";

String contactEmail1 = searchTerm + "_email1@" + domain;
String contactEmail2 = searchTerm + "_email2@" + domain;
String contactEmail3 = searchTerm + "_email3@" + domain;
String contactEmail4 = searchTerm + "_email4@" + domain;

Account account = createRandomAccountWithContacts(contactEmail1, contactEmail2, contactEmail3, contactEmail4);
Account account2 = createRandomAccountWithContacts(contactEmail1, contactEmail2, contactEmail3, contactEmail4);

shareAccountWithPrimary(account2, account);

incrementRankings(account2, contactEmail1, 2);
incrementRankings(account2, contactEmail3, 2);

// create preferred account and delete it to simulate the corner case
var preferredAccount = createRandomAccountWithContacts();
Provisioning.getInstance().deleteAccount(preferredAccount.getId());

var orderedAccounts = new ArrayList<Account>();
orderedAccounts.add(preferredAccount);
orderedAccounts.add(account2);
var fullAutocompleteResponse = performFullAutocompleteRequest(searchTerm, account, orderedAccounts);

assertEquals(4, fullAutocompleteResponse.getMatches().size());
List<Integer> expectedRanking = List.of(2, 2, 0, 0, 2, 0, 0);
List<String> expectedMatchedEmailAddresses = Arrays.asList(contactEmail1, contactEmail3, contactEmail2,
contactEmail4);
for (int i = 0; i < fullAutocompleteResponse.getMatches().size(); i++) {
AutoCompleteMatch autoCompleteMatch = fullAutocompleteResponse.getMatches().get(i);
assertEquals(expectedRanking.get(i), autoCompleteMatch.getRanking());
assertEquals("<" + expectedMatchedEmailAddresses.get(i) + ">", autoCompleteMatch.getEmail());
}
}

@Test
void should_return_contact_group_when_matches() throws Exception {
String searchTerm = "my";
Expand Down Expand Up @@ -383,6 +426,73 @@ void should_return_contact_group_when_matches() throws Exception {
}
}

@Test
void autoCompleteMatchElementBuilder_should_omit_attributes_if_passed_value_and_default_are_null() throws Exception {
var searchTerm = "my";
var domain = UUID.randomUUID() + "something.com";
var contactEmail1 = searchTerm + "email1@" + domain;

var account = createRandomAccountWithContacts(contactEmail1);
var zsc = MailDocumentHandler.getZimbraSoapContext(ServiceTestUtil.getRequestContext(account));

var autoCompleteMatch = new AutoCompleteMatch();
autoCompleteMatch.setRanking(10);
autoCompleteMatch.setCompany("HellYeahInc");

var matchElementBuilder = new AutoCompleteMatchElementBuilder(zsc);
matchElementBuilder.addIntegerAttribute(MailConstants.A_RANKING, autoCompleteMatch.getRanking(), 0);
matchElementBuilder.addStringAttributeWithDefault(MailConstants.A_COMPANY, autoCompleteMatch.getCompany(), null);
matchElementBuilder.addBooleanAttribute(MailConstants.A_IS_GROUP, autoCompleteMatch.getGroup(), Boolean.FALSE);

matchElementBuilder.addStringAttributeWithDefault(MailConstants.A_LASTNAME, autoCompleteMatch.getLastName(), null);
matchElementBuilder.addBooleanAttribute(MailConstants.A_EXP, autoCompleteMatch.getCanExpandGroupMembers(), null);

var matchElement = matchElementBuilder.build();

assertEquals("<match ranking=\"10\" company=\"HellYeahInc\" isGroup=\"0\"/>", matchElement.toString(),
"if the passed values (value and default value) are null, the attribute should be omitted");
}

@Test
void autoCompleteMatchElementBuilder_should_add_attributes_with_defaultValues_if_passed_values_are_null()
throws Exception {
var searchTerm = "my";
var domain = UUID.randomUUID() + "something.com";
var contactEmail1 = searchTerm + "email1@" + domain;

var account = createRandomAccountWithContacts(contactEmail1);
var zsc = MailDocumentHandler.getZimbraSoapContext(ServiceTestUtil.getRequestContext(account));

var autoCompleteMatch = new AutoCompleteMatch();
var matchElementBuilder = new AutoCompleteMatchElementBuilder(zsc);

matchElementBuilder.addIntegerAttribute(MailConstants.A_RANKING, autoCompleteMatch.getRanking(), 20);
matchElementBuilder.addBooleanAttribute(MailConstants.A_IS_GROUP, autoCompleteMatch.getGroup(), Boolean.TRUE);
matchElementBuilder.addStringAttributeWithDefault(MailConstants.A_LASTNAME, autoCompleteMatch.getLastName(), "myLastName");

var matchElement2 = matchElementBuilder.build();

assertEquals("<match last=\"myLastName\" ranking=\"20\" isGroup=\"1\"/>",
matchElement2.toString(),
"if the passed value is null and defaultValue is valid, the attribute should be added with the defaultValue passed");
}

@Test
void should_throw_exception_when_request_element_cannot_be_converted_to_fac_request_object() throws Exception {
var account = accountCreatorFactory.get().create();

var mockElement = mock(Element.class);
when(JaxbUtil.elementToJaxb(mockElement)).thenReturn(null);

var fullAutoComplete = Mockito.spy(FullAutoComplete.class);
var requestContext = ServiceTestUtil.getRequestContext(account);

var serviceException = assertThrows(ServiceException.class,
() -> fullAutoComplete.handle(mockElement, requestContext));

assertEquals(ServiceException.FAILURE, serviceException.getCode());
}

@ParameterizedTest
@MethodSource("parsePreferredAccountsTestData")
void testParsePreferredAccountsFrom(String input, String expectedPreferredAccount,
Expand Down

0 comments on commit 818b57d

Please sign in to comment.