Skip to content

Commit

Permalink
Fix for null return values in query expressions.
Browse files Browse the repository at this point in the history
  • Loading branch information
@semancik
semancik committed Oct 23, 2014
1 parent a4ddf90 commit 6a02713
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 45 deletions.
60 changes: 23 additions & 37 deletions ...el-common/src/main/java/com/evolveum/midpoint/model/common/expression/ExpressionUtil.java
View file
Expand Up @@ -62,6 +62,7 @@
import com.evolveum.midpoint.prism.polystring.PolyString;
import com.evolveum.midpoint.prism.query.EqualFilter;
import com.evolveum.midpoint.prism.query.LogicalFilter;
import com.evolveum.midpoint.prism.query.NoneFilter;
import com.evolveum.midpoint.prism.query.ObjectFilter;
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.prism.query.PropertyValueFilter;
Expand Down Expand Up @@ -352,7 +353,8 @@ public static ObjectQuery evaluateQueryExpressions(ObjectQuery origQuery, Expres
return null;
}
ObjectQuery query = origQuery.clone();
evaluateFilterExpressionsInternal(query.getFilter(), variables, expressionFactory, prismContext, shortDesc, task, result);
ObjectFilter evaluatedFilter = evaluateFilterExpressionsInternal(query.getFilter(), variables, expressionFactory, prismContext, shortDesc, task, result);
query.setFilter(evaluatedFilter);
return query;
}

Expand All @@ -363,39 +365,38 @@ public static ObjectFilter evaluateFilterExpressions(ObjectFilter origFilter, Ex
return null;
}

ObjectFilter filter = origFilter.clone();

evaluateFilterExpressionsInternal(filter, variables, expressionFactory, prismContext, shortDesc, task, result);

return filter;
return evaluateFilterExpressionsInternal(origFilter, variables, expressionFactory, prismContext, shortDesc, task, result);
}

