Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed issue of getting 0 results when filtering with 2+ meta attributes #145

Merged
merged 3 commits into from
Jul 22, 2024
Merged

Fixed issue of getting 0 results when filtering with 2+ meta attributes #145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9c61d49
Fixed issue of getting 0 results when filtering with 2+ meta attributes
HasiniSama Jul 14, 2024
82faa29
Merge branch 'main' into extending_org_search_feature
HasiniSama Jul 22, 2024
deb798e
Added unit tests for multiattribute filtering
HasiniSama Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...on/identity/organization/management/service/constant/OrganizationManagementConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public class OrganizationManagementConstants {
public static final String AND = "and";
public static final String FILTER_PLACEHOLDER_PREFIX = "FILTER_ID_";
public static final String PARENT_ID_FILTER_PLACEHOLDER_PREFIX = "FILTER_PARENT_ID_";
public static final String META_ATTRIBUTE_PLACEHOLDER_PREFIX = "UM_ORG_ATTRIBUTE_";
private static final String ORGANIZATION_MANAGEMENT_ERROR_CODE_PREFIX = "ORG-";
private static final Map<String, String> attributeColumnMap = new HashMap<>();
public static final Map<String, String> ATTRIBUTE_COLUMN_MAP = Collections.unmodifiableMap(attributeColumnMap);
Expand Down Expand Up @@ -421,7 +422,7 @@ public enum ErrorMessages {
ERROR_CODE_ERROR_EVALUATING_ADD_ORGANIZATION_AUTHORIZATION("65019", "Unable to create the organization.",
"Server encountered an error while evaluating authorization of user to create the " +
"organization in parent organization with ID: %s."),
ERROR_CODE_ERROR_BUILDING_PAGINATED_RESPONSE_URL("65020", "Unable to retrieve the organizations.",
ERROR_CODE_ERROR_BUILDING_PAGINATED_RESPONSE_URL("65020", "Unable to retrieve the paginated results.",
"Server encountered an error while building paginated response URL."),
ERROR_CODE_ERROR_MISSING_SUPER("65021", "Unable to create the organization.",
"Server encountered an error while retrieving the super organization."),
Expand Down
2 changes: 1 addition & 1 deletion .../java/org/wso2/carbon/identity/organization/management/service/constant/SQLConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ public class SQLConstants {
"FETCH NEXT :" + SQLPlaceholders.DB_SCHEMA_LIMIT + "; ROWS ONLY";

public static final String INNER_JOIN_UM_ORG_ATTRIBUTE =
"INNER JOIN UM_ORG_ATTRIBUTE ON UM_ORG.UM_ID = UM_ORG_ATTRIBUTE.UM_ORG_ID WHERE";
"INNER JOIN UM_ORG_ATTRIBUTE %s ON UM_ORG.UM_ID = %s.UM_ORG_ID WHERE";

public static final String GET_ALL_UM_ORG_ATTRIBUTES = "SELECT UM_ORG.UM_ID, UM_ORG.UM_ORG_NAME, " +
"UM_ORG.UM_CREATED_TIME, UM_ORG.UM_STATUS, UM_ORG_ATTRIBUTE.UM_ATTRIBUTE_KEY, " +
Expand Down
26 changes: 15 additions & 11 deletions ...rbon/identity/organization/management/service/dao/impl/OrganizationManagementDAOImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@
import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.VIEW_ID_COLUMN;
import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.VIEW_LAST_MODIFIED_COLUMN;
import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.VIEW_NAME_COLUMN;
import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.VIEW_ORGANIZATION_ATTRIBUTES_TABLE;
import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.VIEW_PARENT_ID_COLUMN;
import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.VIEW_STATUS_COLUMN;
import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.VIEW_TENANT_UUID_COLUMN;
Expand Down Expand Up @@ -739,8 +738,9 @@ private List<Organization> getOrganizationsList(boolean authorizedSubOrgsOnly, b
String userID = getUserId();
String sqlStmt = prepareGetOrganizationQuery(authorizedSubOrgsOnly, recursive, sortOrder, applicationAudience,
filterQueryBuilder, parentIdFilterQueryBuilder, userID);
boolean isFilteringMetaAttributes = filterQueryBuilder.getMetaAttributeCount() > 0;

if (filterQueryBuilder.getHasSubAttribute()) {
if (isFilteringMetaAttributes) {
sqlStmt = String.format(GET_ALL_UM_ORG_ATTRIBUTES, sqlStmt);
}
List<Organization> organizations;
Expand All @@ -749,7 +749,7 @@ private List<Organization> getOrganizationsList(boolean authorizedSubOrgsOnly, b
try {
organizations = namedJdbcTemplate.executeQuery(sqlStmt,
(resultSet, rowNumber) -> {
if (filterQueryBuilder.getHasSubAttribute()) {
if (isFilteringMetaAttributes) {
String orgId = resultSet.getString(1);
Organization organization = organizationMap.get(orgId);
if (organization == null) {
Expand All @@ -769,7 +769,7 @@ private List<Organization> getOrganizationsList(boolean authorizedSubOrgsOnly, b
} catch (DataAccessException e) {
throw handleServerException(ERROR_CODE_ERROR_RETRIEVING_ORGANIZATIONS, e);
}
if (filterQueryBuilder.getHasSubAttribute()) {
if (isFilteringMetaAttributes) {
return new ArrayList<>(organizationMap.values());
}
return organizations;
Expand Down Expand Up @@ -1557,8 +1557,12 @@ private String prepareGetOrganizationQuery(boolean authorizedSubOrgsOnly, boolea

String sqlStmt = getOrgSqlStatement(authorizedSubOrgsOnly, applicationAudience);
String getOrgSqlStmtTail = getOrgSqlStmtTail(authorizedSubOrgsOnly, applicationAudience);
if (filterQueryBuilder.getHasSubAttribute()) {
sqlStmt = sqlStmt.replace("WHERE", INNER_JOIN_UM_ORG_ATTRIBUTE);

if (filterQueryBuilder.getMetaAttributeCount() > 0) {
for (String placeholder : filterQueryBuilder.getMetaAttributePlaceholders()) {
sqlStmt = sqlStmt.replace("WHERE",
String.format(INNER_JOIN_UM_ORG_ATTRIBUTE, placeholder, placeholder));
}
}
sqlStmt = appendFilterQueries(sqlStmt, filterQueryBuilder, parentIdFilterQueryBuilder, getOrgSqlStmtTail,
recursive, sortOrder);
Expand Down Expand Up @@ -1662,11 +1666,11 @@ private void setFilterAttributes(NamedPreparedStatement namedPreparedStatement,

private String handleViewAttrKeyColumn(ExpressionNode expressionNode, FilterQueryBuilder filterQueryBuilder) {

filterQueryBuilder.setHasSubAttribute(true);
String placeholder = filterQueryBuilder.generateMetaAttributePlaceholder();
String attributeValue = expressionNode.getAttributeValue();
String subAttributeName = StringUtils.substringAfter(attributeValue,
ORGANIZATION_ATTRIBUTES_FIELD + ".");
return VIEW_ORGANIZATION_ATTRIBUTES_TABLE + "." + VIEW_ATTR_KEY_COLUMN + " = '" + subAttributeName + "' AND " +
VIEW_ORGANIZATION_ATTRIBUTES_TABLE + "." + VIEW_ATTR_VALUE_COLUMN;
String subAttributeName = StringUtils.substringAfter(attributeValue, ORGANIZATION_ATTRIBUTES_FIELD + ".");

return placeholder + "." + VIEW_ATTR_KEY_COLUMN + " = '" + subAttributeName + "' AND " + placeholder + "."
+ VIEW_ATTR_VALUE_COLUMN;
}
}
34 changes: 25 additions & 9 deletions ...va/org/wso2/carbon/identity/organization/management/service/model/FilterQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.List;
import java.util.Map;

import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.META_ATTRIBUTE_PLACEHOLDER_PREFIX;

/**
* Create filter query builder.
*/
Expand All @@ -32,7 +34,8 @@ public class FilterQueryBuilder {
private List<String> timestampParameters = new ArrayList<>();
private int count = 1;
private String filter;
private boolean hasSubAttribute;
private List<String> metaAttributePlaceholders = new ArrayList<>();
private int metaAttributeCount;

/**
* Get filter query builder attributes.
Expand Down Expand Up @@ -97,22 +100,35 @@ public String getFilterQuery() {
}

/**
* Get the value of hasSubAttribute.
* Get filter query builder meta attribute placeholders.
*
* @return List of filter query builder meta attribute placeholders.
*/
public List<String> getMetaAttributePlaceholders() {

return metaAttributePlaceholders;
}

/**
* Generate and add a filter query builder meta attribute placeholder.
*
* @return boolean value indicating if the filter contains a sub attribute.
* @return Next meta attribute placeholder.
*/
public boolean getHasSubAttribute() {
HasiniSama marked this conversation as resolved.
Show resolved Hide resolved
public String generateMetaAttributePlaceholder() {

return hasSubAttribute;
metaAttributeCount++;
String placeholder = META_ATTRIBUTE_PLACEHOLDER_PREFIX + metaAttributeCount;
metaAttributePlaceholders.add(placeholder);
return placeholder;
}

/**
* Set the value of hasSubAttribute.
* Get the current count of meta attribute placeholders.
*
* @param hasSubAttribute boolean value indicating if the filter contains a sub attributes.
* @return Meta attribute count.
*/
public void setHasSubAttribute(boolean hasSubAttribute) {
public int getMetaAttributeCount() {

this.hasSubAttribute = hasSubAttribute;
return metaAttributeCount;
}
}
2 changes: 2 additions & 0 deletions ...org/wso2/carbon/identity/organization/management/service/OrganizationManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,8 @@ public Object[][] dataForFilterOrganizationsByMetaAttributes() {

return new Object[][]{
{"attributes.country co S", false},
{"attributes.country co S and name eq Greater", false},
{"attributes.country sw S and attributes.city ew o", false},
{"attributes.country co Z", true},
{"attributes.invalid co S", true}
};
Expand Down
35 changes: 35 additions & 0 deletions .../identity/organization/management/service/dao/impl/OrganizationManagementDAOImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ public class OrganizationManagementDAOImplTest {
private static final String ATTRIBUTE_KEY = "country";
private static final String ATTRIBUTE_KEY_REGION = "region";
private static final String ATTRIBUTE_VALUE = "Sri Lanka";
private static final String ATTRIBUTE_KEY_CITY = "city";
private static final String ATTRIBUTE_VALUE_CITY = "Colombo";
private static final String ORG_NAME = "XYZ builders";
private static final String CHILD_ORG_NAME = "ABC builders";
private static final String ORG_DESCRIPTION = "This is a construction company.";
Expand All @@ -76,6 +78,7 @@ public void setUp() throws Exception {
orgId = generateUniqueID();
childOrgId = generateUniqueID();
storeChildOrganization(orgId, ORG_NAME, ORG_DESCRIPTION, SUPER_ORG_ID);
storeOrganizationAttributes(orgId, ATTRIBUTE_KEY_CITY, ATTRIBUTE_VALUE_CITY);
storeChildOrganization(childOrgId, CHILD_ORG_NAME, ORG_DESCRIPTION, orgId);
storeOrganizationAttributes(childOrgId, ATTRIBUTE_KEY_REGION, ATTRIBUTE_VALUE);
}
Expand Down Expand Up @@ -213,6 +216,38 @@ public void testFilterOrganizationsByMetaAttributes(String attributeValue, Strin
SUPER_ORG_ID, "DESC", expressionNodes, new ArrayList<>());

Assert.assertEquals(organizations.get(0).getName(), ORG_NAME);
}

@DataProvider(name = "dataForFilterOrganizationsByMultipleAttributes")
public Object[][] dataForFilterOrganizationsByMultipleAttributes() {

return new Object[][]{
{"attributes.country co S and name co XYZ"},
{"attributes.country sw S and attributes.country ew a and attributes.city eq Colombo"},
};
}

@Test(dataProvider = "dataForFilterOrganizationsByMultipleAttributes")
public void testFilterOrganizationsByMultipleAttributes(String filter)
throws OrganizationManagementServerException {

TestUtils.mockCarbonContext(SUPER_ORG_ID);
List<ExpressionNode> expressionNodes = new ArrayList<>();
String[] filters = filter.split(" and ");

for (String singleFilter : filters) {
String[] parts = singleFilter.split(" ");
String attributeValue = parts[0];
String operation = parts[1];
String value = parts[2];

ExpressionNode expressionNode = getExpressionNode(attributeValue, operation, value);
expressionNodes.add(expressionNode);
}
List<Organization> organizations = organizationManagementDAO.getOrganizationsList(false, 10,
SUPER_ORG_ID, "DESC", expressionNodes, new ArrayList<>());

Assert.assertEquals(organizations.get(0).getName(), ORG_NAME);
Assert.assertEquals(organizations.get(0).getAttributes().get(0).getKey(), ATTRIBUTE_KEY);
Assert.assertEquals(organizations.get(0).getAttributes().get(0).getValue(), ATTRIBUTE_VALUE);
}
Expand Down
Loading