Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Commit

Permalink
rhbz882770 - separate search and other conditions to fix a bug
Browse files Browse the repository at this point in the history
  • Loading branch information
Patrick Huang committed Apr 13, 2014
1 parent b752eb0 commit de82469
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 69 deletions.
76 changes: 46 additions & 30 deletions zanata-war/src/main/java/org/zanata/search/FilterConstraintToQuery.java
View file
Expand Up @@ -100,6 +100,8 @@ private String buildQuery(String selectStatement, String docIdCondition) {
String obsoleteCondition = eq("tf.obsolete", "0");
String searchCondition = buildSearchCondition();
String stateCondition = buildStateCondition();
String otherSourceCondition = buildSourceConditionsOtherThanSearch();
String otherTargetCondition = buildTargetConditionsOtherThanSearch();

QueryBuilder query =
QueryBuilder
Expand All @@ -108,14 +110,16 @@ private String buildQuery(String selectStatement, String docIdCondition) {
.leftJoin("tf.targets tfts")
.with(eq("tfts.index", locale.placeHolder()))
.where(and(obsoleteCondition, docIdCondition,
searchCondition, stateCondition))
searchCondition, stateCondition,
otherSourceCondition, otherTargetCondition))
.orderBy("tf.pos");
return query.toQueryString();
}

/**
* This builds a query for editor modal navigation. It only select text flow
* id and text flow target content state.
* id and text flow target content state (text flow pos is also in select
* but just for ordering).
*
* @return the HQL query
*/
Expand All @@ -128,39 +132,23 @@ public String toModalNavigationQuery() {
}