private static void evaluateFilterExpressionsInternal(ObjectFilter filter, ExpressionVariables variables,
private static ObjectFilter evaluateFilterExpressionsInternal(ObjectFilter filter, ExpressionVariables variables,
ExpressionFactory expressionFactory, PrismContext prismContext,
String shortDesc, Task task, OperationResult result) throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException {
if (filter == null) {
return;
return null;
}

if (filter instanceof LogicalFilter) {
List<ObjectFilter> conditions = ((LogicalFilter) filter).getConditions();
LogicalFilter evaluatedFilter = (LogicalFilter) filter.clone();
evaluatedFilter.getConditions().clear();
for (ObjectFilter condition : conditions) {
evaluateFilterExpressionsInternal(condition, variables, expressionFactory, prismContext, shortDesc, task, result);
ObjectFilter evaluatedSubFilter = evaluateFilterExpressionsInternal(condition, variables, expressionFactory, prismContext, shortDesc, task, result);
evaluatedFilter.addCondition(evaluatedSubFilter);
}
return;
return evaluatedFilter;

} else if (filter instanceof PropertyValueFilter) {

PropertyValueFilter pvfilter = (PropertyValueFilter) filter;

if (pvfilter.getValues() != null && !pvfilter.getValues().isEmpty()) {
// We have value. Nothing to evaluate.
return;
return pvfilter.clone();
}

ExpressionWrapper expressionWrapper = pvfilter.getExpression();
if (expressionWrapper == null || expressionWrapper.getExpression() == null) {
LOGGER.warn("No valueExpression in filter in {}", shortDesc);
return;
return NoneFilter.createNone();
}
if (!(expressionWrapper.getExpression() instanceof ExpressionType)) {
throw new SchemaException("Unexpected expression type " + expressionWrapper.getExpression().getClass() + " in filter in " + shortDesc);
Expand All @@ -410,18 +411,22 @@ private static void evaluateFilterExpressionsInternal(ObjectFilter filter, Expre
if (expressionResult == null || expressionResult.isEmpty()) {
LOGGER.debug("Result of search filter expression was null or empty. Expression: {}",
valueExpression);
return;
return NoneFilter.createNone();
}
// TODO: log more context
LOGGER.trace("Search filter expression in the rule for {} evaluated to {}.", new Object[] {
shortDesc, expressionResult });
if (filter instanceof EqualFilter) {
((EqualFilter) filter).setValue(expressionResult);
pvfilter.setExpression(null);

PropertyValueFilter evaluatedFilter = (PropertyValueFilter) pvfilter.clone();
if (evaluatedFilter instanceof EqualFilter) {
((EqualFilter) evaluatedFilter).setValue(expressionResult);
evaluatedFilter.setExpression(null);
}
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Transformed filter to:\n{}", filter.debugDump());
LOGGER.trace("Transformed filter to:\n{}", evaluatedFilter.debugDump());
}
return evaluatedFilter;

} catch (RuntimeException ex) {
LoggingUtils.logException(LOGGER, "Couldn't evaluate expression " + valueExpression + ".", ex);
throw new SystemException("Couldn't evaluate expression" + valueExpression + ": "
Expand All @@ -447,25 +452,6 @@ private static void evaluateFilterExpressionsInternal(ObjectFilter filter, Expre

}

// seems to be unused [mederly]
// public static ExpressionType createExpression(Element valueExpressionElement, PrismContext prismContext) throws SchemaException {
// ExpressionType valueExpression = null;
// try {
// valueExpression = prismContext.getJaxbDomHack().toJavaValue(
// valueExpressionElement, ExpressionType.class);
//
// if (LOGGER.isTraceEnabled()) {
// LOGGER.trace("Filter transformed to expression\n{}", valueExpression);
// }
// } catch (JAXBException ex) {
// LoggingUtils.logException(LOGGER, "Expression element couldn't be transformed.", ex);
// throw new SchemaException("Expression element couldn't be transformed: " + ex.getMessage(), ex);
// }
//
// return valueExpression;
//
// }

private static PrismPropertyValue evaluateExpression(ExpressionVariables variables, PrismContext prismContext,
ExpressionType expressionType, ObjectFilter filter, ExpressionFactory expressionFactory,
String shortDesc, Task task, OperationResult parentResult) throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException {
Expand Down
111 changes: 104 additions & 7 deletions testing/story/src/test/java/com/evolveum/midpoint/testing/story/TestVillage.java
View file
Expand Up @@ -216,7 +216,10 @@ public class TestVillage extends AbstractStoryTest {
private static final String ACCOUNT_MANCOMB_USERNAME = "mancomb";
private static final String ACCOUNT_MANCOMB_FIST_NAME = "Mancomb";
private static final String ACCOUNT_MANCOMB_LAST_NAME = "Seepgood";

private static final String ACCOUNT_MANCOMB_LOC = "-";
private static final String ACCOUNT_MANCOMB_ORG = "-";
private static final String USER_MANCOMB_NAME = ACCOUNT_MANCOMB_FIST_NAME+"."+ACCOUNT_MANCOMB_LAST_NAME;

private static final String ACCOUNT_COBB_USERNAME = "cobb";
private static final String ACCOUNT_COBB_FIST_NAME = "Cobb";
private static final String ACCOUNT_COBB_LAST_NAME = "Loom";
Expand Down Expand Up @@ -561,7 +564,7 @@ public void test110AddSrcAccountLemonhead() throws Exception {
}

/**
* Wally has no org. Username without an org should be created.
* Wally has no org. User without an org should be created.
*/
@Test
public void test120AddSrcAccountWally() throws Exception {
Expand All @@ -582,13 +585,107 @@ public void test120AddSrcAccountWally() throws Exception {
assertUserNoRole(userAfter, ACCOUNT_WALLY_FIST_NAME, ACCOUNT_WALLY_LAST_NAME, null);
assertLocGov(userAfter, null, null);
}

@Test
public void test121WallyAssignBasicRole() throws Exception {
final String TEST_NAME = "test121WallyAssignBasicRole";
TestUtil.displayTestTile(this, TEST_NAME);
Task task = taskManager.createTaskInstance(TestTrafo.class.getName() + "." + TEST_NAME);

PrismObject<UserType> user = findUserByUsername(USER_WALLY_NAME);

// WHEN
assignRole(user.getOid(), ROLE_BASIC_OID);

// THEN
PrismObject<UserType> userAfter = getUser(user.getOid());
assertUserLdap(userAfter, ACCOUNT_WALLY_FIST_NAME, ACCOUNT_WALLY_LAST_NAME, null);
assertLocGov(userAfter, null, null);
}

@Test
public void test122WallyUnAssignBasicRole() throws Exception {
final String TEST_NAME = "test122WallyUnAssignBasicRole";
TestUtil.displayTestTile(this, TEST_NAME);
Task task = taskManager.createTaskInstance(TestTrafo.class.getName() + "." + TEST_NAME);

PrismObject<UserType> user = findUserByUsername(USER_WALLY_NAME);

// WHEN
unassignRole(user.getOid(), ROLE_BASIC_OID);

// THEN
PrismObject<UserType> userAfter = getUser(user.getOid());
assertUserNoRole(userAfter, ACCOUNT_WALLY_FIST_NAME, ACCOUNT_WALLY_LAST_NAME, null);
assertLocGov(userAfter, null, null);
}

/**
* Wally has no org. User without an org should be created.
*/
@Test
public void test130AddSrcAccountMancomb() throws Exception {
final String TEST_NAME = "test130AddSrcAccountMancomb";
TestUtil.displayTestTile(this, TEST_NAME);
Task task = taskManager.createTaskInstance(TestTrafo.class.getName() + "." + TEST_NAME);

DummyAccount newAccount = new DummyAccount(ACCOUNT_MANCOMB_USERNAME);
newAccount.addAttributeValue(DUMMY_ACCOUNT_ATTRIBUTE_SRC_FIRST_NAME, ACCOUNT_MANCOMB_FIST_NAME);
newAccount.addAttributeValue(DUMMY_ACCOUNT_ATTRIBUTE_SRC_LAST_NAME, ACCOUNT_MANCOMB_LAST_NAME);
newAccount.addAttributeValue(DUMMY_ACCOUNT_ATTRIBUTE_SRC_LOC, ACCOUNT_MANCOMB_LOC);
newAccount.addAttributeValue(DUMMY_ACCOUNT_ATTRIBUTE_SRC_ORG, ACCOUNT_MANCOMB_ORG);

// WHEN
dummyResourceSrc.addAccount(newAccount);
waitForTaskNextRun(TASK_LIVE_SYNC_DUMMY_SOURCE_OID, true);

// THEN
PrismObject<UserType> userAfter = findUserByUsername(USER_MANCOMB_NAME);
assertUserNoRole(userAfter, ACCOUNT_MANCOMB_FIST_NAME, ACCOUNT_MANCOMB_LAST_NAME, null);
assertLocGov(userAfter, ACCOUNT_MANCOMB_LOC, ACCOUNT_MANCOMB_ORG);
}

@Test
public void test131MancombAssignBasicRole() throws Exception {
final String TEST_NAME = "test131WallyAssignBasicRole";
TestUtil.displayTestTile(this, TEST_NAME);
Task task = taskManager.createTaskInstance(TestTrafo.class.getName() + "." + TEST_NAME);

PrismObject<UserType> user = findUserByUsername(USER_MANCOMB_NAME);

// WHEN
assignRole(user.getOid(), ROLE_BASIC_OID);

// THEN
PrismObject<UserType> userAfter = getUser(user.getOid());
assertUserLdap(userAfter, ACCOUNT_MANCOMB_FIST_NAME, ACCOUNT_MANCOMB_LAST_NAME, null);
assertLocGov(userAfter, ACCOUNT_MANCOMB_LOC, ACCOUNT_MANCOMB_ORG);
}

@Test
public void test132MancombUnAssignBasicRole() throws Exception {
final String TEST_NAME = "test132MancombUnAssignBasicRole";
TestUtil.displayTestTile(this, TEST_NAME);
Task task = taskManager.createTaskInstance(TestTrafo.class.getName() + "." + TEST_NAME);

PrismObject<UserType> user = findUserByUsername(USER_MANCOMB_NAME);

// WHEN
unassignRole(user.getOid(), ROLE_BASIC_OID);

// THEN
PrismObject<UserType> userAfter = getUser(user.getOid());
assertUserNoRole(userAfter, ACCOUNT_MANCOMB_FIST_NAME, ACCOUNT_MANCOMB_LAST_NAME, null);
assertLocGov(userAfter, ACCOUNT_MANCOMB_LOC, ACCOUNT_MANCOMB_ORG);
}


/**
* Change of org should trigger rename
*/
@Test
public void test130ModifySrcAccountHermanReplaceOrg() throws Exception {
final String TEST_NAME = "test130ModifySrcAccountHermanReplaceOrg";
public void test150ModifySrcAccountHermanReplaceOrg() throws Exception {
final String TEST_NAME = "test150ModifySrcAccountHermanReplaceOrg";
TestUtil.displayTestTile(this, TEST_NAME);
Task task = taskManager.createTaskInstance(TestTrafo.class.getName() + "." + TEST_NAME);

Expand All @@ -611,8 +708,8 @@ public void test130ModifySrcAccountHermanReplaceOrg() throws Exception {
* Change of org should trigger rename
*/
@Test
public void test132ModifySrcAccountHermanDeleteOrg() throws Exception {
final String TEST_NAME = "test132ModifySrcAccountHermanDeleteOrg";
public void test152ModifySrcAccountHermanDeleteOrg() throws Exception {
final String TEST_NAME = "test152ModifySrcAccountHermanDeleteOrg";
TestUtil.displayTestTile(this, TEST_NAME);
Task task = taskManager.createTaskInstance(TestTrafo.class.getName() + "." + TEST_NAME);

Expand Down Expand Up @@ -776,7 +873,7 @@ private void assertLocGov(PrismObject<UserType> user, String expLoc, String expO
} else {
assertEquals("Wrong costCenter in "+user, userType.getCostCenter(), expOrg+":"+expLoc);
}
if (expOrg != null) {
if (expOrg != null && !expOrg.equals("-")) {
PrismObject<OrgType> org = findObjectByName(OrgType.class, expOrg);
assertAssigned(user, org.getOid(), OrgType.COMPLEX_TYPE);
String orgId = org.asObjectable().getIdentifier();
Expand Down
5 changes: 4 additions & 1 deletion testing/story/src/test/resources/village/role-basic.xml
View file
Expand Up @@ -44,7 +44,10 @@
</q:path>
<expression>
<script>
<code>'cn='+costCenter+',ou=groups,dc=example,dc=com'</code>
<code>
if (costCenter == "-:-") { return null; }
'cn='+costCenter+',ou=groups,dc=example,dc=com'
</code>
</script>
</expression>
</q:equal>
Expand Down

0 comments on commit 6a02713

Please sign in to comment.