protected String buildSearchCondition() {
if (!hasSearch) {
return null;
}
String searchInSourceCondition = null;
List<String> sourceConjunction = Lists.newArrayList();
if (hasSearch && constraints.isSearchInSource()) {
String contentsCriterion =
if (constraints.isSearchInSource()) {
searchInSourceCondition =
contentCriterion.contentsCriterionAsString("tf",
constraints.isCaseSensitive(),
Parameters.searchString.placeHolder());
addToJunctionIfNotNull(sourceConjunction, contentsCriterion);
}
addToJunctionIfNotNull(sourceConjunction,
buildSourceCommentCondition(constraints.getSourceComment()));
addToJunctionIfNotNull(sourceConjunction, buildMsgContextCondition());
addToJunctionIfNotNull(sourceConjunction, buildResourceIdCondition());

if (!sourceConjunction.isEmpty()) {
searchInSourceCondition = QueryBuilder.and(sourceConjunction);
}

String searchInTargetCondition = null;
List<String> targetConjunction = Lists.newArrayList();

if (hasSearch && constraints.isSearchInTarget()) {
if (constraints.isSearchInTarget()) {
List<String> targetConjunction = Lists.newArrayList();
targetConjunction.add(contentCriterion.contentsCriterionAsString(
null, constraints.isCaseSensitive(),
Parameters.searchString.placeHolder()));
}
addToJunctionIfNotNull(targetConjunction,
buildLastModifiedByCondition());
addToJunctionIfNotNull(targetConjunction,
buildTargetCommentCondition(constraints.getTransComment()));
addToJunctionIfNotNull(targetConjunction,
buildLastModifiedDateCondition());
if (!targetConjunction.isEmpty()) {
targetConjunction.add(eq("textFlow", "tf"));
targetConjunction.add(eq("locale", locale.placeHolder()));

Expand All @@ -169,19 +157,47 @@ protected String buildSearchCondition() {
.where(QueryBuilder.and(targetConjunction))
.toQueryString();
}
if (searchInSourceCondition == null && searchInTargetCondition == null) {
return null;
}
return QueryBuilder
.or(searchInSourceCondition, searchInTargetCondition);
}

private static List<String> addToJunctionIfNotNull(List<String> junction,
protected String buildSourceConditionsOtherThanSearch() {
List<String> sourceConjunction = Lists.newArrayList();
addToJunctionIfNotNull(sourceConjunction,
buildSourceCommentCondition(constraints.getSourceComment()));
addToJunctionIfNotNull(sourceConjunction, buildMsgContextCondition());
addToJunctionIfNotNull(sourceConjunction, buildResourceIdCondition());
if (sourceConjunction.isEmpty()) {
return null;
}
return QueryBuilder.and(sourceConjunction);
}

protected String buildTargetConditionsOtherThanSearch() {
List<String> targetConjunction = Lists.newArrayList();
addToJunctionIfNotNull(targetConjunction,
buildLastModifiedByCondition());
addToJunctionIfNotNull(targetConjunction,
buildTargetCommentCondition(constraints.getTransComment()));
addToJunctionIfNotNull(targetConjunction,
buildLastModifiedDateCondition());
if (targetConjunction.isEmpty()) {
return null;
}
targetConjunction.add(eq("textFlow", "tf"));
targetConjunction.add(eq("locale", locale.placeHolder()));

return QueryBuilder.exists().from("HTextFlowTarget")
.where(and(targetConjunction)).toQueryString();
}

private static boolean addToJunctionIfNotNull(List<String> junction,
String criterion) {
if (criterion != null) {
junction.add(criterion);
return true;
}
return junction;
return false;
}

private String buildResourceIdCondition() {
Expand Down
76 changes: 45 additions & 31 deletions zanata-war/src/test/java/org/zanata/search/FilterConstraintToQueryJpaTest.java
View file
Expand Up @@ -35,9 +35,11 @@
@Slf4j
public class FilterConstraintToQueryJpaTest extends ZanataJpaTest {

private HLocale hLocale;
private ContentStateGroup untranslatedOnly = ContentStateGroup.builder()
.removeAll().includeNew(true).build();
private ContentStateGroup allContentStates = ContentStateGroup.builder()
.addAll().build();
private HLocale hLocale;
private DocumentId documentId;
private DateTime today = new DateTime();
private DateTime yesterday = new DateTime().minusDays(1);
Expand All @@ -50,8 +52,9 @@ public void setUpData() {
hLocale =
EntityMakerBuilder.builder().build()
.makeAndPersist(getEm(), HLocale.class);
transformer = new GetTransUnitsNavigationService.TextFlowResultTransformer(
hLocale);
transformer =
new GetTransUnitsNavigationService.TextFlowResultTransformer(
hLocale);

admin = makePerson("admin");
translator = makePerson("translator");
Expand Down Expand Up @@ -150,7 +153,9 @@ public void getAll() {
@SuppressWarnings("unchecked")
private List<HTextFlow> getNavigationResult(String navigationQuery,
FilterConstraintToQuery constraintToQuery) {
org.hibernate.Query query = getSession().createQuery(navigationQuery).setResultTransformer(transformer);
org.hibernate.Query query =
getSession().createQuery(navigationQuery).setResultTransformer(
transformer);
return constraintToQuery.setQueryParameters(query, hLocale).list();
}

Expand Down Expand Up @@ -180,20 +185,21 @@ private void verifyModalNavigationQuery(
verifyIdAndContentStateMatches(result, navigationResult);
}

private void verifyIdAndContentStateMatches(List<HTextFlow> one, List<HTextFlow> two) {
private void verifyIdAndContentStateMatches(List<HTextFlow> one,
List<HTextFlow> two) {
assertThat(one.size(), Matchers.equalTo(two.size()));
for (int i = 0; i < one.size(); i++) {
HTextFlow textFlow1 = one.get(i);
HTextFlow textFlow2 = two.get(i);
assertThat(textFlow1.getId(), Matchers.equalTo(textFlow2.getId()));
assertThat(getContentState(textFlow1, hLocale), Matchers.equalTo(
getContentState(textFlow2, hLocale)));
assertThat(getContentState(textFlow1, hLocale),
Matchers.equalTo(getContentState(textFlow2, hLocale)));
}
}

private static ContentState getContentState(HTextFlow hTextFlow, HLocale hLocale) {
HTextFlowTarget target =
hTextFlow.getTargets().get(hLocale.getId());
private static ContentState getContentState(HTextFlow hTextFlow,
HLocale hLocale) {
HTextFlowTarget target = hTextFlow.getTargets().get(hLocale.getId());
return target == null ? ContentState.New : target.getState();
}

Expand All @@ -218,13 +224,9 @@ public void filterByContentInSourceAndTarget() {
public void filterByUntranslated() {
FilterConstraintToQuery constraintToQuery =
FilterConstraintToQuery.filterInSingleDocument(
FilterConstraints
.builder()
.keepNone()
.includeStates(
ContentStateGroup.builder().removeAll()
.includeNew(true).build())
.build(), documentId);
FilterConstraints.builder().keepNone()
.includeStates(untranslatedOnly).build(),
documentId);

String hql = constraintToQuery.toEntityQuery();
List<HTextFlow> result = getResultList(hql, constraintToQuery);
Expand All @@ -238,13 +240,9 @@ public void filterByUntranslated() {
public void filterByUntranslatedAndSourceContent() {
FilterConstraintToQuery constraintToQuery =
FilterConstraintToQuery.filterInSingleDocument(
FilterConstraints
.builder()
.keepNone()
FilterConstraints.builder().keepNone()
.checkInSource(true)
.includeStates(
ContentStateGroup.builder().removeAll()
.includeNew(true).build())
.includeStates(untranslatedOnly)
.filterBy("source 4").build(), documentId);

String hql = constraintToQuery.toEntityQuery();
Expand Down Expand Up @@ -359,11 +357,10 @@ public void filterByTargetChangedDateAfter() {
@Test
public void filterByTargetChangedDateBefore() {
FilterConstraintToQuery constraintToQuery =
FilterConstraintToQuery.filterInSingleDocument(
FilterConstraints.builder().keepNone()
.includeStates(allContentStates)
.targetChangedBefore(today).build(),
documentId);
FilterConstraintToQuery
.filterInSingleDocument(FilterConstraints.builder()
.keepNone().includeStates(allContentStates)
.targetChangedBefore(today).build(), documentId);

String hql = constraintToQuery.toEntityQuery();
List<HTextFlow> result = getResultList(hql, constraintToQuery);
Expand All @@ -378,9 +375,26 @@ public void filterByUntranslatedAndModifiedPerson() {
FilterConstraintToQuery constraintToQuery =
FilterConstraintToQuery.filterInSingleDocument(
FilterConstraints.builder().keepNone()
.includeStates(ContentStateGroup.builder().removeAll().includeNew(true).build())
.lastModifiedBy("admin").build(),
documentId);
.includeStates(untranslatedOnly)
.lastModifiedBy("admin").build(), documentId);

String hql = constraintToQuery.toEntityQuery();
List<HTextFlow> result = getResultList(hql, constraintToQuery);
List<String> ids = transformToResIds(result);
log.debug("result: {}", ids);
assertThat(ids, Matchers.contains("res4"));
verifyModalNavigationQuery(constraintToQuery, result);
}

@Test
public void filterByContentAndModifiedPersonAndState() {
FilterConstraintToQuery constraintToQuery =
FilterConstraintToQuery.filterInSingleDocument(
FilterConstraints.builder().keepNone()
.checkInSource(true).checkInTarget(true)
.filterBy("source")
.includeStates(untranslatedOnly)
.lastModifiedBy("admin").build(), documentId);

String hql = constraintToQuery.toEntityQuery();
List<HTextFlow> result = getResultList(hql, constraintToQuery);
Expand Down
14 changes: 6 additions & 8 deletions zanata-war/src/test/java/org/zanata/search/FilterConstraintToQueryTest.java
View file
Expand Up @@ -79,8 +79,8 @@ public void testBuildSearchConditionInSource() {

assertThat(
result,
Matchers.equalToIgnoringCase("(("
+ SOURCE_CONTENT_CASE_INSENSITIVE + "))"));
Matchers.equalToIgnoringCase("("
+ SOURCE_CONTENT_CASE_INSENSITIVE + ")"));
}

@Test
Expand Down Expand Up @@ -113,9 +113,9 @@ public void testBuildSearchConditionInBoth() {

assertThat(
result,
Matchers.equalToIgnoringCase("(("
Matchers.equalToIgnoringCase("("
+ SOURCE_CONTENT_CASE_INSENSITIVE
+ ") OR EXISTS ( FROM HTextFlowTarget WHERE ("
+ " OR EXISTS ( FROM HTextFlowTarget WHERE ("
+ TARGET_CONTENT_CASE_INSENSITIVE + " AND textFlow=tf and locale=:locale)))"));
}

Expand Down Expand Up @@ -233,9 +233,7 @@ public void testToHQLWithSearchAndStateCondition() {
AND
(
(
(
lower(tf.content0) LIKE :searchString
)
lower(tf.content0) LIKE :searchString
)
OR EXISTS ( FROM HTextFlowTarget WHERE (
(
Expand Down Expand Up @@ -269,7 +267,7 @@ AND state IN (:contentStateList)
ORDER BY tf.pos
*/
assertThat(result, Matchers.equalTo(
"SELECT distinct tf FROM HTextFlow tf LEFT JOIN tf.targets tfts WITH tfts.index=:locale WHERE (tf.obsolete=0 AND tf.document.id=:documentId AND (((lower(tf.content0) like :searchString)) OR EXISTS ( FROM HTextFlowTarget WHERE ((lower(content0) like :searchString) AND textFlow=tf AND locale=:locale))) AND ( EXISTS ( FROM HTextFlowTarget WHERE ((textFlow=tf AND locale=:locale) AND state in (:contentStateList))) OR (:locale not in indices(tf.targets) AND (lower(tf.content0) like :searchString)))) ORDER BY tf.pos"));
"SELECT distinct tf FROM HTextFlow tf LEFT JOIN tf.targets tfts WITH tfts.index=:locale WHERE (tf.obsolete=0 AND tf.document.id=:documentId AND ((lower(tf.content0) like :searchString) OR EXISTS ( FROM HTextFlowTarget WHERE ((lower(content0) like :searchString) AND textFlow=tf AND locale=:locale))) AND ( EXISTS ( FROM HTextFlowTarget WHERE ((textFlow=tf AND locale=:locale) AND state in (:contentStateList))) OR (:locale not in indices(tf.targets) AND (lower(tf.content0) like :searchString)))) ORDER BY tf.pos"));
}

@Test
Expand Down
3 changes: 3 additions & 0 deletions zanata-war/src/test/resources/log4j.xml
View file
Expand Up @@ -35,6 +35,9 @@
<logger name="gwt-log">
<level value="OFF" />
</logger>
<logger name="org.zanata.search">
<level value="DEBUG" />
</logger>
<root>
<level value="ERROR" />
<appender-ref ref="STDOUT" />
Expand Down

0 comments on commit de82469

Please sign in to comment.