From 887708a9e3520f628bb3354a7aa9f999926c5d7f Mon Sep 17 00:00:00 2001 From: David Mason Date: Fri, 5 Jul 2013 13:08:04 +1000 Subject: [PATCH 01/26] remove unused imports, injections and annotations --- .../webtrans/server/rpc/GetDocumentListHandler.java | 8 -------- .../webtrans/server/rpc/GetGlossaryDetailsHandler.java | 1 - .../webtrans/server/rpc/GetTransMemoryDetailsHandler.java | 1 - .../zanata/webtrans/server/rpc/GetTransMemoryHandler.java | 4 ---- .../webtrans/server/rpc/GetTranslatorListHandler.java | 2 -- .../webtrans/server/rpc/GetValidationRulesHandler.java | 3 --- .../zanata/webtrans/server/rpc/ReplaceTextHandler.java | 2 -- .../server/rpc/RevertTransUnitUpdatesHandler.java | 2 -- .../webtrans/server/rpc/RunDocValidationHandler.java | 4 ---- .../webtrans/server/rpc/UpdateGlossaryTermHandler.java | 3 --- .../webtrans/server/rpc/UpdateTransUnitHandler.java | 2 -- 11 files changed, 32 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetDocumentListHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetDocumentListHandler.java index 207690794a..5979668846 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetDocumentListHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetDocumentListHandler.java @@ -14,13 +14,11 @@ import org.jboss.seam.annotations.Scope; import org.zanata.common.ProjectType; import org.zanata.dao.DocumentDAO; -import org.zanata.dao.TextFlowTargetDAO; import org.zanata.model.HDocument; import org.zanata.model.HPerson; import org.zanata.model.HProjectIteration; import org.zanata.security.ZanataIdentity; import org.zanata.service.TranslationFileService; -import org.zanata.service.TranslationStateCache; import org.zanata.webtrans.server.ActionHandlerFor; import org.zanata.webtrans.shared.model.AuditInfo; import org.zanata.webtrans.shared.model.DocumentId; @@ -40,14 +38,8 @@ public class GetDocumentListHandler extends AbstractActionHandler { - @In - private ZanataIdentity identity; @In private ValidationService validationServiceImpl; diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/ReplaceTextHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/ReplaceTextHandler.java index 389f726b7a..63d3e4a9c5 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/ReplaceTextHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/ReplaceTextHandler.java @@ -28,8 +28,6 @@ import org.jboss.seam.annotations.In; import org.jboss.seam.annotations.Name; import org.jboss.seam.annotations.Scope; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.zanata.service.SecurityService; import org.zanata.webtrans.server.ActionHandlerFor; import org.zanata.webtrans.shared.model.TransUnitUpdateRequest; diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/RevertTransUnitUpdatesHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/RevertTransUnitUpdatesHandler.java index 52aaea2e4e..d4a47ef200 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/RevertTransUnitUpdatesHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/RevertTransUnitUpdatesHandler.java @@ -22,7 +22,6 @@ import java.util.List; -import lombok.extern.slf4j.Slf4j; import net.customware.gwt.dispatch.server.ExecutionContext; import net.customware.gwt.dispatch.shared.ActionException; @@ -54,7 +53,6 @@ @Name("webtrans.gwt.RevertTransUnitUpdatesHandler") @Scope(ScopeType.STATELESS) @ActionHandlerFor(RevertTransUnitUpdates.class) -@Slf4j public class RevertTransUnitUpdatesHandler extends AbstractActionHandler { @In diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/RunDocValidationHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/RunDocValidationHandler.java index 764256bfcb..d1211c6ae1 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/RunDocValidationHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/RunDocValidationHandler.java @@ -10,7 +10,6 @@ import org.jboss.seam.annotations.In; import org.jboss.seam.annotations.Name; import org.jboss.seam.annotations.Scope; -import org.zanata.dao.LocaleDAO; import org.zanata.service.ValidationService; import org.zanata.webtrans.server.ActionHandlerFor; import org.zanata.webtrans.shared.model.DocumentId; @@ -25,9 +24,6 @@ public class RunDocValidationHandler extends AbstractActionHandler { @In From 943d32bb4e86b98451a7977c5707a0d014fd798d Mon Sep 17 00:00:00 2001 From: David Mason Date: Fri, 5 Jul 2013 15:22:18 +1000 Subject: [PATCH 02/26] Extract builder and states class from FilterConstraints --- .../main/java/org/zanata/dao/TextFlowDAO.java | 16 +- .../java/org/zanata/search/ActiveStates.java | 146 +++++++ .../search/FilterConstraintToQuery.java | 8 +- .../org/zanata/search/FilterConstraints.java | 367 ++++++------------ .../impl/TextFlowSearchServiceImpl.java | 10 +- .../rpc/GetProjectTransUnitListsHandler.java | 15 +- .../server/rpc/GetTransUnitListHandler.java | 15 +- .../rpc/GetTransUnitsNavigationService.java | 14 +- .../webtrans/shared/rpc/GetTransUnitList.java | 1 + .../shared/rpc/GetTransUnitsNavigation.java | 2 + .../java/org/zanata/dao/TextFlowDAOTest.java | 70 ++-- .../search/FilterConstraintToQueryTest.java | 54 ++- .../impl/TextFlowSearchServiceImplTest.java | 6 +- 13 files changed, 399 insertions(+), 325 deletions(-) create mode 100644 zanata-war/src/main/java/org/zanata/search/ActiveStates.java diff --git a/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java b/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java index 41fce047e3..467e2c6f6f 100644 --- a/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java +++ b/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java @@ -41,6 +41,7 @@ import org.zanata.model.HDocument; import org.zanata.model.HLocale; import org.zanata.model.HTextFlow; +import org.zanata.search.ActiveStates; import org.zanata.search.FilterConstraintToQuery; import org.zanata.search.FilterConstraints; import org.zanata.webtrans.shared.model.DocumentId; @@ -70,7 +71,6 @@ public TextFlowDAO(Session session) super(HTextFlow.class, session); } - @SuppressWarnings("unchecked") public OpenBitSet findIdsWithTranslations(LocaleId locale) { Query q = getSession().getNamedQuery(HTextFlow.QUERY_TRANSLATED_TEXTFLOWIDS); @@ -169,11 +169,11 @@ public List getNavigationByDocumentId(Long documentId, HLocale hLocal * @param alias HTextFlowTarget alias * @return '1' if accept all status or a SQL condition clause with target content state conditions in parentheses '()' joined by 'or' */ - protected static String buildContentStateCondition(FilterConstraints filterConstraints, String alias) + protected static String buildContentStateCondition(FilterConstraints constraints, String alias) { - - if (filterConstraints.isTranslatedIncluded() == filterConstraints.isFuzzyIncluded() - && filterConstraints.isTranslatedIncluded() == filterConstraints.isNewIncluded()) + + ActiveStates includedStates = constraints.getIncludedStates(); + if (includedStates.hasAllStates() || includedStates.hasNoStates()) { return "1"; } @@ -181,17 +181,17 @@ protected static String buildContentStateCondition(FilterConstraints filterConst builder.append("("); List conditions = Lists.newArrayList(); final String column = alias + ".state"; - if (filterConstraints.isTranslatedIncluded()) + if (constraints.getIncludedStates().isTranslatedOn()) { conditions.add(column + "=2"); // Translated conditions.add(column + "=3"); // Approved } - if (filterConstraints.isFuzzyIncluded()) + if (constraints.getIncludedStates().isFuzzyOn()) { conditions.add(column + "=1"); // Fuzzy conditions.add(column + "=4"); // Rejected } - if (filterConstraints.isNewIncluded()) + if (constraints.getIncludedStates().isNewOn()) { conditions.add(column + "=0 or " + column + " is null"); } diff --git a/zanata-war/src/main/java/org/zanata/search/ActiveStates.java b/zanata-war/src/main/java/org/zanata/search/ActiveStates.java new file mode 100644 index 0000000000..eeabfa61bb --- /dev/null +++ b/zanata-war/src/main/java/org/zanata/search/ActiveStates.java @@ -0,0 +1,146 @@ +package org.zanata.search; + +import java.util.List; + +import org.zanata.common.ContentState; + +import com.google.common.collect.Lists; + +import lombok.Getter; +import lombok.AllArgsConstructor; +import lombok.ToString; + +@ToString +@AllArgsConstructor +@Getter +public class ActiveStates +{ + private boolean newOn; + private boolean fuzzyOn; + private boolean translatedOn; + private boolean approvedOn; + private boolean rejectedOn; + + /** + * @return a Builder with all states on by default + */ + public static Builder builder() + { + return new Builder(); + } + + public boolean hasAllStates() + { + return newOn && fuzzyOn && translatedOn && approvedOn && rejectedOn; + } + + public boolean hasNoStates() + { + return !(newOn || fuzzyOn || translatedOn || approvedOn || rejectedOn); + } + + public List asList() + { + List result = Lists.newArrayList(); + if (newOn) + { + result.add(ContentState.New); + } + if (fuzzyOn) + { + result.add(ContentState.NeedReview); + } + if (translatedOn) + { + result.add(ContentState.Translated); + } + if (approvedOn) + { + result.add(ContentState.Approved); + } + if (rejectedOn) + { + result.add(ContentState.Rejected); + } + return result; + } + + public static class Builder + { + private boolean newOn; + private boolean fuzzyOn; + private boolean translatedOn; + private boolean approvedOn; + private boolean rejectedOn; + + public Builder() + { + allOn(); + } + + public ActiveStates build() + { + return new ActiveStates(newOn, fuzzyOn, translatedOn, approvedOn, rejectedOn); + } + + public Builder allOn() + { + this.newOn = true; + this.fuzzyOn = true; + this.translatedOn = true; + this.approvedOn = true; + this.rejectedOn = true; + return this; + } + + public Builder allOff() + { + this.newOn = false; + this.fuzzyOn = false; + this.translatedOn = false; + this.approvedOn = false; + this.rejectedOn = false; + return this; + } + + public Builder fromStates(ActiveStates states) + { + this.newOn = states.newOn; + this.fuzzyOn = states.fuzzyOn; + this.translatedOn = states.translatedOn; + this.approvedOn = states.approvedOn; + this.rejectedOn = states.rejectedOn; + return this; + } + + public Builder setNewOn(boolean on) + { + newOn = on; + return this; + } + + public Builder setFuzzyOn(boolean on) + { + fuzzyOn = on; + return this; + } + + public Builder setTranslatedOn(boolean on) + { + translatedOn = on; + return this; + } + + public Builder setApprovedOn(boolean on) + { + approvedOn = on; + return this; + } + + public Builder setRejectedOn(boolean on) + { + rejectedOn = on; + return this; + } + } +} diff --git a/zanata-war/src/main/java/org/zanata/search/FilterConstraintToQuery.java b/zanata-war/src/main/java/org/zanata/search/FilterConstraintToQuery.java index 3d12655843..a6cf05e406 100644 --- a/zanata-war/src/main/java/org/zanata/search/FilterConstraintToQuery.java +++ b/zanata-war/src/main/java/org/zanata/search/FilterConstraintToQuery.java @@ -140,7 +140,7 @@ private Criterion contentsCriterion(String alias) protected String buildStateCondition() { - if (constraints.isAllStateIncluded()) + if (constraints.getIncludedStates().hasAllStates()) { return null; } @@ -148,7 +148,7 @@ protected String buildStateCondition() String stateInListWhereClause = and(textFlowAndLocaleRestriction.toString(), String.format("state in (%s)", STATE_LIST_PLACEHOLDER)); String stateInListCondition = QueryBuilder.exists().from("HTextFlowTarget").where(stateInListWhereClause).toQueryString(); - if (constraints.isNewIncluded()) + if (constraints.getIncludedStates().isNewOn()) { String nullTargetCondition = String.format("%s not in indices(tf.targets)", LOCALE_PLACEHOLDER); if (hasSearch && constraints.isSearchInSource()) @@ -175,9 +175,9 @@ public Query setQueryParameters(Query textFlowQuery, HLocale hLocale) { textFlowQuery.setParameter(SEARCH_NAMED_PARAM, searchString); } - if (!constraints.isAllStateIncluded()) + if (!constraints.getIncludedStates().hasAllStates()) { - textFlowQuery.setParameterList(STATE_LIST_NAMED_PARAM, constraints.getContentStateAsList()); + textFlowQuery.setParameterList(STATE_LIST_NAMED_PARAM, constraints.getIncludedStates().asList()); } return textFlowQuery; } diff --git a/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java b/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java index f456ea79b1..9e53cfc2f2 100644 --- a/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java +++ b/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java @@ -20,305 +20,192 @@ */ package org.zanata.search; -//TODO could make a hierarchy of filter constraints, which could be consumed -//by a hierarchy of filters. For the moment there aren't enough uses to -//justify this. May want to add document(someDocument) to these constraints +//TODO May want to add document(someDocument) to these constraints //so that only one search method is needed on the interface. -import java.util.List; +import lombok.Getter; -import org.zanata.common.ContentState; import com.google.common.base.Objects; -import com.google.common.collect.Lists; /** * Specifies a set of constraints to be applied by a filter. * * @author David Mason, damason@redhat.com */ +@Getter public class FilterConstraints { private String searchString; - private boolean isCaseSensitive; - private boolean searchInSource; private boolean searchInTarget; + private ActiveStates includedStates; - private boolean newIncluded; - private boolean fuzzyIncluded; - private boolean translatedIncluded; - private boolean approvedIncluded; - private boolean rejectedIncluded; - - //TODO rhbz953734 - need to consider other content state?? - private FilterConstraints(String searchString, boolean caseSensitive, boolean searchInSource, boolean searchInTarget, boolean newIncluded, boolean fuzzyIncluded, boolean translatedIncluded, boolean approvedIncluded, boolean rejectedIncluded) + private FilterConstraints(String searchString, boolean caseSensitive, + boolean searchInSource, boolean searchInTarget, + ActiveStates includedStates) { this.searchString = searchString; this.isCaseSensitive = caseSensitive; this.searchInSource = searchInSource; this.searchInTarget = searchInTarget; - this.newIncluded = newIncluded; - this.fuzzyIncluded = fuzzyIncluded; - this.translatedIncluded = translatedIncluded; - this.approvedIncluded = approvedIncluded; - this.rejectedIncluded = rejectedIncluded; - } - - /** - * Create a chainable filter constraints that specifies a case-insensitive - * search in both source and target, including all content states. - * - * Use chainable methods to alter these constraints - * - * @param searchString the string to search for in source and target - * @return the new {@link FilterConstraints} - */ - public static FilterConstraints filterBy(String searchString) - { - return new FilterConstraints(searchString, false, true, true, true, true, true, true, true); + this.includedStates = includedStates; } - public static FilterConstraints keepAll() - { - return new FilterConstraints("", false, true, true, true, true, true, true, true); - } - - public static FilterConstraints keepNone() + public static Builder builder() { - return new FilterConstraints("", false, false, false, false, false, false, false, false); + return new Builder(); } - - //chainable setters - // TODO use builder instead - - /** - * Specify that search string does not require the same case as content to be considered a match - * - * @return this object for chaining - */ - public FilterConstraints ignoreCase() - { - isCaseSensitive = false; - return this; - } - - /** - * Specify that search string must have the same case as content to be considered a match - * - * @return this object for chaining - */ - public FilterConstraints matchCase() - { - isCaseSensitive = true; - return this; - } - - /** - * Specify search case-sensitivity - * - * @param caseSensitive true if the search string must have the same case as content to be considered a match - * @return this object for chaining - */ - public FilterConstraints caseSensitive(boolean caseSensitive) - { - this.isCaseSensitive = caseSensitive; - return this; - } - - /** - * Return text flows that match the search string in their target content - * - * @return this object for chaining - */ - public FilterConstraints filterTarget() - { - searchInTarget = true; - return this; - } - - /** - * Do not search for the search string in the target - * - * @return this object for chaining - */ - public FilterConstraints ignoreTarget() + @Override + public String toString() { - searchInTarget = false; - return this; + // @formatter:off + return Objects.toStringHelper(this). + add("searchString", searchString). + add("isCaseSensitive", isCaseSensitive). + add("searchInSource", searchInSource). + add("searchInTarget", searchInTarget). + add("includedStates", includedStates). + toString(); + // @formatter:on } - /** - * Return text flows that match the search string in their source content - * - * @return this object for chaining - */ - public FilterConstraints filterSource() + public static class Builder { - searchInSource = true; - return this; - } + private String searchString; + private boolean caseSensitive; + private boolean searchInSource; + private boolean searchInTarget; + private ActiveStates.Builder states; - /** - * Do not search for the search string in the source - * - * @return this object for chaining - */ - public FilterConstraints ignoreSource() - { - searchInSource = false; - return this; - } + public Builder() + { + states = ActiveStates.builder(); + setKeepAll(); + } - /** - * Do not return any text flows with New targets - * - * @return this object for chaining - */ - public FilterConstraints excludeNew() - { - newIncluded = false; - return this; - } + public FilterConstraints build() + { + return new FilterConstraints(searchString, caseSensitive, + searchInSource, searchInTarget, states.build()); + } - /** - * Do not return any text flows with Fuzzy targets - * - * @return this object for chaining - */ - public FilterConstraints excludeFuzzy() - { - fuzzyIncluded = false; - return this; - } + public Builder keepAll() + { + setKeepAll(); + return this; + } - /** - * Do not return any text flows with Translated targets - * - * @return this object for chaining - */ - public FilterConstraints excludeTranslated() - { - translatedIncluded = false; - return this; - } - - public FilterConstraints excludeApproved() - { - approvedIncluded = false; - return this; - } - - public FilterConstraints excludeRejected() - { - rejectedIncluded = false; - return this; - } + private void setKeepAll() + { + searchString = ""; + caseSensitive = false; + searchInSource = true; + searchInTarget = true; + states.allOn(); + } + public Builder keepNone() + { + searchString = ""; + caseSensitive = false; + searchInSource = false; + searchInTarget = false; + states.allOff(); + return this; + } - //getters + public Builder filterBy(String searchString) + { + this.searchString = searchString; + return this; + } - public String getSearchString() - { - return searchString; - } + public Builder caseSensitive(boolean caseSensitive) + { + this.caseSensitive = caseSensitive; + return this; + } - public boolean isCaseSensitive() - { - return this.isCaseSensitive; - } + public Builder checkInSource(boolean check) + { + searchInSource = check; + return this; + } - public boolean isSearchInSource() - { - return searchInSource; - } + public Builder checkInTarget(boolean check) + { + searchInTarget = check; + return this; + } - public boolean isSearchInTarget() - { - return searchInTarget; - } + public Builder includeStates(ActiveStates states) + { + this.states.fromStates(states); + return this; + } - public boolean isNewIncluded() - { - return newIncluded; - } + public Builder includeNew() + { + states.setNewOn(true); + return this; + } - public boolean isFuzzyIncluded() - { - return fuzzyIncluded; - } + public Builder excludeNew() + { + states.setNewOn(false); + return this; + } - public boolean isTranslatedIncluded() - { - return translatedIncluded; - } - - public boolean isApprovedIncluded() - { - return approvedIncluded; - } - - public boolean isRejectedIncluded() - { - return rejectedIncluded; - } + public Builder includeFuzzy() + { + states.setFuzzyOn(true); + return this; + } - public FilterConstraints filterByStatus(boolean newState, boolean fuzzyState, boolean translatedState, boolean approvedState, boolean rejectedState) - { - if (translatedState == fuzzyState && translatedState == newState && translatedState == approvedState && translatedState == rejectedState) + public Builder excludeFuzzy() { - return new FilterConstraints(searchString, isCaseSensitive, isSearchInSource(), isSearchInTarget(), true, true, true, true, true); + states.setFuzzyOn(false); + return this; } - return new FilterConstraints(searchString, isCaseSensitive, isSearchInSource(), isSearchInTarget(), newState, fuzzyState, translatedState, approvedState, rejectedState); - } - public boolean isAllStateIncluded() - { - return translatedIncluded && fuzzyIncluded && newIncluded && approvedIncluded && rejectedIncluded; - } + public Builder includeTranslated() + { + states.setTranslatedOn(true); + return this; + } - public List getContentStateAsList() - { - List result = Lists.newArrayList(); - if (translatedIncluded) + public Builder excludeTranslated() { - result.add(ContentState.Translated); + states.setTranslatedOn(false); + return this; } - if (fuzzyIncluded) + + public Builder includeApproved() { - result.add(ContentState.NeedReview); + states.setApprovedOn(true); + return this; } - if (newIncluded) + + public Builder excludeApproved() { - result.add(ContentState.New); + states.setApprovedOn(false); + return this; } - if (approvedIncluded) + + public Builder includeRejected() { - result.add(ContentState.Approved); + states.setRejectedOn(true); + return this; } - if (rejectedIncluded) + + public Builder excludeRejected() { - result.add(ContentState.Rejected); + states.setRejectedOn(false); + return this; } - return result; - } - @Override - public String toString() - { - // @formatter:off - return Objects.toStringHelper(this). - add("searchString", searchString). - add("isCaseSensitive", isCaseSensitive). - add("searchInSource", searchInSource). - add("searchInTarget", searchInTarget). - add("newIncluded", newIncluded). - add("fuzzyIncluded", fuzzyIncluded). - add("translatedIncluded", translatedIncluded). - add("approvedIncluded", approvedIncluded). - add("rejectedIncluded", rejectedIncluded). - toString(); - // @formatter:on } + } diff --git a/zanata-war/src/main/java/org/zanata/service/impl/TextFlowSearchServiceImpl.java b/zanata-war/src/main/java/org/zanata/service/impl/TextFlowSearchServiceImpl.java index 1bad576876..e31a97de91 100644 --- a/zanata-war/src/main/java/org/zanata/service/impl/TextFlowSearchServiceImpl.java +++ b/zanata-war/src/main/java/org/zanata/service/impl/TextFlowSearchServiceImpl.java @@ -57,6 +57,7 @@ import org.zanata.model.HProjectIteration; import org.zanata.model.HTextFlow; import org.zanata.model.HTextFlowTarget; +import org.zanata.search.ActiveStates; import org.zanata.search.FilterConstraintToQuery; import org.zanata.search.FilterConstraints; import org.zanata.service.LocaleService; @@ -139,7 +140,8 @@ private List findTextFlowsByDocumentPaths(WorkspaceId workspace, List return Collections.emptyList(); } - if (!constraints.isNewIncluded() && !constraints.isFuzzyIncluded() && !constraints.isTranslatedIncluded()) + ActiveStates includedStates = constraints.getIncludedStates(); + if (!includedStates.isNewOn() && !includedStates.isFuzzyOn() && !includedStates.isTranslatedOn()) { // including nothing return Collections.emptyList(); @@ -298,19 +300,19 @@ private List findTextFlowsWithHibernateSearch(String projectSlug, Str } targetQuery.add(localeQuery, Occur.MUST); - if (!constraints.isTranslatedIncluded()) + if (!constraints.getIncludedStates().isTranslatedOn()) { TermQuery approvedStateQuery = new TermQuery(new Term(IndexFieldLabels.CONTENT_STATE_FIELD, ContentState.Approved.toString())); targetQuery.add(approvedStateQuery, Occur.MUST_NOT); } - if (!constraints.isFuzzyIncluded()) + if (!constraints.getIncludedStates().isFuzzyOn()) { TermQuery approvedStateQuery = new TermQuery(new Term(IndexFieldLabels.CONTENT_STATE_FIELD, ContentState.NeedReview.toString())); targetQuery.add(approvedStateQuery, Occur.MUST_NOT); } - if (!constraints.isNewIncluded()) + if (!constraints.getIncludedStates().isNewOn()) { TermQuery approvedStateQuery = new TermQuery(new Term(IndexFieldLabels.CONTENT_STATE_FIELD, ContentState.New.toString())); targetQuery.add(approvedStateQuery, Occur.MUST_NOT); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetProjectTransUnitListsHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetProjectTransUnitListsHandler.java index 62c1ebde64..549176c1f1 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetProjectTransUnitListsHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetProjectTransUnitListsHandler.java @@ -96,15 +96,12 @@ public GetProjectTransUnitListsResult execute(GetProjectTransUnitLists action, E return new GetProjectTransUnitListsResult(action, docPaths, matchingTUs); } - FilterConstraints filterConstraints = FilterConstraints.filterBy(action.getSearchString()).caseSensitive(action.isCaseSensitive()); - if (!action.isSearchInSource()) - { - filterConstraints.ignoreSource(); - } - if (!action.isSearchInTarget()) - { - filterConstraints.ignoreTarget(); - } + FilterConstraints filterConstraints = FilterConstraints.builder() + .filterBy(action.getSearchString()) + .caseSensitive(action.isCaseSensitive()) + .checkInSource(action.isSearchInSource()) + .checkInTarget(action.isSearchInTarget()) + .build(); List matchingFlows = textFlowSearchServiceImpl.findTextFlows(action.getWorkspaceId(), action.getDocumentPaths(), filterConstraints); log.info("Returned {} results for search", matchingFlows.size()); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java index 0f3ef5bc79..e7404d99fa 100755 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java @@ -35,6 +35,7 @@ import org.zanata.exception.ZanataServiceException; import org.zanata.model.HLocale; import org.zanata.model.HTextFlow; +import org.zanata.search.ActiveStates; import org.zanata.search.FilterConstraints; import org.zanata.security.ZanataIdentity; import org.zanata.service.LocaleService; @@ -136,9 +137,17 @@ private List getTextFlows(GetTransUnitList action, HLocale hLocale, i else { // @formatter:off - FilterConstraints constraints = FilterConstraints - .filterBy(action.getPhrase()).ignoreCase().filterSource().filterTarget() - .filterByStatus(action.isFilterUntranslated(), action.isFilterNeedReview(), action.isFilterTranslated(), action.isFilterApproved(), action.isFilterRejected()); + FilterConstraints constraints = FilterConstraints.builder() + .filterBy(action.getPhrase()) + .caseSensitive(false).checkInSource(true).checkInTarget(true) + .includeStates(ActiveStates.builder() + .setNewOn(action.isFilterUntranslated()) + .setFuzzyOn(action.isFilterNeedReview()) + .setTranslatedOn(action.isFilterTranslated()) + .setApprovedOn(action.isFilterApproved()) + .setRejectedOn(action.isFilterRejected()) + .build()) + .build(); // @formatter:on log.debug("Fetch TransUnits filtered by status and/or search: {}", constraints); if (!hasValidationFilter(action)) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java index c53d85535a..556476bc9b 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java @@ -35,6 +35,7 @@ import org.zanata.model.HLocale; import org.zanata.model.HTextFlow; import org.zanata.model.HTextFlowTarget; +import org.zanata.search.ActiveStates; import org.zanata.search.FilterConstraints; import org.zanata.webtrans.shared.model.TransUnitId; import org.zanata.webtrans.shared.rpc.GetTransUnitsNavigation; @@ -53,7 +54,18 @@ public class GetTransUnitsNavigationService protected GetTransUnitsNavigationResult getNavigationIndexes(GetTransUnitsNavigation action, HLocale hLocale) { - FilterConstraints filterConstraints = FilterConstraints.filterBy(action.getPhrase()).filterSource().filterTarget().filterByStatus(action.isNewState(), action.isFuzzyState(), action.isTranslatedState(), action.isApprovedState(), action.isRejectedState()); + FilterConstraints filterConstraints = FilterConstraints.builder() + .filterBy(action.getPhrase()) + .checkInSource(true).checkInTarget(true) + .includeStates(ActiveStates.builder() + .setNewOn(action.isNewState()) + .setFuzzyOn(action.isFuzzyState()) + .setTranslatedOn(action.isTranslatedState()) + .setApprovedOn(action.isApprovedState()) + .setRejectedOn(action.isRejectedState()) + .build()) + .build(); + List idIndexList = new ArrayList(); Map transIdStateMap = new HashMap(); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java index e6c7d76e8e..f5578f6f98 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java @@ -16,6 +16,7 @@ public class GetTransUnitList extends AbstractWorkspaceAction validationIds; private TransUnitId targetTransUnitId; diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java index 05410230a8..7c050a7f2d 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java @@ -28,6 +28,8 @@ public class GetTransUnitsNavigation { private Long id; private String phrase; + + // FIXME use state object private boolean isFuzzyState, isNewState, isTranslatedState, isApprovedState, isRejectedState; @SuppressWarnings("unused") diff --git a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java index 018457f69e..7a53849be8 100644 --- a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java +++ b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java @@ -67,26 +67,27 @@ private void printTestData() } + // FIXME looks like this test does not take more recently added states into account + // should ensure all states are in test data and check test logic @Test public void canGetAllUntranslatedTextFlowForADocument() { HLocale deLocale = getEm().find(HLocale.class, 3L); log.info("locale: {}", deLocale); - FilterConstraints untranslated = FilterConstraints.keepAll().excludeFuzzy().excludeTranslated(); + FilterConstraints untranslated = FilterConstraints.builder().keepAll().excludeFuzzy().excludeTranslated().build(); List result = dao.getTextFlowByDocumentIdWithConstraints(new DocumentId(1L, ""), deLocale, untranslated, 0, 10); assertThat(result.size(), is(0)); HLocale frLocale = getEm().find(HLocale.class, 6L); result = dao.getTextFlowByDocumentIdWithConstraints(new DocumentId(1L, ""), frLocale, untranslated, 0, 10); assertThat(result.size(), is(1)); - } @Test public void canGetTextFlowWithNullTarget() { HLocale deLocale = getEm().find(HLocale.class, 3L); - FilterConstraints untranslated = FilterConstraints.keepAll().excludeFuzzy().excludeTranslated(); + FilterConstraints untranslated = FilterConstraints.builder().keepAll().excludeFuzzy().excludeTranslated().build(); List result = dao.getTextFlowByDocumentIdWithConstraints(new DocumentId(4L, ""), deLocale, untranslated, 0, 10); assertThat(result, Matchers.hasSize(1)); } @@ -100,55 +101,60 @@ public void canGetTextFlowsByStatus() { DocumentId documentId1 = new DocumentId(1L, ""); // esLocale fuzzy, // frLocale new, deLocale // approved - List result = dao.getTextFlowByDocumentIdWithConstraints(documentId1, esLocale, FilterConstraints.keepAll().excludeTranslated().excludeNew(), 0, 10); + List result = dao.getTextFlowByDocumentIdWithConstraints(documentId1, esLocale, + FilterConstraints.builder().keepAll().excludeTranslated().excludeNew().build(), 0, 10); assertThat(result, Matchers.hasSize(1)); - result = dao.getTextFlowByDocumentIdWithConstraints(documentId1, frLocale, FilterConstraints.keepAll().excludeFuzzy(), 0, 10); + result = dao.getTextFlowByDocumentIdWithConstraints(documentId1, frLocale, + FilterConstraints.builder().keepAll().excludeFuzzy().build(), 0, 10); assertThat(result, Matchers.hasSize(1)); - result = dao.getTextFlowByDocumentIdWithConstraints(documentId1, deLocale, FilterConstraints.keepAll().excludeFuzzy().excludeNew(), 0, 10); + result = dao.getTextFlowByDocumentIdWithConstraints(documentId1, deLocale, + FilterConstraints.builder().keepAll().excludeFuzzy().excludeNew().build(), 0, 10); assertThat(result, Matchers.hasSize(1)); HLocale enUSLocale = getEm().find(HLocale.class, 4L); DocumentId documentId2 = new DocumentId(2L, ""); // all 3 text flows has // en-US fuzzy target - result = dao.getTextFlowByDocumentIdWithConstraints(documentId2, enUSLocale, FilterConstraints.keepAll().excludeTranslated().excludeFuzzy(), 0, 10); + result = dao.getTextFlowByDocumentIdWithConstraints(documentId2, enUSLocale, + FilterConstraints.builder().keepAll().excludeTranslated().excludeFuzzy().build(), 0, 10); assertThat(result, Matchers.empty()); - result = dao.getTextFlowByDocumentIdWithConstraints(documentId2, enUSLocale, FilterConstraints.keepAll().excludeNew(), 0, 10); + result = dao.getTextFlowByDocumentIdWithConstraints(documentId2, enUSLocale, + FilterConstraints.builder().keepAll().excludeNew().build(), 0, 10); assertThat(result, Matchers.hasSize(3)); } + // TODO this should be split into 8 tests @Test public void canBuildContentStateQuery() { // accept all - assertThat(TextFlowDAO.buildContentStateCondition(FilterConstraints.keepAll(), "tft"), Matchers.equalTo("1")); - assertThat(TextFlowDAO.buildContentStateCondition(FilterConstraints.keepNone(), "tft"), Matchers.equalTo("1")); + assertThat(TextFlowDAO.buildContentStateCondition(FilterConstraints.builder().keepAll().build(), "tft"), Matchers.equalTo("1")); + assertThat(TextFlowDAO.buildContentStateCondition(FilterConstraints.builder().keepNone().build(), "tft"), Matchers.equalTo("1")); // single status filter - FilterConstraints filterConstraints = FilterConstraints.keepNone(); - - filterConstraints = filterConstraints.filterByStatus(false, false, true, false, false); - assertThat(TextFlowDAO.buildContentStateCondition(filterConstraints, "tft"), Matchers.equalTo("(tft.state=2 or tft.state=3)")); - - filterConstraints = filterConstraints.filterByStatus(false, true, false, false, false); - assertThat(TextFlowDAO.buildContentStateCondition(filterConstraints, "tft"), Matchers.equalTo("(tft.state=1 or tft.state=4)")); - - filterConstraints = filterConstraints.filterByStatus(true, false, false, false, false); - assertThat(TextFlowDAO.buildContentStateCondition(filterConstraints, "tft"), Matchers.equalTo("(tft.state=0 or tft.state is null)")); + FilterConstraints.Builder constraints = FilterConstraints.builder(); + + constraints.keepNone().includeTranslated(); + assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=2 or tft.state=3)")); + + constraints.keepNone().includeFuzzy(); + assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=1 or tft.state=4)")); + + constraints.keepNone().includeNew(); + assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=0 or tft.state is null)")); // two status - - filterConstraints = filterConstraints.filterByStatus(true, false, true, false, false); - assertThat(TextFlowDAO.buildContentStateCondition(filterConstraints, "tft"), Matchers.equalTo("(tft.state=2 or tft.state=3 or tft.state=0 or tft.state is null)")); - - filterConstraints = filterConstraints.filterByStatus(false, true, true, false, false); - assertThat(TextFlowDAO.buildContentStateCondition(filterConstraints, "tft"), Matchers.equalTo("(tft.state=2 or tft.state=3 or tft.state=1 or tft.state=4)")); - - filterConstraints = filterConstraints.filterByStatus(true, true, false, false, false); - assertThat(TextFlowDAO.buildContentStateCondition(filterConstraints, "tft"), Matchers.equalTo("(tft.state=1 or tft.state=4 or tft.state=0 or tft.state is null)")); + constraints.keepNone().includeNew().includeTranslated(); + assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=2 or tft.state=3 or tft.state=0 or tft.state is null)")); + + constraints.keepNone().includeFuzzy().includeApproved(); + assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=2 or tft.state=3 or tft.state=1 or tft.state=4)")); + + constraints.keepNone().includeNew().includeFuzzy(); + assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=1 or tft.state=4 or tft.state=0 or tft.state is null)")); } @Test @@ -169,7 +175,10 @@ public void testGetTextFlowByDocumentIdWithConstraint() { HLocale deLocale = getEm().find(HLocale.class, 3L); - List result = dao.getTextFlowByDocumentIdWithConstraints(new DocumentId(new Long(4), ""), deLocale, FilterConstraints.filterBy("mssg").excludeTranslated().excludeFuzzy(), 0, 10); + List result = dao.getTextFlowByDocumentIdWithConstraints( + new DocumentId(new Long(4), ""), deLocale, + FilterConstraints.builder().filterBy("mssg").excludeTranslated().excludeFuzzy().build(), + 0, 10); assertThat(result, Matchers.hasSize(1)); } @@ -181,6 +190,5 @@ public void queryTest1() "where (exists (from HTextFlowTarget where textFlow = tf and content0 like '%mssg%'))"; Query query = getSession().createQuery(queryString); List result = query.list(); - } } diff --git a/zanata-war/src/test/java/org/zanata/search/FilterConstraintToQueryTest.java b/zanata-war/src/test/java/org/zanata/search/FilterConstraintToQueryTest.java index a4c9b2a4a1..04108706ff 100644 --- a/zanata-war/src/test/java/org/zanata/search/FilterConstraintToQueryTest.java +++ b/zanata-war/src/test/java/org/zanata/search/FilterConstraintToQueryTest.java @@ -48,7 +48,8 @@ public void beforeMethod() @Test public void testBuildSearchConditionWithNothingToSearch() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.keepAll(), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().keepAll().build(), documentId); String result = constraintToQuery.buildSearchCondition(); @@ -58,7 +59,8 @@ public void testBuildSearchConditionWithNothingToSearch() @Test public void testBuildSearchConditionInSource() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.filterBy("FiLe").ignoreTarget().filterSource(), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().filterBy("FiLe").checkInTarget(false).checkInSource(true).build(), documentId); String result = constraintToQuery.buildSearchCondition(); @@ -68,7 +70,8 @@ public void testBuildSearchConditionInSource() @Test public void testBuildSearchConditionInTarget() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.filterBy("FiLe").ignoreSource().filterTarget(), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().filterBy("FiLe").checkInSource(false).checkInTarget(true).build(), documentId); String result = constraintToQuery.buildSearchCondition(); @@ -79,7 +82,8 @@ public void testBuildSearchConditionInTarget() @Test public void testBuildSearchConditionInBoth() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.filterBy("FiLe"), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().filterBy("FiLe").build(), documentId); String result = constraintToQuery.buildSearchCondition(); @@ -89,7 +93,8 @@ public void testBuildSearchConditionInBoth() @Test public void testCaseSensitiveSearch() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.filterBy("FiLe").caseSensitive(true), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().filterBy("FiLe").caseSensitive(true).build(), documentId); String result = constraintToQuery.buildSearchCondition(); @@ -99,7 +104,8 @@ public void testCaseSensitiveSearch() @Test public void testBuildStateConditionWithAllState() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.keepAll(), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().keepAll().build(), documentId); String result = constraintToQuery.buildSearchCondition(); @@ -109,7 +115,8 @@ public void testBuildStateConditionWithAllState() @Test public void testBuildStateConditionWithoutUntranslatedState() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.keepAll().excludeNew(), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().keepAll().excludeNew().build(), documentId); String result = constraintToQuery.buildStateCondition(); @@ -119,7 +126,8 @@ public void testBuildStateConditionWithoutUntranslatedState() @Test public void testBuildStateConditionWithUntranslatedStateButNoSearch() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.keepAll().excludeTranslated(), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().keepAll().excludeTranslated().build(), documentId); String result = constraintToQuery.buildStateCondition(); @@ -136,7 +144,8 @@ public void testBuildStateConditionWithUntranslatedStateButNoSearch() @Test public void testBuildStateConditionWithUntranslatedStateAndSearch() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.filterBy("blah").excludeTranslated(), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().filterBy("blah").excludeTranslated().build(), documentId); String result = constraintToQuery.buildStateCondition(); @@ -154,7 +163,8 @@ public void testBuildStateConditionWithUntranslatedStateAndSearch() @Test public void testToHQLWithNoCondition() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.keepAll(), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().keepAll().build(), documentId); String result = constraintToQuery.toHQL(); @@ -164,7 +174,8 @@ public void testToHQLWithNoCondition() @Test public void testToHQLWithNoConditionForMultipleDocuments() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInMultipleDocuments(FilterConstraints.keepAll(), Lists.newArrayList(1L)); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInMultipleDocuments( + FilterConstraints.builder().keepAll().build(), Lists.newArrayList(1L)); String result = constraintToQuery.toHQL(); @@ -174,7 +185,8 @@ public void testToHQLWithNoConditionForMultipleDocuments() @Test public void testToHQLWithSearchAndStateCondition() { - FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(FilterConstraints.filterBy("FiLe").excludeTranslated(), documentId); + FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument( + FilterConstraints.builder().filterBy("FiLe").excludeTranslated().build(), documentId); String result = constraintToQuery.toHQL(); log.info("hql: {}", result); @@ -201,7 +213,7 @@ public void testToHQLWithSearchAndStateCondition() @Test public void testSetParametersForQuery() { - FilterConstraints constraints = FilterConstraints.filterBy("file").excludeTranslated(); + FilterConstraints constraints = FilterConstraints.builder().filterBy("file").excludeTranslated().build(); FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(constraints, documentId); constraintToQuery.setQueryParameters(query, hLocale); @@ -209,14 +221,14 @@ public void testSetParametersForQuery() verify(query).setParameter(FilterConstraintToQuery.DOC_ID_NAMED_PARAM, documentId.getId()); verify(query).setParameter(FilterConstraintToQuery.LOCALE_NAMED_PARAM, hLocale.getId()); verify(query).setParameter(FilterConstraintToQuery.SEARCH_NAMED_PARAM, "%file%"); - verify(query).setParameterList(FilterConstraintToQuery.STATE_LIST_NAMED_PARAM, constraints.getContentStateAsList()); + verify(query).setParameterList(FilterConstraintToQuery.STATE_LIST_NAMED_PARAM, constraints.getIncludedStates().asList()); verifyNoMoreInteractions(query); } @Test public void testSetParametersForQueryWithMultipleDocuments() { - FilterConstraints constraints = FilterConstraints.filterBy("file").excludeTranslated(); + FilterConstraints constraints = FilterConstraints.builder().filterBy("file").excludeTranslated().build(); List docIdList = Lists.newArrayList(1L, 2L); FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInMultipleDocuments(constraints, docIdList); @@ -225,28 +237,28 @@ public void testSetParametersForQueryWithMultipleDocuments() verify(query).setParameterList(FilterConstraintToQuery.DOC_IDS_LIST_NAMED_PARAM, docIdList); verify(query).setParameter(FilterConstraintToQuery.LOCALE_NAMED_PARAM, hLocale.getId()); verify(query).setParameter(FilterConstraintToQuery.SEARCH_NAMED_PARAM, "%file%"); - verify(query).setParameterList(FilterConstraintToQuery.STATE_LIST_NAMED_PARAM, constraints.getContentStateAsList()); + verify(query).setParameterList(FilterConstraintToQuery.STATE_LIST_NAMED_PARAM, constraints.getIncludedStates().asList()); verifyNoMoreInteractions(query); } @Test public void testSetParametersForQueryWithNoSearch() { - FilterConstraints constraints = FilterConstraints.keepAll().excludeTranslated(); + FilterConstraints constraints = FilterConstraints.builder().keepAll().excludeTranslated().build(); FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(constraints, documentId); constraintToQuery.setQueryParameters(query, hLocale); verify(query).setParameter(FilterConstraintToQuery.DOC_ID_NAMED_PARAM, documentId.getId()); verify(query).setParameter(FilterConstraintToQuery.LOCALE_NAMED_PARAM, hLocale.getId()); - verify(query).setParameterList(FilterConstraintToQuery.STATE_LIST_NAMED_PARAM, constraints.getContentStateAsList()); + verify(query).setParameterList(FilterConstraintToQuery.STATE_LIST_NAMED_PARAM, constraints.getIncludedStates().asList()); verifyNoMoreInteractions(query); } @Test public void testSetParametersForQueryWithNoStateFilter() { - FilterConstraints constraints = FilterConstraints.filterBy("FiLe"); + FilterConstraints constraints = FilterConstraints.builder().filterBy("FiLe").build(); FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(constraints, documentId); constraintToQuery.setQueryParameters(query, hLocale); @@ -260,7 +272,7 @@ public void testSetParametersForQueryWithNoStateFilter() @Test public void testSetParametersForQueryWithSearchCaseSensitive() { - FilterConstraints constraints = FilterConstraints.filterBy("FiLe").caseSensitive(true); + FilterConstraints constraints = FilterConstraints.builder().filterBy("FiLe").caseSensitive(true).build(); FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(constraints, documentId); constraintToQuery.setQueryParameters(query, hLocale); @@ -274,7 +286,7 @@ public void testSetParametersForQueryWithSearchCaseSensitive() @Test public void testSetParametersForQueryWithSearchTermAsPercent() { - FilterConstraints constraints = FilterConstraints.filterBy("% blah blah %").caseSensitive(true); + FilterConstraints constraints = FilterConstraints.builder().filterBy("% blah blah %").caseSensitive(true).build(); FilterConstraintToQuery constraintToQuery = FilterConstraintToQuery.filterInSingleDocument(constraints, documentId); constraintToQuery.setQueryParameters(query, hLocale); diff --git a/zanata-war/src/test/java/org/zanata/service/impl/TextFlowSearchServiceImplTest.java b/zanata-war/src/test/java/org/zanata/service/impl/TextFlowSearchServiceImplTest.java index 0dfea64cc1..0df0720280 100644 --- a/zanata-war/src/test/java/org/zanata/service/impl/TextFlowSearchServiceImplTest.java +++ b/zanata-war/src/test/java/org/zanata/service/impl/TextFlowSearchServiceImplTest.java @@ -24,16 +24,13 @@ import org.zanata.service.TextFlowSearchService; import org.zanata.webtrans.shared.model.WorkspaceId; -import lombok.extern.slf4j.Slf4j; import static org.hamcrest.MatcherAssert.*; import static org.mockito.Mockito.*; -import static org.mockito.Mockito.when; /** * @author Patrick Huang pahuang@redhat.com */ @Test(groups = { "jpa-tests" }) -@Slf4j public class TextFlowSearchServiceImplTest extends ZanataDbunitJpaTest { private TextFlowSearchService service; @@ -69,7 +66,8 @@ public void beforeMethod() @Test public void testFindTextFlows() throws Exception { - List result = service.findTextFlows(workspaceId, FilterConstraints.filterBy("file")); + List result = service.findTextFlows(workspaceId, + FilterConstraints.builder().filterBy("file").build()); assertThat(result.size(), Matchers.equalTo(7)); } From dd5806758b5a20725c035f1ab0051b8c32ad5837 Mon Sep 17 00:00:00 2001 From: David Mason Date: Fri, 5 Jul 2013 16:40:50 +1000 Subject: [PATCH 03/26] Use ActiveStates in search and navigation actions --- .../server/rpc/GetTransUnitListHandler.java | 13 +--- .../rpc/GetTransUnitsNavigationService.java | 9 +-- .../webtrans/shared/rpc/GetTransUnitList.java | 60 ++++++++++--------- .../shared/rpc/GetTransUnitsNavigation.java | 57 +++++++----------- 4 files changed, 55 insertions(+), 84 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java index e7404d99fa..fe76eb80dd 100755 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java @@ -88,7 +88,7 @@ public GetTransUnitListResult execute(GetTransUnitList action, ExecutionContext GetTransUnitsNavigationResult navigationResult = null; if (action.isNeedReloadIndex()) { - GetTransUnitsNavigation getTransUnitsNavigation = new GetTransUnitsNavigation(action.getDocumentId().getId(), action.getPhrase(), action.isFilterUntranslated(), action.isFilterNeedReview(), action.isFilterTranslated(), action.isFilterApproved(), action.isFilterRejected()); + GetTransUnitsNavigation getTransUnitsNavigation = new GetTransUnitsNavigation(action.getDocumentId().getId(), action.getPhrase(), action.getFilterStates()); log.debug("get trans unit navigation action: {}", getTransUnitsNavigation); navigationResult = getTransUnitsNavigationService.getNavigationIndexes(getTransUnitsNavigation, hLocale); @@ -117,16 +117,13 @@ public GetTransUnitListResult execute(GetTransUnitList action, ExecutionContext private List getTextFlows(GetTransUnitList action, HLocale hLocale, int offset) { List textFlows; - // no status and phrase filter if (!hasStatusAndPhaseFilter(action)) { - // no validation filter log.debug("Fetch TransUnits:*"); if (!hasValidationFilter(action)) { textFlows = textFlowDAO.getTextFlowsByDocumentId(action.getDocumentId(), offset, action.getCount()); } - // has validation filter else { textFlows = textFlowDAO.getAllTextFlowsByDocumentId(action.getDocumentId()); @@ -140,13 +137,7 @@ private List getTextFlows(GetTransUnitList action, HLocale hLocale, i FilterConstraints constraints = FilterConstraints.builder() .filterBy(action.getPhrase()) .caseSensitive(false).checkInSource(true).checkInTarget(true) - .includeStates(ActiveStates.builder() - .setNewOn(action.isFilterUntranslated()) - .setFuzzyOn(action.isFilterNeedReview()) - .setTranslatedOn(action.isFilterTranslated()) - .setApprovedOn(action.isFilterApproved()) - .setRejectedOn(action.isFilterRejected()) - .build()) + .includeStates(action.getFilterStates()) .build(); // @formatter:on log.debug("Fetch TransUnits filtered by status and/or search: {}", constraints); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java index 556476bc9b..f92cfb2608 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java @@ -35,7 +35,6 @@ import org.zanata.model.HLocale; import org.zanata.model.HTextFlow; import org.zanata.model.HTextFlowTarget; -import org.zanata.search.ActiveStates; import org.zanata.search.FilterConstraints; import org.zanata.webtrans.shared.model.TransUnitId; import org.zanata.webtrans.shared.rpc.GetTransUnitsNavigation; @@ -57,13 +56,7 @@ protected GetTransUnitsNavigationResult getNavigationIndexes(GetTransUnitsNaviga FilterConstraints filterConstraints = FilterConstraints.builder() .filterBy(action.getPhrase()) .checkInSource(true).checkInTarget(true) - .includeStates(ActiveStates.builder() - .setNewOn(action.isNewState()) - .setFuzzyOn(action.isFuzzyState()) - .setTranslatedOn(action.isTranslatedState()) - .setApprovedOn(action.isApprovedState()) - .setRejectedOn(action.isRejectedState()) - .build()) + .includeStates(action.getActiveStates()) .build(); List idIndexList = new ArrayList(); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java index f5578f6f98..16a477ac60 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java @@ -2,6 +2,7 @@ import java.util.List; +import org.zanata.search.ActiveStates; import org.zanata.webtrans.client.service.GetTransUnitActionContext; import org.zanata.webtrans.shared.model.DocumentId; import org.zanata.webtrans.shared.model.TransUnitId; @@ -16,37 +17,37 @@ public class GetTransUnitList extends AbstractWorkspaceAction validationIds; private TransUnitId targetTransUnitId; private boolean needReloadIndex = false; - @SuppressWarnings("unused") private GetTransUnitList() { } - private GetTransUnitList(DocumentId id, int offset, int count, String phrase, boolean filterTranslated, boolean filterNeedReview, boolean filterUntranslated, boolean filterApproved, boolean filterRejected, boolean filterHasError, TransUnitId targetTransUnitId, List validationIds) + private GetTransUnitList(GetTransUnitActionContext context) { - this.documentId = id; - this.offset = offset; - this.count = count; - this.phrase = phrase; - this.filterTranslated = filterTranslated; - this.filterNeedReview = filterNeedReview; - this.filterUntranslated = filterUntranslated; - this.filterApproved = filterApproved; - this.filterRejected = filterRejected; - this.filterHasError = filterHasError; - this.targetTransUnitId = targetTransUnitId; - this.validationIds = validationIds; - + documentId = context.getDocument().getId(); + offset = context.getOffset(); + count = context.getCount(); + phrase = context.getFindMessage(); + filterStates = ActiveStates.builder() + .setNewOn(context.isFilterUntranslated()) + .setFuzzyOn(context.isFilterNeedReview()) + .setTranslatedOn(context.isFilterTranslated()) + .setApprovedOn(context.isFilterApproved()) + .setRejectedOn(context.isFilterRejected()) + .build(); + filterHasError = context.isFilterHasError(); + targetTransUnitId = context.getTargetTransUnitId(); + validationIds = context.getValidationIds(); } public static GetTransUnitList newAction(GetTransUnitActionContext context) { - return new GetTransUnitList(context.getDocument().getId(), context.getOffset(), context.getCount(), context.getFindMessage(), context.isFilterTranslated(), context.isFilterNeedReview(), context.isFilterUntranslated(), context.isFilterApproved(), context.isFilterRejected(), context.isFilterHasError(), context.getTargetTransUnitId(), context.getValidationIds()); + return new GetTransUnitList(context); } public boolean isNeedReloadIndex() @@ -80,29 +81,34 @@ public String getPhrase() return this.phrase; } + public ActiveStates getFilterStates() + { + return filterStates; + } + public boolean isFilterTranslated() { - return filterTranslated; + return filterStates.isTranslatedOn(); } public boolean isFilterNeedReview() { - return filterNeedReview; + return filterStates.isFuzzyOn(); } public boolean isFilterUntranslated() { - return filterUntranslated; + return filterStates.isNewOn(); } public boolean isFilterApproved() { - return filterApproved; + return filterStates.isApprovedOn(); } public boolean isFilterRejected() { - return filterRejected; + return filterStates.isRejectedOn(); } public boolean isFilterHasError() @@ -123,7 +129,7 @@ public List getValidationIds() public boolean isAcceptAllStatus() { //all filter options are checked or unchecked - return filterNeedReview == filterTranslated && filterNeedReview == filterUntranslated && filterNeedReview == filterHasError && filterApproved == filterNeedReview && filterRejected == filterNeedReview; + return filterStates.hasNoStates() && !filterHasError || filterStates.hasAllStates() && filterHasError; } @Override @@ -135,11 +141,7 @@ public String toString() add("count", count). add("documentId", documentId). add("phrase", phrase). - add("filterTranslated", filterTranslated). - add("filterNeedReview", filterNeedReview). - add("filterUntranslated", filterUntranslated). - add("filterApproved", filterApproved). - add("filterRejected", filterRejected). + add("filterStates", filterStates). add("filterHasError", filterHasError). add("targetTransUnitId", targetTransUnitId). add("needReloadIndex", needReloadIndex). diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java index 7c050a7f2d..1ca50389b7 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java @@ -21,6 +21,7 @@ package org.zanata.webtrans.shared.rpc; +import org.zanata.search.ActiveStates; import org.zanata.webtrans.client.service.GetTransUnitActionContext; import com.google.common.base.Objects; @@ -29,28 +30,36 @@ public class GetTransUnitsNavigation private Long id; private String phrase; - // FIXME use state object - private boolean isFuzzyState, isNewState, isTranslatedState, isApprovedState, isRejectedState; + private ActiveStates activeStates; @SuppressWarnings("unused") private GetTransUnitsNavigation() { } - public GetTransUnitsNavigation(Long id, String phrase, boolean isNewState, boolean isFuzzyState, boolean isTranslatedState, boolean isApprovedState, boolean isRejectedState) + public GetTransUnitsNavigation(Long id, String phrase, ActiveStates activeStates) { this.id = id; this.phrase = phrase; - this.isNewState = isNewState; - this.isFuzzyState = isFuzzyState; - this.isTranslatedState = isTranslatedState; - this.isApprovedState = isApprovedState; - this.isRejectedState = isRejectedState; + this.activeStates = activeStates; + } + + public GetTransUnitsNavigation(GetTransUnitActionContext context) + { + this(context.getDocument().getId().getId(), + context.getFindMessage(), + ActiveStates.builder() + .setNewOn(context.isFilterUntranslated()) + .setFuzzyOn(context.isFilterNeedReview()) + .setTranslatedOn(context.isFilterTranslated()) + .setApprovedOn(context.isFilterApproved()) + .setRejectedOn(context.isFilterRejected()) + .build()); } public static GetTransUnitsNavigation newAction(GetTransUnitActionContext context) { - return new GetTransUnitsNavigation(context.getDocument().getId().getId(), context.getFindMessage(), context.isFilterUntranslated(), context.isFilterNeedReview(), context.isFilterTranslated(), context.isFilterApproved(), context.isFilterRejected()); + return new GetTransUnitsNavigation(context); } public Long getId() @@ -63,29 +72,9 @@ public String getPhrase() return this.phrase; } - public boolean isFuzzyState() - { - return isFuzzyState; - } - - public boolean isNewState() - { - return isNewState; - } - - public boolean isTranslatedState() - { - return isTranslatedState; - } - - public boolean isApprovedState() - { - return isApprovedState; - } - - public boolean isRejectedState() + public ActiveStates getActiveStates() { - return isRejectedState; + return activeStates; } @Override @@ -95,11 +84,7 @@ public String toString() return Objects.toStringHelper(this). add("id", id). add("phrase", phrase). - add("isFuzzyState", isFuzzyState). - add("isNewState", isNewState). - add("isTranslatedState", isTranslatedState). - add("isApprovedState", isApprovedState). - add("isRejectedState", isRejectedState). + add("activeStates", activeStates). toString(); // @formatter:on } From 29ea2e5dfe7a041aac139acd41955f96be47c75f Mon Sep 17 00:00:00 2001 From: David Mason Date: Mon, 8 Jul 2013 10:25:39 +1000 Subject: [PATCH 04/26] split TextFlowDAOTest.canBuildContentStateQuery() into more atomic tests --- .../java/org/zanata/dao/TextFlowDAOTest.java | 81 ++++++++++++++----- 1 file changed, 60 insertions(+), 21 deletions(-) diff --git a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java index 7a53849be8..ab5fb456be 100644 --- a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java +++ b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java @@ -126,35 +126,74 @@ public void canGetTextFlowsByStatus() { assertThat(result, Matchers.hasSize(3)); } - // TODO this should be split into 8 tests + @Test - public void canBuildContentStateQuery() + public void canBuildAcceptAllQuery() { - // accept all - assertThat(TextFlowDAO.buildContentStateCondition(FilterConstraints.builder().keepAll().build(), "tft"), Matchers.equalTo("1")); - assertThat(TextFlowDAO.buildContentStateCondition(FilterConstraints.builder().keepNone().build(), "tft"), Matchers.equalTo("1")); - - // single status filter - FilterConstraints.Builder constraints = FilterConstraints.builder(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition( + FilterConstraints.builder().keepAll().build(), "tft"); + assertThat("Conditional that accepts all should be '1'", contentStateCondition, is("1")); + } - constraints.keepNone().includeTranslated(); - assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=2 or tft.state=3)")); + @Test + public void canBuildAcceptAllQueryWhenNoStatesSelected() + { + String contentStateCondition = TextFlowDAO.buildContentStateCondition( + FilterConstraints.builder().keepNone().build(), "tft"); + assertThat("Conditional that accepts all should be '1'", contentStateCondition, is("1")); + } - constraints.keepNone().includeFuzzy(); - assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=1 or tft.state=4)")); + @Test + public void canBuildNewOnlyConditional() + { + FilterConstraints constraints = FilterConstraints.builder() + .keepNone().includeNew().build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); + assertThat(contentStateCondition, is("(tft.state=0 or tft.state is null)")); + } - constraints.keepNone().includeNew(); - assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=0 or tft.state is null)")); + @Test + public void canBuildFuzzyOnlyConditional() + { + FilterConstraints constraints = FilterConstraints.builder() + .keepNone().includeFuzzy().build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); + assertThat(contentStateCondition, is("(tft.state=1 or tft.state=4)")); + } - // two status - constraints.keepNone().includeNew().includeTranslated(); - assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=2 or tft.state=3 or tft.state=0 or tft.state is null)")); + @Test + public void canBuildTranslatedOnlyConditional() + { + FilterConstraints constraints = FilterConstraints.builder() + .keepNone().includeTranslated().build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); + assertThat(contentStateCondition, is("(tft.state=2 or tft.state=3)")); + } - constraints.keepNone().includeFuzzy().includeApproved(); - assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=2 or tft.state=3 or tft.state=1 or tft.state=4)")); + @Test + public void canBuildContentStateQuery() + { + FilterConstraints constraints = FilterConstraints.builder() + .keepNone().includeNew().includeFuzzy().build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); + assertThat(contentStateCondition, is("(tft.state=1 or tft.state=4 or tft.state=0 or tft.state is null)")); + } + @Test + public void canBuildNewAndTranslatedConditional() + { + FilterConstraints constraints = FilterConstraints.builder() + .keepNone().includeNew().includeTranslated().build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); + assertThat(contentStateCondition, is("(tft.state=2 or tft.state=3 or tft.state=0 or tft.state is null)")); + } - constraints.keepNone().includeNew().includeFuzzy(); - assertThat(TextFlowDAO.buildContentStateCondition(constraints.build(), "tft"), Matchers.equalTo("(tft.state=1 or tft.state=4 or tft.state=0 or tft.state is null)")); + @Test + public void canBuildFuzzyAndTranslatedConditional() + { + FilterConstraints constraints = FilterConstraints.builder() + .keepNone().includeFuzzy().includeTranslated().build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); + assertThat(contentStateCondition, is("(tft.state=2 or tft.state=3 or tft.state=1 or tft.state=4)")); } @Test From 60ae8a847380d0386a3ab2d42523a0fd578df654 Mon Sep 17 00:00:00 2001 From: David Mason Date: Mon, 8 Jul 2013 11:28:23 +1000 Subject: [PATCH 05/26] split TextFlowDAOTest.canGetTextFlowsByStatus into more atomic tests --- .../java/org/zanata/dao/TextFlowDAOTest.java | 84 ++++++++++++++----- 1 file changed, 64 insertions(+), 20 deletions(-) diff --git a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java index ab5fb456be..4e4ea60561 100644 --- a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java +++ b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java @@ -93,37 +93,79 @@ public void canGetTextFlowWithNullTarget() { } @Test - public void canGetTextFlowsByStatus() { - HLocale esLocale = getEm().find(HLocale.class, 5L); + public void canGetTextFlowsByStatusNotNew() + { + HLocale enUSLocale = getEm().find(HLocale.class, 4L); + // all 3 text flows are fuzzy for en-US in this document + DocumentId documentId2 = new DocumentId(2L, ""); + List result = dao.getTextFlowByDocumentIdWithConstraints(documentId2, enUSLocale, + FilterConstraints.builder().keepAll().excludeNew().build(), 0, 10); + + assertThat(result, Matchers.hasSize(3)); + } + + @Test + public void canGetTextFlowsByStatusNotFuzzy() + { + // frLocale new in this document + DocumentId documentId = new DocumentId(1L, ""); HLocale frLocale = getEm().find(HLocale.class, 6L); - HLocale deLocale = getEm().find(HLocale.class, 3L); + FilterConstraints notFuzzy = FilterConstraints.builder().keepAll().excludeFuzzy().build(); - DocumentId documentId1 = new DocumentId(1L, ""); // esLocale fuzzy, - // frLocale new, deLocale - // approved - List result = dao.getTextFlowByDocumentIdWithConstraints(documentId1, esLocale, - FilterConstraints.builder().keepAll().excludeTranslated().excludeNew().build(), 0, 10); + List result = dao.getTextFlowByDocumentIdWithConstraints(documentId, frLocale, + notFuzzy, 0, 10); assertThat(result, Matchers.hasSize(1)); + } - result = dao.getTextFlowByDocumentIdWithConstraints(documentId1, frLocale, - FilterConstraints.builder().keepAll().excludeFuzzy().build(), 0, 10); + @Test + public void canGetTextFlowsByStatusNotTranslatedNotNew() + { + // esLocale fuzzy in this document + DocumentId documentId = new DocumentId(1L, ""); + HLocale esLocale = getEm().find(HLocale.class, 5L); + FilterConstraints notNewOrTranslated = FilterConstraints.builder().keepAll().excludeTranslated().excludeNew().build(); + + List result = dao.getTextFlowByDocumentIdWithConstraints(documentId, esLocale, + notNewOrTranslated, 0, 10); assertThat(result, Matchers.hasSize(1)); + } - result = dao.getTextFlowByDocumentIdWithConstraints(documentId1, deLocale, - FilterConstraints.builder().keepAll().excludeFuzzy().excludeNew().build(), 0, 10); + @Test + public void canGetTextFlowsByStatusNotFuzzyNotNew() + { + // deLocale approved in this document + DocumentId documentId = new DocumentId(1L, ""); + HLocale deLocale = getEm().find(HLocale.class, 3L); + FilterConstraints notNewOrFuzzy = + FilterConstraints.builder().keepAll().excludeFuzzy().excludeNew().build(); + + List result = dao.getTextFlowByDocumentIdWithConstraints(documentId, deLocale, + notNewOrFuzzy, 0, 10); assertThat(result, Matchers.hasSize(1)); + } + @Test + public void canGetTextFlowsByStatusNotFuzzyNotTranslated() + { HLocale enUSLocale = getEm().find(HLocale.class, 4L); - DocumentId documentId2 = new DocumentId(2L, ""); // all 3 text flows has - // en-US fuzzy target - - result = dao.getTextFlowByDocumentIdWithConstraints(documentId2, enUSLocale, - FilterConstraints.builder().keepAll().excludeTranslated().excludeFuzzy().build(), 0, 10); + // all 3 text flows are fuzzy for en-US in this document + DocumentId documentId2 = new DocumentId(2L, ""); + FilterConstraints notFuzzyOrTranslated = + FilterConstraints.builder().keepAll().excludeTranslated().excludeFuzzy().build(); + List result = dao.getTextFlowByDocumentIdWithConstraints(documentId2, enUSLocale, + notFuzzyOrTranslated, 0, 10); assertThat(result, Matchers.empty()); + } - result = dao.getTextFlowByDocumentIdWithConstraints(documentId2, enUSLocale, - FilterConstraints.builder().keepAll().excludeNew().build(), 0, 10); - assertThat(result, Matchers.hasSize(3)); + @Test + public void thisBreaksForSomeReason() { + // fails regardless of using different documentId, locale or constraints + DocumentId id = new DocumentId(1L, ""); + HLocale locale = getEm().find(HLocale.class, 3L); + FilterConstraints constraints = FilterConstraints.builder().build(); + + dao.getTextFlowByDocumentIdWithConstraints(id, locale, constraints, 0, 10); + dao.getTextFlowByDocumentIdWithConstraints(id, locale, constraints, 0, 10); } @@ -222,6 +264,8 @@ public void testGetTextFlowByDocumentIdWithConstraint() assertThat(result, Matchers.hasSize(1)); } + // What is this testing? I can't tell if it is ensuring that no exception is thrown, + // or if it is just half-written and useless. @Test public void queryTest1() { From 18cef6f7f643957f40943e23a05f13b2b9155269 Mon Sep 17 00:00:00 2001 From: David Mason Date: Mon, 8 Jul 2013 13:30:50 +1000 Subject: [PATCH 06/26] Revert FilterConstraints includeStates(ActiveStates) to old behaviour Old behaviour was to flip all states to on in the case that they were all off. The change should only be temporary since it is surprising - some changes to the code for the editor are necessary before this can be made more sensible. --- .../java/org/zanata/search/FilterConstraints.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java b/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java index 9e53cfc2f2..8f52112a8f 100644 --- a/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java +++ b/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java @@ -142,7 +142,18 @@ public Builder checkInTarget(boolean check) public Builder includeStates(ActiveStates states) { - this.states.fromStates(states); + //FIXME this behaviour is too surprising. + // It exists because the editor UI should show all states when either + // all or none of the states are checked. This logic should just happen + // in the editor backend *before* sending a request to the server. + if (states.hasNoStates()) + { + this.states.allOn(); + } + else + { + this.states.fromStates(states); + } return this; } From 1cbcf3bc6cbc3dbbe66e37dd7607da8e4420e9a0 Mon Sep 17 00:00:00 2001 From: David Mason Date: Mon, 8 Jul 2013 13:58:29 +1000 Subject: [PATCH 07/26] disable mysterious breaking test in TextFlowDAOTest --- zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java index 4e4ea60561..456bf710ba 100644 --- a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java +++ b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java @@ -157,7 +157,7 @@ public void canGetTextFlowsByStatusNotFuzzyNotTranslated() assertThat(result, Matchers.empty()); } - @Test + @Test(enabled = false) public void thisBreaksForSomeReason() { // fails regardless of using different documentId, locale or constraints DocumentId id = new DocumentId(1L, ""); From ba6eb0096e7f71ee9152165de9f8041a30cc99ca Mon Sep 17 00:00:00 2001 From: David Mason Date: Mon, 8 Jul 2013 14:54:31 +1000 Subject: [PATCH 08/26] protect some wrapped builder calls from automatic formatting --- .../webtrans/server/rpc/GetProjectTransUnitListsHandler.java | 2 ++ .../webtrans/server/rpc/GetTransUnitsNavigationService.java | 2 ++ .../java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java | 2 ++ 3 files changed, 6 insertions(+) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetProjectTransUnitListsHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetProjectTransUnitListsHandler.java index 549176c1f1..22ac725a4f 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetProjectTransUnitListsHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetProjectTransUnitListsHandler.java @@ -96,12 +96,14 @@ public GetProjectTransUnitListsResult execute(GetProjectTransUnitLists action, E return new GetProjectTransUnitListsResult(action, docPaths, matchingTUs); } + // @formatter:off FilterConstraints filterConstraints = FilterConstraints.builder() .filterBy(action.getSearchString()) .caseSensitive(action.isCaseSensitive()) .checkInSource(action.isSearchInSource()) .checkInTarget(action.isSearchInTarget()) .build(); + // @formatter:on List matchingFlows = textFlowSearchServiceImpl.findTextFlows(action.getWorkspaceId(), action.getDocumentPaths(), filterConstraints); log.info("Returned {} results for search", matchingFlows.size()); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java index f92cfb2608..5ef402be2b 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitsNavigationService.java @@ -53,11 +53,13 @@ public class GetTransUnitsNavigationService protected GetTransUnitsNavigationResult getNavigationIndexes(GetTransUnitsNavigation action, HLocale hLocale) { + // @formatter:off FilterConstraints filterConstraints = FilterConstraints.builder() .filterBy(action.getPhrase()) .checkInSource(true).checkInTarget(true) .includeStates(action.getActiveStates()) .build(); + // @formatter:on List idIndexList = new ArrayList(); Map transIdStateMap = new HashMap(); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java index 16a477ac60..8d543385ba 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java @@ -33,6 +33,7 @@ private GetTransUnitList(GetTransUnitActionContext context) offset = context.getOffset(); count = context.getCount(); phrase = context.getFindMessage(); + // @formatter :off filterStates = ActiveStates.builder() .setNewOn(context.isFilterUntranslated()) .setFuzzyOn(context.isFilterNeedReview()) @@ -40,6 +41,7 @@ private GetTransUnitList(GetTransUnitActionContext context) .setApprovedOn(context.isFilterApproved()) .setRejectedOn(context.isFilterRejected()) .build(); + // @formatter :on filterHasError = context.isFilterHasError(); targetTransUnitId = context.getTargetTransUnitId(); validationIds = context.getValidationIds(); From d06f5984eaa1a956f60726aefe29e670dde50dc2 Mon Sep 17 00:00:00 2001 From: David Mason Date: Tue, 9 Jul 2013 10:01:11 +1000 Subject: [PATCH 09/26] clear database before starting TextFlowDAOTest --- zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java index 456bf710ba..2b9616fead 100644 --- a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java +++ b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java @@ -31,6 +31,7 @@ public class TextFlowDAOTest extends ZanataDbunitJpaTest @Override protected void prepareDBUnitOperations() { + beforeTestOperations.add(new DataSetOperation("org/zanata/test/model/ClearAllTables.dbunit.xml", DatabaseOperation.DELETE_ALL)); beforeTestOperations.add(new DataSetOperation("org/zanata/test/model/ProjectsData.dbunit.xml", DatabaseOperation.CLEAN_INSERT)); beforeTestOperations.add(new DataSetOperation("org/zanata/test/model/TextFlowTestData.dbunit.xml", DatabaseOperation.CLEAN_INSERT)); beforeTestOperations.add(new DataSetOperation("org/zanata/test/model/LocalesData.dbunit.xml", DatabaseOperation.CLEAN_INSERT)); From 0bad9feb50c94fe490cecb4c909b104875c57b3d Mon Sep 17 00:00:00 2001 From: Damian Jansen Date: Fri, 5 Jul 2013 11:54:46 +1000 Subject: [PATCH 10/26] User Registration tests Currently, successful registration testing is blocked by Captcha validation. Includes register page, and testcases for field validation on usernames and email RFC2822 compliance. Basic validation for password matching and Captcha rejection. --- .../main/java/org/zanata/page/HomePage.java | 9 + .../org/zanata/page/account/RegisterPage.java | 127 ++++++++ .../main/java/org/zanata/util/RFC2822.java | 277 ++++++++++++++++++ .../feature/account/RegisterDetailedTest.java | 260 ++++++++++++++++ .../src/main/webapp/account/register.xhtml | 2 +- 5 files changed, 674 insertions(+), 1 deletion(-) create mode 100644 functional-test/src/main/java/org/zanata/page/account/RegisterPage.java create mode 100644 functional-test/src/main/java/org/zanata/util/RFC2822.java create mode 100644 functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java diff --git a/functional-test/src/main/java/org/zanata/page/HomePage.java b/functional-test/src/main/java/org/zanata/page/HomePage.java index 1ac0cb2b6c..b58226ae46 100644 --- a/functional-test/src/main/java/org/zanata/page/HomePage.java +++ b/functional-test/src/main/java/org/zanata/page/HomePage.java @@ -26,6 +26,7 @@ import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; +import org.zanata.page.account.RegisterPage; import org.zanata.page.administration.AdministrationPage; import org.zanata.page.groups.VersionGroupsPage; import org.zanata.page.projects.ProjectsPage; @@ -108,4 +109,12 @@ public AdministrationPage goToAdministration() adminLink.click(); return new AdministrationPage(getDriver()); } + + public RegisterPage goToRegistration() + { + getDriver().findElement(By.linkText("More")).click(); + WebElement registerLink = getDriver().findElement(By.id("Register")); + registerLink.click(); + return new RegisterPage(getDriver()); + } } diff --git a/functional-test/src/main/java/org/zanata/page/account/RegisterPage.java b/functional-test/src/main/java/org/zanata/page/account/RegisterPage.java new file mode 100644 index 0000000000..bc61baa12c --- /dev/null +++ b/functional-test/src/main/java/org/zanata/page/account/RegisterPage.java @@ -0,0 +1,127 @@ +package org.zanata.page.account; + + +import java.util.Map; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.support.FindBy; +import org.zanata.page.AbstractPage; +import lombok.extern.slf4j.Slf4j; + +/** + * @author Damian Jansen djansen@redhat.com + */ +@Slf4j +public class RegisterPage extends AbstractPage +{ + + @FindBy(id = "registerForm:nameField:name") + private WebElement nameField; + + @FindBy(id = "registerForm:emailField:email") + private WebElement emailField; + + @FindBy(id = "registerForm:usernameField:username") + private WebElement usernameField; + + @FindBy(id = "registerForm:passwordField:password") + private WebElement passwordField; + + @FindBy(id = "registerForm:passwordConfirmField:passwordConfirm") + private WebElement confirmPasswordField; + + @FindBy(id = "registerForm:captcha:verifyCaptcha") + private WebElement captchaField; + + @FindBy(id = "registerForm:agreedToTerms:agreedToTerms") + private WebElement termsCheckbox; + + @FindBy(id = "registerForm:registerButton") + private WebElement registerButton; + + public RegisterPage(WebDriver driver) + { + super(driver); + } + + public RegisterPage enterName(String name) + { + nameField.sendKeys(name); + return new RegisterPage(getDriver()); + } + + public RegisterPage enterUserName(String userName) + { + usernameField.sendKeys(userName); + return new RegisterPage(getDriver()); + } + + public RegisterPage enterEmail(String email) + { + emailField.sendKeys(email); + return new RegisterPage(getDriver()); + } + + public RegisterPage enterPassword(String password) + { + passwordField.sendKeys(password); + return new RegisterPage(getDriver()); + } + + public RegisterPage enterConfirmPassword(String confirmPassword) + { + confirmPasswordField.sendKeys(confirmPassword); + return new RegisterPage(getDriver()); + } + + public RegisterPage enterCaptcha(String captcha) + { + captchaField.sendKeys(captcha); + return new RegisterPage(getDriver()); + } + + public RegisterPage clickTerms() + { + termsCheckbox.click(); + return new RegisterPage(getDriver()); + } + + public AbstractPage register() + { + registerButton.click(); + return new AbstractPage(getDriver()); + } + + public RegisterPage registerFailure() + { + registerButton.click(); + return new RegisterPage(getDriver()); + } + + public RegisterPage clearFields() + { + nameField.clear(); + emailField.clear(); + usernameField.clear(); + passwordField.clear(); + confirmPasswordField.clear(); + captchaField.clear(); + return new RegisterPage(getDriver()); + } + + /* + Pass in a map of strings, to be entered into the registration fields. + Fields: name, email, username, password, confirmpassword, captcha + */ + public RegisterPage setFields(Map fields) + { + clearFields(); + enterName(fields.get("name")); + enterEmail(fields.get("email")); + enterUserName(fields.get("username")); + enterPassword(fields.get("password")); + enterConfirmPassword(fields.get("confirmpassword")); + enterCaptcha(fields.get("captcha")); + return new RegisterPage(getDriver()); + } +} diff --git a/functional-test/src/main/java/org/zanata/util/RFC2822.java b/functional-test/src/main/java/org/zanata/util/RFC2822.java new file mode 100644 index 0000000000..df61e4c886 --- /dev/null +++ b/functional-test/src/main/java/org/zanata/util/RFC2822.java @@ -0,0 +1,277 @@ +package org.zanata.util; + +import java.util.HashMap; +import java.util.Map; + +/** + * @author Damian Jansen djansen@redhat.com + */ +public class RFC2822 { + + /* + * Definitions + * localpart: the section of an address preceding the @ symbol + * domain: the section of an address following the @ symbol + * label: section of domain between the @ symbol or period and the next period or end e.g example in me@example.com + * quote / quoting: a section of the localpart contained within quotation marks + * + * BUG982048 + * This defect is an list of all items that, valid or invalid, are not correctly recognised by the email validation + * in Zanata. + * + * Untested: + * RFC2822, section 3.4.1 + * The contents of a bracketed domain can have a \ precede a character to escape it, + * and the following character must not be 10 (LF) or 13 (CR). + * This allows spaces in the domain as long as they are escaped. + * + * RFC 2821, section 4.5.3.1 + * The maximum length of a "useful" email address is 255 characters. + * + * RFC 3696 + * The maximum allowable length of an email address is 320 characters. + */ + + public static Map invalidEmailAddresses() + { + Map invalidEmailAddresses = new HashMap(); + + /* + * RFC 2822, section 3.4.1 + * Email addresses consist of a local part, the "@" symbol, and the domain. + */ + invalidEmailAddresses.put("3.4.1 Plain address", "plainaddress"); + invalidEmailAddresses.put("3.4.1 Missing @", "email.example.com"); + invalidEmailAddresses.put("3.4.1 Missing localpart", "@example.com"); + invalidEmailAddresses.put("3.4.1 Missing domain", "email@"); + invalidEmailAddresses.put("3.4.1 Two @ sign", "email@example@example.com"); + + /* + * RFC 2822, section 3.4.1 + * No periods can start or end the local part. + * Two periods together is invalid. + */ + invalidEmailAddresses.put("3.4.1 Leading dot", ".email@example.com"); + invalidEmailAddresses.put("3.4.1 Trailing dot", "email.@example.com"); + invalidEmailAddresses.put("3.4.1 Multiple dots", "email..email@example.com"); + + /* + * RFC 2822, section 2.2 + * All email addresses are in 7-bit US ASCII. + */ + // BUG982048invalidEmailAddresses.put("3.4.1 Non unicode characters", "あいうえお@example.com"); + + /* + * RFC 2822, section 3.4.1 + * Unquoted local parts can consist of TEXT + * TEXT can contain: + * alphabetic + * numeric + * and symbols !#$%'*+-/=?^_`{|}~ + */ + invalidEmailAddresses.put("3.4.1 Invalid unquoted character", "test,user@example.com"); + invalidEmailAddresses.put("3.4.1 Invalid unquoted character", "test(user@example.com"); + invalidEmailAddresses.put("3.4.1 Invalid unquoted character", "test)user@example.com"); + + /* + * RFC 2822, section 3.4.1 + * The quoted local part starts with a quotation mark, ends with a quotation mark. + */ + invalidEmailAddresses.put("3.4.1 Invalid quoting", "test\"user@example.com"); + + /* + * RFC 2822, section 4.4 + * If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted + * or quoted chunks separated by periods + */ + invalidEmailAddresses.put("3.4.1 Invalid quoting", "\"test\"user@example.com"); + invalidEmailAddresses.put("4.4 Invalid quoting", "\"test\"quote\"user@example.com"); + + /* + * RFC 2822, section 3.4.1 + * The contents of a quoted local part can not contain characters: + * 9 (TAB) + * 10 (LF) + * 13 (CR) + * 32 (space) + * 34 (") + * 91-94 ([, \, ], ^) + */ + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test,user\"@example.com"); + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test\\user\"@example.com"); + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test[user\"@example.com"); + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test]user\"@example.com"); + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test^user\"@example.com"); + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test user\"@example.com"); + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test\"user\"@example.com"); + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "test.\"".concat("\t").concat("\".user@example.com")); + + /* + * RFC 2822, section 3.4.1 + * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). + */ + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "test.\"\\".concat("\r").concat("\".user@example.com")); + invalidEmailAddresses.put("3.4.1 Invalid quoted character", "test.\"\\".concat("\n").concat("\".user@example.com")); + + /* + * RFC 1035, section 2.3.4 + * A plain domain consists of labels separated with periods. No period can start or end a domain name. + * No two periods in succession can be in a domain name. + */ + invalidEmailAddresses.put("RFC1035-2.3.4 Trailing dot in domain", "email@example.com."); + invalidEmailAddresses.put("RFC1035-2.3.4 Leading dot in domain", "email@.example.com"); + invalidEmailAddresses.put("RFC1035-2.3.4 Multiple dots in domain", "email@example..com"); + + /* + * RFC 2822, section 3.4.1 + * Bracketed domains must: + * start with [, end with ] + * not contain characters 9 (TAB), 10 (LF), 13 (CR), 32 (space), 91-94 ([, \, ], ^) + */ + invalidEmailAddresses.put("3.4.1 Incorrectly quoted domain", "email@[example].com"); + invalidEmailAddresses.put("3.4.1 Incorrectly quoted domain", "email@[ex^ample.com]"); + invalidEmailAddresses.put("3.4.1 Incorrectly quoted domain", "email@[exa\\mple].com"); + + /* + * RFC 1035, section 2.3.4 + * The maximum length of a label is 63 characters. + */ + // BUG982048 invalidEmailAddresses.put("RFC1035-2.3.4 Domain label too long", + // "email@IJUr9P6Y7Fx7rFy4sziQDT0qvSC7XKK6jrD0CNC41jorAKgFYIXLTN5ITJLohy58.com"); + + /* + * RFC 1035, section 2.3.4 + * A label may contain hyphens, but no two hyphens in a row. + * A label must not start nor end with a hyphen. + */ + // BUG982048 invalidEmailAddresses.put("2.3.4 Leading dash in domain", "email@-example.com"); + // BUG982048 invalidEmailAddresses.put("2.3.4 Trailing dash in domain", "email@example-.com"); + // BUG982048 invalidEmailAddresses.put("2.3.4 Multiple dashes in domain", "email@exa--mple.com"); + invalidEmailAddresses.put("2.3.4 Leading dash in bracketed domain", "email@[-example.com]"); + invalidEmailAddresses.put("2.3.4 Trailing dash in bracketed domain", "email@[example.com-]"); + invalidEmailAddresses.put("2.3.4 Multiple dashes in bracketed domain", "email@[exa--mple.com]"); + + /* + * The contents of a bracketed domain can have a \ precede a character to escape it, and the following character + * must not be 10 (LF) or 13 (CR). + */ + invalidEmailAddresses.put("3.4.1 Invalid bracketed domain", "test@[\\".concat("\r").concat("example.com]")); + invalidEmailAddresses.put("3.4.1 Invalid bracketed domain", "test@[\\".concat("\n").concat("example.com]")); + + /* + * RFC 2821, section 4.5.3.1 + * The maximum length of the local part is 64 characters. + */ + invalidEmailAddresses.put("RFC2821-4.5.3.1 Max localpart length is 64", + "emailuhpealgyxntsh5upl5gqn5a4ruqs7mw6wz21j6dn72amzwozqlyua4jx16rd@example.com"); + + /* + * RFC 3696, section 2 + * The top level domain must be all alphabetic. + */ + invalidEmailAddresses.put("RFC3696-2 Encoded html", "Joe Smith "); + invalidEmailAddresses.put("RFC3696-2 Following text", "email@example.com (Joe Smith)"); + // BUG982048 invalidEmailAddresses.put("RFC3696-2 Invalid IP", "email@111.222.333.44444"); + + /* + * RFC 2821, section 4.5.3.1 + * The maximum length of a "useful" email address is 255 characters. + */ + /* + * BUG982048 + invalidEmailAddresses.put("4.5.3.1 Max email length is 255", + "email@"+ + "Hk3yhCtbBRw3wCT76tL1ryAdfrIaaDszHqvZqnNrZPlNn3Wd7u."+ + "RfpxrueSghp9dkGTGwT9s0fyJL850Sned72RD3Mm5PpEh6QJwQ."+ + "3CeXyEHQEhXNOQdWhYVjGBLzlHz1sJfi4lfn7ighLXcxa5cMAK."+ + "jFXsG8BVsvkODKktTXJ70bQmDWtWQzuh3oz4twumVArDGEbzS1."+ + "slyaBcQqVgUdqXTBdbMY7YJxZwrzZQBBGjCl4e.com"); + */ + return invalidEmailAddresses; + } + + /* + * An map of valid emails conforming to RFC2822 + */ + public static Map validEmailAddresses() + { + Map validEmailAddresses = new HashMap(); + + /* + * RFC 2822, section 3.4.1 + * Email addresses consist of a local part, the "@" symbol, and the domain. + */ + validEmailAddresses.put("3.4.1 Basic email", "email@example.com"); + + /* + * RFC 2822, sections 3.4.1 and 4.4 + * The local part can be unquoted, quoted in its entirety, or quoted on a per-label basis. + * The quoted local part starts with a quotation mark, ends with a quotation mark. + */ + // BUG982048 validEmailAddresses.put("3.4.1 Basic quoted email", "\"email\"@example.com"); + + /* + * RFC 2822, section 3.4.1 + * TEXT can contain alphabetic, numeric, and these symbols: !#$%'*+-/=?^_`{|}~ + */ + validEmailAddresses.put("3.4.1 Allowed special characters in localpart", "email.!#$%'*+-/=?^_`{|}~.dot@example.com"); + + /* + RFC 2822, section 4.4 + If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted + or quoted chunks separated by periods. + */ + // BUG982048 validEmailAddresses.put("4.4 Quoted label with surrounding labels", "dot.\"email\".dot@example.com"); + // BUG982048 validEmailAddresses.put("4.4 Localpart with empty quote", "dot.\"\".dot@example.com"); + + /* + * RFC 2822, section 3.4.1 + * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). + * This supersedes the previous rule, allowing spaces and quotation marks in the email address as long as they + * are escaped. + */ + // BUG982048 validEmailAddresses.put("3.4.1 Quoted email with escaped special characters", "email.\"(),:;<>\\@\\[\\]\\\\\"@example.com"); + // BUG982048 validEmailAddresses.put("3.4.1 Quoted email with escaped quotes", "email.\"\\\"\"@example.com"); + // BUG982048 validEmailAddresses.put("3.4.1 Quoted email with space character", "\"special\\ email\"@example.com"); + + /* + * RFC 2822, section 3.4.1 + * The domain can be bracketed or plain. + */ + // BUG982048 validEmailAddresses.put("3.4.1 Email with bracketed domain", "email@[example.com]"); + // BUG982048 validEmailAddresses.put("3.4.1 Bracketed IPv6 domain", "email@[123.45.67.89]"); + // BUG982048 validEmailAddresses.put("3.4.1 Bracketed IPv6 domain", "email@[IPv6:2001:2d12:c4fe:5afe::1]"); + + /* + * RFC 1035, section 2.3.4 + * A plain domain consists of labels separated with periods. No period can start or end a domain name. + */ + validEmailAddresses.put("RFC1035-2.3.4 Localpart with multiple labels", "another.email@example.com"); + validEmailAddresses.put("RFC1035-2.3.4 Domain with multiple labels", "email@another.example.com"); + + /* + * RFC 1035, section 2.3.4 + * The maximum length of a label is 63 characters. + */ + validEmailAddresses.put("RFC1035-2.3.4 Domain label of 63 characters", + "email@B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8.com"); + validEmailAddresses.put("RFC1035-2.3.4 Localpart label of 63 characters", + "B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8@example.com"); + + /* + * RFC 1035, section 2.3.4 + * A label may contain hyphens, but no two hyphens in a row. + */ + validEmailAddresses.put("RFC1035-2.3.4 Hyphenated domain label", "email@another-example.com"); + validEmailAddresses.put("RFC1035-2.3.4 Hyphenated localpart label", "my-email@example.com"); + + /* + * RFC 2821, section 4.5.3.1 + * The maximum length of the local part is 64 characters. + */ + validEmailAddresses.put("RFC1035-2.3.4 Localpart length of 64 characters", + "B3NQyUsDdzODMoymfDdifn6Wztx2wrivm.80LEngHGl182frm6ifCPyv5SntbDg8@example.com"); + + return validEmailAddresses; + } +} \ No newline at end of file diff --git a/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java b/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java new file mode 100644 index 0000000000..2b03712090 --- /dev/null +++ b/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java @@ -0,0 +1,260 @@ +/* + * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the + * @author tags. See the copyright.txt file in the distribution for a full + * listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This software is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this software; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF + * site: http://www.fsf.org. + */ +package org.zanata.feature.account; + +import org.hamcrest.Matchers; +import org.junit.*; +import org.zanata.page.HomePage; +import org.zanata.page.account.RegisterPage; +import org.zanata.util.ResetDatabaseRule; +import org.zanata.util.RFC2822; +import org.zanata.workflow.AbstractWebWorkFlow; + +import java.util.*; + +import static org.hamcrest.MatcherAssert.assertThat; +import lombok.extern.slf4j.Slf4j; + +/** + * @author Damian Jansen djansen@redhat.com + */ +@Slf4j +public class RegisterDetailedTest +{ + @ClassRule + public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(ResetDatabaseRule.Config.WithData, ResetDatabaseRule.Config.NoResetAfter); + + Map fields; + private HomePage homePage; + + @Before + public void before() + { + // fields contains a set of data that can be successfully registered + fields = new HashMap(); + + // Conflicting fields - must be set for each test function to avoid "not available" errors + fields.put("email", "test@test.com"); + fields.put("username", "testusername"); + + fields.put("name", "test"); + fields.put("password", "testpassword"); + fields.put("confirmpassword", "testpassword"); + fields.put("captcha", "555"); // TODO: Expect captcha error, fix + homePage = new AbstractWebWorkFlow().goToHome(); + } + + @Test + @Ignore("Captcha prevents test completion") + public void registerSuccessful() + { + RegisterPage registerPage = homePage.goToRegistration(); + registerPage = registerPage.setFields(fields); + assertThat("No errors are shown", registerPage.getErrors().size(), Matchers.equalTo(0)); + registerPage.register(); + } + + @Test + public void usernameLengthValidation() + { + String errorMsg = "size must be between 3 and 20"; + fields.put("email", "length.test@test.com"); + RegisterPage registerPage = homePage.goToRegistration(); + + fields.put("username", "bo"); + registerPage = registerPage.setFields(fields); + assertThat("Size errors are shown for string too short", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + + fields.put("username", "testusername"); + registerPage = registerPage.setFields(fields); + assertThat("Size errors are not shown", registerPage.getErrors(), Matchers.not(Matchers.hasItem(errorMsg))); + + fields.put("username", "12345678901234567890a"); + registerPage = registerPage.setFields(fields); + assertThat("Size errors are shown for string too long", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + } + + @Test + public void usernameCharacterValidation() + { + String errorMsg = "lowercase letters and digits (regex \"^[a-z\\d_]{3,20}$\")"; + fields.put("email", "character.test@example.com"); + for (Map.Entry entry : usernameCharacterValidationData().entrySet()) + { + log.info("Test " + entry.getKey() + ":" + entry.getValue()); + fields.put("username", entry.getValue()); + RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + assertThat("Validation errors are shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + } + } + + @Test + @Ignore("Captcha prevents test completion") + public void usernamePreExisting() + { + String errorMsg = "This username is not available"; + fields.put("email", "exists.test@test.com"); + fields.put("username", "alreadyexists"); + RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + registerPage.register(); + + fields.put("email", "exists2.test@test.com"); + registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + assertThat("Username not available message is shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + } + + @Test + public void emailValidation() + { + String errorMsg = "not a well-formed email address"; + fields.put("username", "emailvalidation"); + for (Map.Entry entry : RFC2822.invalidEmailAddresses().entrySet()) + { + log.info("Test " + entry.getKey() + ":" + entry.getValue()); + fields.put("email", entry.getValue()); + RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + assertThat("Email validation errors are shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + } + } + + @Test + public void validEmailAcceptance() + { + String errorMsg = "not a well-formed email address"; + fields.put("username", "emailvalidation"); + for (Map.Entry entry : RFC2822.validEmailAddresses().entrySet()) + { + log.info("Test " + entry.getKey() + ":" + entry.getValue()); + fields.put("email", entry.getValue()); + RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + assertThat("Email validation errors are not shown", registerPage.getErrors(), + Matchers.not(Matchers.hasItem(errorMsg))); + } + } + + @Test + public void rejectIncorrectCaptcha() + { + String errorMsg = "incorrect response"; + fields.put("username", "rejectbadcaptcha"); + fields.put("email", "rejectbadcaptcha@example.com"); + fields.put("captcha", "9000"); + + RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields).registerFailure(); + assertThat("The Captcha entry is rejected", registerPage.getErrors(), Matchers.contains(errorMsg)); + } + + @Test + public void passwordsMatch() + { + String errorMsg = "Passwords do not match"; + fields.put("username", "passwordsmatch"); + fields.put("email", "passwordsmatchtest@example.com"); + fields.put("password", "passwordsmatch"); + fields.put("confirmpassword", "passwordsdonotmatch"); + + RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + assertThat("Passwords fail to match error is shown", registerPage.getErrors(), Matchers.contains(errorMsg)); + } + + @Test + public void requiredFields() + { + String errorMsg = "value is required"; + fields.put("name", ""); + fields.put("username", ""); + fields.put("email", ""); + fields.put("password", ""); + fields.put("confirmpassword", ""); + + RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + assertThat("Value is required shows for all fields", registerPage.getErrors(), + Matchers.contains(errorMsg, errorMsg, errorMsg, errorMsg, errorMsg)); + } + + /* + Bugs + */ + @Test(expected = AssertionError.class) + public void bug981082_inaccurateErrorMessage() + { + String errorMsg = "size must be between 3 and 20"; + fields.put("email", "bug981082test@test.com"); + fields.put("username", "mo"); + + RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + assertThat("Size errors are shown for string too short", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + + fields.put("username", "johndoeusernamevalidation"); + registerPage = registerPage.setFields(fields); + assertThat("Size errors are shown for string too long", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + } + + @Test(expected = AssertionError.class) + public void bug981498_underscoreRules() + { + String errorMsg = "lowercase letters and digits (regex \"^[a-z\\d_]{3,20}$\")"; + fields.put("email", "bug981498test@example.com"); + fields.put("username", "______"); + RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + assertThat("A username of all underscores is not valid", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + } + + /* + Returns a hash of invalid characters for a username + Hash is String reason for invalid 'character', String character + */ + private LinkedHashMap usernameCharacterValidationData() + { + LinkedHashMap inputData = new LinkedHashMap(100); + inputData.put("Invalid char |", "user|name"); + inputData.put("Invalid char /", "user/name"); + inputData.put("Invalid char ", "user\\name"); + inputData.put("Invalid char +", "user+name"); + inputData.put("Invalid char *", "user*name"); + inputData.put("Invalid char |", "user|name"); + inputData.put("Invalid char (", "user(name"); + inputData.put("Invalid char )", "user)name"); + inputData.put("Invalid char $", "user$name"); + inputData.put("Invalid char [", "user[name"); + inputData.put("Invalid char ]", "user]name"); + inputData.put("Invalid char :", "user:name"); + inputData.put("Invalid char ;", "user;name"); + inputData.put("Invalid char '", "user'name"); + inputData.put("Invalid char ,", "user,name"); + inputData.put("Invalid char ?", "user?name"); + inputData.put("Invalid char !", "user!name"); + inputData.put("Invalid char @", "user@name"); + inputData.put("Invalid char #", "user#name"); + inputData.put("Invalid char %", "user%name"); + inputData.put("Invalid char ^", "user^name"); + inputData.put("Invalid char =", "user=name"); + inputData.put("Invalid char .", "user.name"); + inputData.put("Invalid char {", "user{name"); + inputData.put("Invalid char }", "user}name"); + // Capital letters are prohibited + for (char c = 'A'; c <= 'Z'; c++) { + String letter = String.valueOf(c); + inputData.put("Invalid capital char ".concat(letter), "user".concat(letter).concat("name")); + } + return inputData; + } +} diff --git a/zanata-war/src/main/webapp/account/register.xhtml b/zanata-war/src/main/webapp/account/register.xhtml index 116417bb8c..eebe55aab7 100644 --- a/zanata-war/src/main/webapp/account/register.xhtml +++ b/zanata-war/src/main/webapp/account/register.xhtml @@ -95,7 +95,7 @@
- +
From 96d8f0598490355aeb518daafd8895fceaa413f8 Mon Sep 17 00:00:00 2001 From: Damian Jansen Date: Wed, 10 Jul 2013 10:28:32 +1000 Subject: [PATCH 11/26] Better description of the RFC2822 class The description was lacking / inaccurate. --- functional-test/src/main/java/org/zanata/util/RFC2822.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/functional-test/src/main/java/org/zanata/util/RFC2822.java b/functional-test/src/main/java/org/zanata/util/RFC2822.java index df61e4c886..d4937a46a6 100644 --- a/functional-test/src/main/java/org/zanata/util/RFC2822.java +++ b/functional-test/src/main/java/org/zanata/util/RFC2822.java @@ -9,10 +9,15 @@ public class RFC2822 { /* + * Synopsis: + * The functions of this class contain valid and invalid email addresses, as stipulated in the + * RFC2822 Internet Message Format standard, or referred to standards. + * * Definitions * localpart: the section of an address preceding the @ symbol * domain: the section of an address following the @ symbol - * label: section of domain between the @ symbol or period and the next period or end e.g example in me@example.com + * label: section of localpart or domain between the start, @ symbol, period or end (also referred to as "atom") + * e.g. me, myself, example, com in me.myself@example.com * quote / quoting: a section of the localpart contained within quotation marks * * BUG982048 From 33e5375531661c64bfdf396606287491bee40e0e Mon Sep 17 00:00:00 2001 From: David Mason Date: Wed, 10 Jul 2013 13:10:15 +1000 Subject: [PATCH 12/26] Rename ActiveStates to ContentStateGroup, rename methods and move to gwt shared. Move to gwt includes a few minor changes to allow gwt-rpc serialization. --- .../main/java/org/zanata/dao/TextFlowDAO.java | 10 +- .../java/org/zanata/search/ActiveStates.java | 146 -------------- .../search/FilterConstraintToQuery.java | 2 +- .../org/zanata/search/FilterConstraints.java | 38 ++-- .../impl/TextFlowSearchServiceImpl.java | 14 +- .../server/rpc/GetTransUnitListHandler.java | 2 +- .../shared/model/ContentStateGroup.java | 183 ++++++++++++++++++ .../webtrans/shared/rpc/GetTransUnitList.java | 28 +-- .../shared/rpc/GetTransUnitsNavigation.java | 21 +- 9 files changed, 243 insertions(+), 201 deletions(-) delete mode 100644 zanata-war/src/main/java/org/zanata/search/ActiveStates.java create mode 100644 zanata-war/src/main/java/org/zanata/webtrans/shared/model/ContentStateGroup.java diff --git a/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java b/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java index 467e2c6f6f..8ed888ded6 100644 --- a/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java +++ b/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java @@ -41,9 +41,9 @@ import org.zanata.model.HDocument; import org.zanata.model.HLocale; import org.zanata.model.HTextFlow; -import org.zanata.search.ActiveStates; import org.zanata.search.FilterConstraintToQuery; import org.zanata.search.FilterConstraints; +import org.zanata.webtrans.shared.model.ContentStateGroup; import org.zanata.webtrans.shared.model.DocumentId; import com.google.common.base.Joiner; @@ -172,7 +172,7 @@ public List getNavigationByDocumentId(Long documentId, HLocale hLocal protected static String buildContentStateCondition(FilterConstraints constraints, String alias) { - ActiveStates includedStates = constraints.getIncludedStates(); + ContentStateGroup includedStates = constraints.getIncludedStates(); if (includedStates.hasAllStates() || includedStates.hasNoStates()) { return "1"; @@ -181,17 +181,17 @@ protected static String buildContentStateCondition(FilterConstraints constraints builder.append("("); List conditions = Lists.newArrayList(); final String column = alias + ".state"; - if (constraints.getIncludedStates().isTranslatedOn()) + if (constraints.getIncludedStates().hasTranslated()) { conditions.add(column + "=2"); // Translated conditions.add(column + "=3"); // Approved } - if (constraints.getIncludedStates().isFuzzyOn()) + if (constraints.getIncludedStates().hasFuzzy()) { conditions.add(column + "=1"); // Fuzzy conditions.add(column + "=4"); // Rejected } - if (constraints.getIncludedStates().isNewOn()) + if (constraints.getIncludedStates().hasNew()) { conditions.add(column + "=0 or " + column + " is null"); } diff --git a/zanata-war/src/main/java/org/zanata/search/ActiveStates.java b/zanata-war/src/main/java/org/zanata/search/ActiveStates.java deleted file mode 100644 index eeabfa61bb..0000000000 --- a/zanata-war/src/main/java/org/zanata/search/ActiveStates.java +++ /dev/null @@ -1,146 +0,0 @@ -package org.zanata.search; - -import java.util.List; - -import org.zanata.common.ContentState; - -import com.google.common.collect.Lists; - -import lombok.Getter; -import lombok.AllArgsConstructor; -import lombok.ToString; - -@ToString -@AllArgsConstructor -@Getter -public class ActiveStates -{ - private boolean newOn; - private boolean fuzzyOn; - private boolean translatedOn; - private boolean approvedOn; - private boolean rejectedOn; - - /** - * @return a Builder with all states on by default - */ - public static Builder builder() - { - return new Builder(); - } - - public boolean hasAllStates() - { - return newOn && fuzzyOn && translatedOn && approvedOn && rejectedOn; - } - - public boolean hasNoStates() - { - return !(newOn || fuzzyOn || translatedOn || approvedOn || rejectedOn); - } - - public List asList() - { - List result = Lists.newArrayList(); - if (newOn) - { - result.add(ContentState.New); - } - if (fuzzyOn) - { - result.add(ContentState.NeedReview); - } - if (translatedOn) - { - result.add(ContentState.Translated); - } - if (approvedOn) - { - result.add(ContentState.Approved); - } - if (rejectedOn) - { - result.add(ContentState.Rejected); - } - return result; - } - - public static class Builder - { - private boolean newOn; - private boolean fuzzyOn; - private boolean translatedOn; - private boolean approvedOn; - private boolean rejectedOn; - - public Builder() - { - allOn(); - } - - public ActiveStates build() - { - return new ActiveStates(newOn, fuzzyOn, translatedOn, approvedOn, rejectedOn); - } - - public Builder allOn() - { - this.newOn = true; - this.fuzzyOn = true; - this.translatedOn = true; - this.approvedOn = true; - this.rejectedOn = true; - return this; - } - - public Builder allOff() - { - this.newOn = false; - this.fuzzyOn = false; - this.translatedOn = false; - this.approvedOn = false; - this.rejectedOn = false; - return this; - } - - public Builder fromStates(ActiveStates states) - { - this.newOn = states.newOn; - this.fuzzyOn = states.fuzzyOn; - this.translatedOn = states.translatedOn; - this.approvedOn = states.approvedOn; - this.rejectedOn = states.rejectedOn; - return this; - } - - public Builder setNewOn(boolean on) - { - newOn = on; - return this; - } - - public Builder setFuzzyOn(boolean on) - { - fuzzyOn = on; - return this; - } - - public Builder setTranslatedOn(boolean on) - { - translatedOn = on; - return this; - } - - public Builder setApprovedOn(boolean on) - { - approvedOn = on; - return this; - } - - public Builder setRejectedOn(boolean on) - { - rejectedOn = on; - return this; - } - } -} diff --git a/zanata-war/src/main/java/org/zanata/search/FilterConstraintToQuery.java b/zanata-war/src/main/java/org/zanata/search/FilterConstraintToQuery.java index a6cf05e406..6f46ff3ca2 100644 --- a/zanata-war/src/main/java/org/zanata/search/FilterConstraintToQuery.java +++ b/zanata-war/src/main/java/org/zanata/search/FilterConstraintToQuery.java @@ -148,7 +148,7 @@ protected String buildStateCondition() String stateInListWhereClause = and(textFlowAndLocaleRestriction.toString(), String.format("state in (%s)", STATE_LIST_PLACEHOLDER)); String stateInListCondition = QueryBuilder.exists().from("HTextFlowTarget").where(stateInListWhereClause).toQueryString(); - if (constraints.getIncludedStates().isNewOn()) + if (constraints.getIncludedStates().hasNew()) { String nullTargetCondition = String.format("%s not in indices(tf.targets)", LOCALE_PLACEHOLDER); if (hasSearch && constraints.isSearchInSource()) diff --git a/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java b/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java index 8f52112a8f..d5803bb1da 100644 --- a/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java +++ b/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java @@ -23,6 +23,8 @@ //TODO May want to add document(someDocument) to these constraints //so that only one search method is needed on the interface. +import org.zanata.webtrans.shared.model.ContentStateGroup; + import lombok.Getter; import com.google.common.base.Objects; @@ -39,11 +41,11 @@ public class FilterConstraints private boolean isCaseSensitive; private boolean searchInSource; private boolean searchInTarget; - private ActiveStates includedStates; + private ContentStateGroup includedStates; private FilterConstraints(String searchString, boolean caseSensitive, boolean searchInSource, boolean searchInTarget, - ActiveStates includedStates) + ContentStateGroup includedStates) { this.searchString = searchString; this.isCaseSensitive = caseSensitive; @@ -77,11 +79,11 @@ public static class Builder private boolean caseSensitive; private boolean searchInSource; private boolean searchInTarget; - private ActiveStates.Builder states; + private ContentStateGroup.Builder states; public Builder() { - states = ActiveStates.builder(); + states = ContentStateGroup.builder(); setKeepAll(); } @@ -103,7 +105,7 @@ private void setKeepAll() caseSensitive = false; searchInSource = true; searchInTarget = true; - states.allOn(); + states.addAll(); } public Builder keepNone() @@ -112,7 +114,7 @@ public Builder keepNone() caseSensitive = false; searchInSource = false; searchInTarget = false; - states.allOff(); + states.removeAll(); return this; } @@ -140,7 +142,7 @@ public Builder checkInTarget(boolean check) return this; } - public Builder includeStates(ActiveStates states) + public Builder includeStates(ContentStateGroup states) { //FIXME this behaviour is too surprising. // It exists because the editor UI should show all states when either @@ -148,7 +150,7 @@ public Builder includeStates(ActiveStates states) // in the editor backend *before* sending a request to the server. if (states.hasNoStates()) { - this.states.allOn(); + this.states.addAll(); } else { @@ -159,61 +161,61 @@ public Builder includeStates(ActiveStates states) public Builder includeNew() { - states.setNewOn(true); + states.includeNew(true); return this; } public Builder excludeNew() { - states.setNewOn(false); + states.includeNew(false); return this; } public Builder includeFuzzy() { - states.setFuzzyOn(true); + states.includeFuzzy(true); return this; } public Builder excludeFuzzy() { - states.setFuzzyOn(false); + states.includeFuzzy(false); return this; } public Builder includeTranslated() { - states.setTranslatedOn(true); + states.includeTranslated(true); return this; } public Builder excludeTranslated() { - states.setTranslatedOn(false); + states.includeTranslated(false); return this; } public Builder includeApproved() { - states.setApprovedOn(true); + states.includeApproved(true); return this; } public Builder excludeApproved() { - states.setApprovedOn(false); + states.includeApproved(false); return this; } public Builder includeRejected() { - states.setRejectedOn(true); + states.includeRejected(true); return this; } public Builder excludeRejected() { - states.setRejectedOn(false); + states.includeRejected(false); return this; } diff --git a/zanata-war/src/main/java/org/zanata/service/impl/TextFlowSearchServiceImpl.java b/zanata-war/src/main/java/org/zanata/service/impl/TextFlowSearchServiceImpl.java index e31a97de91..f3016803af 100644 --- a/zanata-war/src/main/java/org/zanata/service/impl/TextFlowSearchServiceImpl.java +++ b/zanata-war/src/main/java/org/zanata/service/impl/TextFlowSearchServiceImpl.java @@ -57,11 +57,11 @@ import org.zanata.model.HProjectIteration; import org.zanata.model.HTextFlow; import org.zanata.model.HTextFlowTarget; -import org.zanata.search.ActiveStates; import org.zanata.search.FilterConstraintToQuery; import org.zanata.search.FilterConstraints; import org.zanata.service.LocaleService; import org.zanata.service.TextFlowSearchService; +import org.zanata.webtrans.shared.model.ContentStateGroup; import org.zanata.webtrans.shared.model.DocumentId; import org.zanata.webtrans.shared.model.WorkspaceId; @@ -140,8 +140,10 @@ private List findTextFlowsByDocumentPaths(WorkspaceId workspace, List return Collections.emptyList(); } - ActiveStates includedStates = constraints.getIncludedStates(); - if (!includedStates.isNewOn() && !includedStates.isFuzzyOn() && !includedStates.isTranslatedOn()) + // FIXME this looks like it assumes only 3 states and would not work properly for getting + // e.g. only approved strings while there is a search active. + ContentStateGroup includedStates = constraints.getIncludedStates(); + if (!includedStates.hasNew() && !includedStates.hasFuzzy() && !includedStates.hasTranslated()) { // including nothing return Collections.emptyList(); @@ -300,19 +302,19 @@ private List findTextFlowsWithHibernateSearch(String projectSlug, Str } targetQuery.add(localeQuery, Occur.MUST); - if (!constraints.getIncludedStates().isTranslatedOn()) + if (!constraints.getIncludedStates().hasTranslated()) { TermQuery approvedStateQuery = new TermQuery(new Term(IndexFieldLabels.CONTENT_STATE_FIELD, ContentState.Approved.toString())); targetQuery.add(approvedStateQuery, Occur.MUST_NOT); } - if (!constraints.getIncludedStates().isFuzzyOn()) + if (!constraints.getIncludedStates().hasFuzzy()) { TermQuery approvedStateQuery = new TermQuery(new Term(IndexFieldLabels.CONTENT_STATE_FIELD, ContentState.NeedReview.toString())); targetQuery.add(approvedStateQuery, Occur.MUST_NOT); } - if (!constraints.getIncludedStates().isNewOn()) + if (!constraints.getIncludedStates().hasNew()) { TermQuery approvedStateQuery = new TermQuery(new Term(IndexFieldLabels.CONTENT_STATE_FIELD, ContentState.New.toString())); targetQuery.add(approvedStateQuery, Occur.MUST_NOT); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java index fe76eb80dd..a412836c76 100755 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTransUnitListHandler.java @@ -35,12 +35,12 @@ import org.zanata.exception.ZanataServiceException; import org.zanata.model.HLocale; import org.zanata.model.HTextFlow; -import org.zanata.search.ActiveStates; import org.zanata.search.FilterConstraints; import org.zanata.security.ZanataIdentity; import org.zanata.service.LocaleService; import org.zanata.service.ValidationService; import org.zanata.webtrans.server.ActionHandlerFor; +import org.zanata.webtrans.shared.model.ContentStateGroup; import org.zanata.webtrans.shared.model.TransUnit; import org.zanata.webtrans.shared.rpc.GetTransUnitList; import org.zanata.webtrans.shared.rpc.GetTransUnitListResult; diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/model/ContentStateGroup.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/model/ContentStateGroup.java new file mode 100644 index 0000000000..a104b68cd7 --- /dev/null +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/model/ContentStateGroup.java @@ -0,0 +1,183 @@ +package org.zanata.webtrans.shared.model; + +import java.util.List; + +import org.zanata.common.ContentState; + +import com.google.common.collect.Lists; +import com.google.gwt.user.client.rpc.IsSerializable; + +import lombok.ToString; + +@ToString +public class ContentStateGroup implements IsSerializable +{ + private boolean hasNew; + private boolean hasFuzzy; + private boolean hasTranslated; + private boolean hasApproved; + private boolean hasRejected; + + private ContentStateGroup() + { + // This exists to allow GWT to serialize + } + + private ContentStateGroup(boolean includeNew, boolean includeFuzzy, boolean includeTranslated, + boolean includeApproved, boolean includeRejected) + { + hasNew = includeNew; + hasFuzzy = includeFuzzy; + hasTranslated = includeTranslated; + hasApproved = includeApproved; + hasRejected = includeRejected; + } + + /** + * @return a Builder with all states on by default + */ + public static Builder builder() + { + return new Builder(); + } + + public boolean hasNew() + { + return hasNew; + } + + public boolean hasFuzzy() + { + return hasFuzzy; + } + + public boolean hasTranslated() + { + return hasTranslated; + } + + public boolean hasApproved() + { + return hasApproved; + } + + public boolean hasRejected() + { + return hasRejected; + } + + public boolean hasAllStates() + { + return hasNew && hasFuzzy && hasTranslated && hasApproved && hasRejected; + } + + public boolean hasNoStates() + { + return !(hasNew || hasFuzzy || hasTranslated || hasApproved || hasRejected); + } + + public List asList() + { + List result = Lists.newArrayList(); + if (hasNew) + { + result.add(ContentState.New); + } + if (hasFuzzy) + { + result.add(ContentState.NeedReview); + } + if (hasTranslated) + { + result.add(ContentState.Translated); + } + if (hasApproved) + { + result.add(ContentState.Approved); + } + if (hasRejected) + { + result.add(ContentState.Rejected); + } + return result; + } + + public static class Builder + { + private boolean hasNew; + private boolean hasFuzzy; + private boolean hasTranslated; + private boolean hasApproved; + private boolean hasRejected; + + public Builder() + { + addAll(); + } + + public ContentStateGroup build() + { + return new ContentStateGroup(hasNew, hasFuzzy, hasTranslated, hasApproved, hasRejected); + } + + public Builder addAll() + { + this.hasNew = true; + this.hasFuzzy = true; + this.hasTranslated = true; + this.hasApproved = true; + this.hasRejected = true; + return this; + } + + public Builder removeAll() + { + this.hasNew = false; + this.hasFuzzy = false; + this.hasTranslated = false; + this.hasApproved = false; + this.hasRejected = false; + return this; + } + + public Builder fromStates(ContentStateGroup states) + { + this.hasNew = states.hasNew; + this.hasFuzzy = states.hasFuzzy; + this.hasTranslated = states.hasTranslated; + this.hasApproved = states.hasApproved; + this.hasRejected = states.hasRejected; + return this; + } + + public Builder includeNew(boolean on) + { + hasNew = on; + return this; + } + + public Builder includeFuzzy(boolean on) + { + hasFuzzy = on; + return this; + } + + public Builder includeTranslated(boolean on) + { + hasTranslated = on; + return this; + } + + public Builder includeApproved(boolean on) + { + hasApproved = on; + return this; + } + + public Builder includeRejected(boolean on) + { + hasRejected = on; + return this; + } + } +} diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java index 8d543385ba..00f3645aea 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java @@ -2,8 +2,8 @@ import java.util.List; -import org.zanata.search.ActiveStates; import org.zanata.webtrans.client.service.GetTransUnitActionContext; +import org.zanata.webtrans.shared.model.ContentStateGroup; import org.zanata.webtrans.shared.model.DocumentId; import org.zanata.webtrans.shared.model.TransUnitId; import org.zanata.webtrans.shared.model.ValidationId; @@ -17,7 +17,7 @@ public class GetTransUnitList extends AbstractWorkspaceAction validationIds; private TransUnitId targetTransUnitId; @@ -34,12 +34,12 @@ private GetTransUnitList(GetTransUnitActionContext context) count = context.getCount(); phrase = context.getFindMessage(); // @formatter :off - filterStates = ActiveStates.builder() - .setNewOn(context.isFilterUntranslated()) - .setFuzzyOn(context.isFilterNeedReview()) - .setTranslatedOn(context.isFilterTranslated()) - .setApprovedOn(context.isFilterApproved()) - .setRejectedOn(context.isFilterRejected()) + filterStates = ContentStateGroup.builder() + .includeNew(context.isFilterUntranslated()) + .includeFuzzy(context.isFilterNeedReview()) + .includeTranslated(context.isFilterTranslated()) + .includeApproved(context.isFilterApproved()) + .includeRejected(context.isFilterRejected()) .build(); // @formatter :on filterHasError = context.isFilterHasError(); @@ -83,34 +83,34 @@ public String getPhrase() return this.phrase; } - public ActiveStates getFilterStates() + public ContentStateGroup getFilterStates() { return filterStates; } public boolean isFilterTranslated() { - return filterStates.isTranslatedOn(); + return filterStates.hasTranslated(); } public boolean isFilterNeedReview() { - return filterStates.isFuzzyOn(); + return filterStates.hasFuzzy(); } public boolean isFilterUntranslated() { - return filterStates.isNewOn(); + return filterStates.hasNew(); } public boolean isFilterApproved() { - return filterStates.isApprovedOn(); + return filterStates.hasApproved(); } public boolean isFilterRejected() { - return filterStates.isRejectedOn(); + return filterStates.hasRejected(); } public boolean isFilterHasError() diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java index 1ca50389b7..1f8cb9a861 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java @@ -21,8 +21,9 @@ package org.zanata.webtrans.shared.rpc; -import org.zanata.search.ActiveStates; import org.zanata.webtrans.client.service.GetTransUnitActionContext; +import org.zanata.webtrans.shared.model.ContentStateGroup; + import com.google.common.base.Objects; public class GetTransUnitsNavigation @@ -30,14 +31,14 @@ public class GetTransUnitsNavigation private Long id; private String phrase; - private ActiveStates activeStates; + private ContentStateGroup activeStates; @SuppressWarnings("unused") private GetTransUnitsNavigation() { } - public GetTransUnitsNavigation(Long id, String phrase, ActiveStates activeStates) + public GetTransUnitsNavigation(Long id, String phrase, ContentStateGroup activeStates) { this.id = id; this.phrase = phrase; @@ -48,12 +49,12 @@ public GetTransUnitsNavigation(GetTransUnitActionContext context) { this(context.getDocument().getId().getId(), context.getFindMessage(), - ActiveStates.builder() - .setNewOn(context.isFilterUntranslated()) - .setFuzzyOn(context.isFilterNeedReview()) - .setTranslatedOn(context.isFilterTranslated()) - .setApprovedOn(context.isFilterApproved()) - .setRejectedOn(context.isFilterRejected()) + ContentStateGroup.builder() + .includeNew(context.isFilterUntranslated()) + .includeFuzzy(context.isFilterNeedReview()) + .includeTranslated(context.isFilterTranslated()) + .includeApproved(context.isFilterApproved()) + .includeRejected(context.isFilterRejected()) .build()); } @@ -72,7 +73,7 @@ public String getPhrase() return this.phrase; } - public ActiveStates getActiveStates() + public ContentStateGroup getActiveStates() { return activeStates; } From b75e3966fcf3d9f33d7495853889d054b4d342e1 Mon Sep 17 00:00:00 2001 From: David Mason Date: Wed, 10 Jul 2013 14:48:42 +1000 Subject: [PATCH 13/26] prevent TextFlowDAO from conflating states in queries --- .../main/java/org/zanata/dao/TextFlowDAO.java | 36 +++++----- .../java/org/zanata/dao/TextFlowDAOTest.java | 72 ++++++++++++------- 2 files changed, 65 insertions(+), 43 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java b/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java index 8ed888ded6..e3f613cbbf 100644 --- a/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java +++ b/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java @@ -128,7 +128,7 @@ public List getNavigationByDocumentId(Long documentId, HLocale hLocal .append(" WHERE tf.document_id = :docId AND tf.obsolete = 0"); queryBuilder .append(" AND ") - .append(buildContentStateCondition(filterConstraints, "tft")); + .append(buildContentStateCondition(filterConstraints.getIncludedStates(), "tft")); boolean hasSearchString = !Strings.isNullOrEmpty(filterConstraints.getSearchString()); if (hasSearchString) { @@ -163,16 +163,12 @@ public List getNavigationByDocumentId(Long documentId, HLocale hLocal * This will build a SQL query condition in where clause. * If all status are equal (i.e. all true or all false), it's treated as accept all and it will return '1'. * - * @param acceptApproved accept approved status - * @param acceptFuzzy accept fuzzy status - * @param acceptUntranslated accept untranslated status - * @param alias HTextFlowTarget alias + * @param includedStates + * @param hTextFlowTargetTableAlias alias being used for the target table in the current query * @return '1' if accept all status or a SQL condition clause with target content state conditions in parentheses '()' joined by 'or' */ - protected static String buildContentStateCondition(FilterConstraints constraints, String alias) + protected static String buildContentStateCondition(ContentStateGroup includedStates, String hTextFlowTargetTableAlias) { - - ContentStateGroup includedStates = constraints.getIncludedStates(); if (includedStates.hasAllStates() || includedStates.hasNoStates()) { return "1"; @@ -180,20 +176,26 @@ protected static String buildContentStateCondition(FilterConstraints constraints StringBuilder builder = new StringBuilder(); builder.append("("); List conditions = Lists.newArrayList(); - final String column = alias + ".state"; - if (constraints.getIncludedStates().hasTranslated()) + final String stateColumn = hTextFlowTargetTableAlias + ".state"; + if (includedStates.hasNew()) + { + conditions.add(stateColumn + "=0 or " + stateColumn + " is null"); + } + if (includedStates.hasFuzzy()) + { + conditions.add(stateColumn + "=1"); + } + if (includedStates.hasTranslated()) { - conditions.add(column + "=2"); // Translated - conditions.add(column + "=3"); // Approved + conditions.add(stateColumn + "=2"); } - if (constraints.getIncludedStates().hasFuzzy()) + if (includedStates.hasApproved()) { - conditions.add(column + "=1"); // Fuzzy - conditions.add(column + "=4"); // Rejected + conditions.add(stateColumn + "=3"); } - if (constraints.getIncludedStates().hasNew()) + if (includedStates.hasRejected()) { - conditions.add(column + "=0 or " + column + " is null"); + conditions.add(stateColumn + "=4"); } Joiner joiner = Joiner.on(" or "); joiner.appendTo(builder, conditions); diff --git a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java index 2b9616fead..f9a5d27d2d 100644 --- a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java +++ b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java @@ -19,6 +19,7 @@ import org.zanata.model.HTextFlow; import org.zanata.model.HTextFlowTarget; import org.zanata.search.FilterConstraints; +import org.zanata.webtrans.shared.model.ContentStateGroup; import org.zanata.webtrans.shared.model.DocumentId; @Test(groups = { "jpa-tests" }) @@ -174,69 +175,88 @@ public void thisBreaksForSomeReason() { public void canBuildAcceptAllQuery() { String contentStateCondition = TextFlowDAO.buildContentStateCondition( - FilterConstraints.builder().keepAll().build(), "tft"); + ContentStateGroup.builder().addAll().build(), "tft"); assertThat("Conditional that accepts all should be '1'", contentStateCondition, is("1")); } + // FIXME the 'none == all' logic should be limited to the editor @Test public void canBuildAcceptAllQueryWhenNoStatesSelected() { String contentStateCondition = TextFlowDAO.buildContentStateCondition( - FilterConstraints.builder().keepNone().build(), "tft"); + ContentStateGroup.builder().removeAll().build(), "tft"); assertThat("Conditional that accepts all should be '1'", contentStateCondition, is("1")); } @Test public void canBuildNewOnlyConditional() { - FilterConstraints constraints = FilterConstraints.builder() - .keepNone().includeNew().build(); - String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); + ContentStateGroup contentStates = ContentStateGroup.builder() + .removeAll().includeNew(true).build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(contentStates, "tft"); assertThat(contentStateCondition, is("(tft.state=0 or tft.state is null)")); } @Test public void canBuildFuzzyOnlyConditional() { - FilterConstraints constraints = FilterConstraints.builder() - .keepNone().includeFuzzy().build(); - String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); - assertThat(contentStateCondition, is("(tft.state=1 or tft.state=4)")); + ContentStateGroup contentStates = ContentStateGroup.builder() + .removeAll().includeFuzzy(true).build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(contentStates, "tft"); + assertThat(contentStateCondition, is("(tft.state=1)")); } @Test public void canBuildTranslatedOnlyConditional() { - FilterConstraints constraints = FilterConstraints.builder() - .keepNone().includeTranslated().build(); - String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); - assertThat(contentStateCondition, is("(tft.state=2 or tft.state=3)")); + ContentStateGroup contentStates = ContentStateGroup.builder() + .removeAll().includeTranslated(true).build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(contentStates, "tft"); + assertThat(contentStateCondition, is("(tft.state=2)")); } @Test - public void canBuildContentStateQuery() + public void canBuildApprovedOnlyConditional() { - FilterConstraints constraints = FilterConstraints.builder() - .keepNone().includeNew().includeFuzzy().build(); - String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); - assertThat(contentStateCondition, is("(tft.state=1 or tft.state=4 or tft.state=0 or tft.state is null)")); + ContentStateGroup contentStates = ContentStateGroup.builder() + .removeAll().includeApproved(true).build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(contentStates, "tft"); + assertThat(contentStateCondition, is("(tft.state=3)")); + } + + @Test + public void canBuildRejectedOnlyConditional() + { + ContentStateGroup contentStates = ContentStateGroup.builder() + .removeAll().includeRejected(true).build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(contentStates, "tft"); + assertThat(contentStateCondition, is("(tft.state=4)")); + } + + @Test + public void canBuildNewAndFuzzyConditional() + { + ContentStateGroup contentStates = ContentStateGroup.builder() + .removeAll().includeNew(true).includeFuzzy(true).build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(contentStates, "tft"); + assertThat(contentStateCondition, is("(tft.state=0 or tft.state is null or tft.state=1)")); } @Test public void canBuildNewAndTranslatedConditional() { - FilterConstraints constraints = FilterConstraints.builder() - .keepNone().includeNew().includeTranslated().build(); - String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); - assertThat(contentStateCondition, is("(tft.state=2 or tft.state=3 or tft.state=0 or tft.state is null)")); + ContentStateGroup contentStates = ContentStateGroup.builder() + .removeAll().includeNew(true).includeTranslated(true).build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(contentStates, "tft"); + assertThat(contentStateCondition, is("(tft.state=0 or tft.state is null or tft.state=2)")); } @Test public void canBuildFuzzyAndTranslatedConditional() { - FilterConstraints constraints = FilterConstraints.builder() - .keepNone().includeFuzzy().includeTranslated().build(); - String contentStateCondition = TextFlowDAO.buildContentStateCondition(constraints, "tft"); - assertThat(contentStateCondition, is("(tft.state=2 or tft.state=3 or tft.state=1 or tft.state=4)")); + ContentStateGroup contentStates = ContentStateGroup.builder() + .removeAll().includeFuzzy(true).includeTranslated(true).build(); + String contentStateCondition = TextFlowDAO.buildContentStateCondition(contentStates, "tft"); + assertThat(contentStateCondition, is("(tft.state=1 or tft.state=2)")); } @Test From fe9691c874b0ba31fed84d92c9a7854aa5bc25e3 Mon Sep 17 00:00:00 2001 From: Damian Jansen Date: Thu, 11 Jul 2013 13:04:49 +1000 Subject: [PATCH 14/26] Make RFC2822 and username tests use Theories rather than loops Control functions (for, if, etc) in tests are bad. The (experimental) Therories class looks to handle data based tests quite elegantly. Move username and email validation to separate classes to test. Also, fix minor bug in GlossaryTestSuite (database reset rules) and give the Register tests a waitFor for the fields/errors to become visible, to prevent element stale / not found problems. --- .../org/zanata/page/account/RegisterPage.java | 69 ++- .../main/java/org/zanata/util/RFC2822.java | 429 +++++++++--------- .../feature/account/RFC2822NegativeTest.java | 78 ++++ .../feature/account/RFC2822PositiveTest.java | 56 +++ .../feature/account/RegisterDetailedTest.java | 130 +----- .../feature/account/RegisterTestSuite.java | 42 ++ .../account/UsernameValidationTest.java | 79 ++++ .../feature/glossary/GlossaryTestSuite.java | 2 +- 8 files changed, 560 insertions(+), 325 deletions(-) create mode 100644 functional-test/src/test/java/org/zanata/feature/account/RFC2822NegativeTest.java create mode 100644 functional-test/src/test/java/org/zanata/feature/account/RFC2822PositiveTest.java create mode 100644 functional-test/src/test/java/org/zanata/feature/account/RegisterTestSuite.java create mode 100644 functional-test/src/test/java/org/zanata/feature/account/UsernameValidationTest.java diff --git a/functional-test/src/main/java/org/zanata/page/account/RegisterPage.java b/functional-test/src/main/java/org/zanata/page/account/RegisterPage.java index bc61baa12c..d2c4c834cb 100644 --- a/functional-test/src/main/java/org/zanata/page/account/RegisterPage.java +++ b/functional-test/src/main/java/org/zanata/page/account/RegisterPage.java @@ -1,12 +1,37 @@ +/* + * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the + * @author tags. See the copyright.txt file in the distribution for a full + * listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This software is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this software; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF + * site: http://www.fsf.org. + */ package org.zanata.page.account; -import java.util.Map; +import com.google.common.base.Function; +import lombok.extern.slf4j.Slf4j; +import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; import org.zanata.page.AbstractPage; -import lombok.extern.slf4j.Slf4j; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; /** * @author Damian Jansen djansen@redhat.com @@ -42,6 +67,16 @@ public class RegisterPage extends AbstractPage public RegisterPage(WebDriver driver) { super(driver); + List elements = new ArrayList(); + elements.add("registerForm:nameField:name"); + elements.add("registerForm:emailField:email"); + elements.add("registerForm:usernameField:username"); + elements.add("registerForm:passwordField:password"); + elements.add("registerForm:passwordConfirmField:passwordConfirm"); + elements.add("registerForm:captcha:verifyCaptcha"); + elements.add("registerForm:agreedToTerms:agreedToTerms"); + elements.add("registerForm:registerButton"); + waitForPage(elements); } public RegisterPage enterName(String name) @@ -86,6 +121,7 @@ public RegisterPage clickTerms() return new RegisterPage(getDriver()); } + // TODO: Add a "signup success" page public AbstractPage register() { registerButton.click(); @@ -124,4 +160,33 @@ public RegisterPage setFields(Map fields) enterCaptcha(fields.get("captcha")); return new RegisterPage(getDriver()); } + + public List waitForErrors() + { + waitForTenSec().until(new Function() + { + @Override + public WebElement apply(WebDriver driver) + { + return getDriver().findElement(By.xpath("//span[@class='errors']")); + } + }); + return getErrors(); + } + + /* + * Wait for all necessary entities to be available + */ + private void waitForPage(List elements) { + for (final String element : elements) { + waitForTenSec().until(new Function() + { + @Override + public WebElement apply(WebDriver driver) + { + return getDriver().findElement(By.id(element)); + } + }); + } + } } diff --git a/functional-test/src/main/java/org/zanata/util/RFC2822.java b/functional-test/src/main/java/org/zanata/util/RFC2822.java index d4937a46a6..f30784ec5f 100644 --- a/functional-test/src/main/java/org/zanata/util/RFC2822.java +++ b/functional-test/src/main/java/org/zanata/util/RFC2822.java @@ -1,3 +1,23 @@ +/* + * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the + * @author tags. See the copyright.txt file in the distribution for a full + * listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This software is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this software; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF + * site: http://www.fsf.org. + */ package org.zanata.util; import java.util.HashMap; @@ -20,10 +40,6 @@ public class RFC2822 { * e.g. me, myself, example, com in me.myself@example.com * quote / quoting: a section of the localpart contained within quotation marks * - * BUG982048 - * This defect is an list of all items that, valid or invalid, are not correctly recognised by the email validation - * in Zanata. - * * Untested: * RFC2822, section 3.4.1 * The contents of a bracketed domain can have a \ precede a character to escape it, @@ -37,246 +53,235 @@ public class RFC2822 { * The maximum allowable length of an email address is 320 characters. */ - public static Map invalidEmailAddresses() - { - Map invalidEmailAddresses = new HashMap(); - - /* - * RFC 2822, section 3.4.1 - * Email addresses consist of a local part, the "@" symbol, and the domain. - */ - invalidEmailAddresses.put("3.4.1 Plain address", "plainaddress"); - invalidEmailAddresses.put("3.4.1 Missing @", "email.example.com"); - invalidEmailAddresses.put("3.4.1 Missing localpart", "@example.com"); - invalidEmailAddresses.put("3.4.1 Missing domain", "email@"); - invalidEmailAddresses.put("3.4.1 Two @ sign", "email@example@example.com"); + /* + * VALID EMAIL ADDRESSES + */ + /* + * RFC 2822, section 3.4.1 + * Email addresses consist of a local part, the "@" symbol, and the domain. + */ + public static String BASIC_EMAIL = "email@example.com"; - /* - * RFC 2822, section 3.4.1 - * No periods can start or end the local part. - * Two periods together is invalid. - */ - invalidEmailAddresses.put("3.4.1 Leading dot", ".email@example.com"); - invalidEmailAddresses.put("3.4.1 Trailing dot", "email.@example.com"); - invalidEmailAddresses.put("3.4.1 Multiple dots", "email..email@example.com"); + /* + * RFC 2822, sections 3.4.1 and 4.4 + * The local part can be unquoted, quoted in its entirety, or quoted on a per-label basis. + * The quoted local part starts with a quotation mark, ends with a quotation mark. + */ + // BUG982048 + public static String BASIC_QUOTED_EMAIL = "\"email\"@example.com"; - /* - * RFC 2822, section 2.2 - * All email addresses are in 7-bit US ASCII. - */ - // BUG982048invalidEmailAddresses.put("3.4.1 Non unicode characters", "あいうえお@example.com"); + /* + * RFC 2822, section 3.4.1 + * TEXT can contain alphabetic, numeric, and these symbols: !#$%'*+-/=?^_`{|}~ + */ + public static String SPECIAL_CHARACTERS_LOCALPART = "email.!#$%'*+-/=?^_`{|}~.dot@example.com"; - /* - * RFC 2822, section 3.4.1 - * Unquoted local parts can consist of TEXT - * TEXT can contain: - * alphabetic - * numeric - * and symbols !#$%'*+-/=?^_`{|}~ - */ - invalidEmailAddresses.put("3.4.1 Invalid unquoted character", "test,user@example.com"); - invalidEmailAddresses.put("3.4.1 Invalid unquoted character", "test(user@example.com"); - invalidEmailAddresses.put("3.4.1 Invalid unquoted character", "test)user@example.com"); + /* + RFC 2822, section 4.4 + If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted + or quoted chunks separated by periods. + */ + public static String ENCLOSED_QUOTED_LABEL = "dot.\"email\".dot@example.com"; + public static String LOCALPART_WITH_EMPTY_QUOTE = "dot.\"\".dot@example.com"; - /* - * RFC 2822, section 3.4.1 - * The quoted local part starts with a quotation mark, ends with a quotation mark. - */ - invalidEmailAddresses.put("3.4.1 Invalid quoting", "test\"user@example.com"); + /* + * RFC 2822, section 3.4.1 + * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). + * This supersedes the previous rule, allowing spaces and quotation marks in the email address as long as they + * are escaped. + */ + public static String QUOTED_ESCAPED_SPECIAL_CHARACTERS = "email.\"(),:;<>\\@\\[\\]\\\\\"@example.com"; + public static String QUOTED_ESCAPED_QUOTES = "email.\"\\\"\"@example.com"; + public static String QUOTED_WITH_SPACE = "\"special\\ email\"@example.com"; - /* - * RFC 2822, section 4.4 - * If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted - * or quoted chunks separated by periods - */ - invalidEmailAddresses.put("3.4.1 Invalid quoting", "\"test\"user@example.com"); - invalidEmailAddresses.put("4.4 Invalid quoting", "\"test\"quote\"user@example.com"); + /* + * RFC 2822, section 3.4.1 + * The domain can be bracketed or plain. + */ + public static String BRACKETED_DOMAIN = "email@[example.com]"; + public static String BRACKETED_IPV4_DOMAIN = "email@[123.45.67.89]"; + public static String BRACKETED_IPV6_DOMAIN = "email@[IPv6:2001:2d12:c4fe:5afe::1]"; - /* - * RFC 2822, section 3.4.1 - * The contents of a quoted local part can not contain characters: - * 9 (TAB) - * 10 (LF) - * 13 (CR) - * 32 (space) - * 34 (") - * 91-94 ([, \, ], ^) - */ - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test,user\"@example.com"); - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test\\user\"@example.com"); - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test[user\"@example.com"); - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test]user\"@example.com"); - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test^user\"@example.com"); - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test user\"@example.com"); - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "\"test\"user\"@example.com"); - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "test.\"".concat("\t").concat("\".user@example.com")); + /* + * RFC 1035, section 2.3.4 + * A plain domain consists of labels separated with periods. No period can start or end a domain name. + */ + public static String LOCALPART_MULTIPLE_LABELS = "another.email@example.com"; + public static String DOMAIN_MULTIPLE_LABELS = "email@another.example.com"; - /* - * RFC 2822, section 3.4.1 - * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). - */ - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "test.\"\\".concat("\r").concat("\".user@example.com")); - invalidEmailAddresses.put("3.4.1 Invalid quoted character", "test.\"\\".concat("\n").concat("\".user@example.com")); + /* + * RFC 1035, section 2.3.4 + * The maximum length of a label is 63 characters. + */ + public static String DOMAIN_LABEL_MAX_CHARACTERS = + "email@B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8.com"; + public static String LOCALPART_LABEL_MAX_CHARACTERS = + "B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8@example.com"; /* * RFC 1035, section 2.3.4 - * A plain domain consists of labels separated with periods. No period can start or end a domain name. - * No two periods in succession can be in a domain name. + * A label may contain hyphens, but no two hyphens in a row. */ - invalidEmailAddresses.put("RFC1035-2.3.4 Trailing dot in domain", "email@example.com."); - invalidEmailAddresses.put("RFC1035-2.3.4 Leading dot in domain", "email@.example.com"); - invalidEmailAddresses.put("RFC1035-2.3.4 Multiple dots in domain", "email@example..com"); + public static String HYPHENATED_DOMAIN_LABEL = "email@another-example.com"; + public static String HYPHENATED_LOCALPART_LABEL = "my-email@example.com"; - /* - * RFC 2822, section 3.4.1 - * Bracketed domains must: - * start with [, end with ] - * not contain characters 9 (TAB), 10 (LF), 13 (CR), 32 (space), 91-94 ([, \, ], ^) - */ - invalidEmailAddresses.put("3.4.1 Incorrectly quoted domain", "email@[example].com"); - invalidEmailAddresses.put("3.4.1 Incorrectly quoted domain", "email@[ex^ample.com]"); - invalidEmailAddresses.put("3.4.1 Incorrectly quoted domain", "email@[exa\\mple].com"); + /* + * RFC 2821, section 4.5.3.1 + * The maximum length of the local part is 64 characters. + */ + public static String LOCALPART_MAX_LENGTH = + "B3NQyUsDdzODMoymfDdifn6Wztx2wrivm.80LEngHGl182frm6ifCPyv5SntbDg8@example.com"; - /* - * RFC 1035, section 2.3.4 - * The maximum length of a label is 63 characters. - */ - // BUG982048 invalidEmailAddresses.put("RFC1035-2.3.4 Domain label too long", - // "email@IJUr9P6Y7Fx7rFy4sziQDT0qvSC7XKK6jrD0CNC41jorAKgFYIXLTN5ITJLohy58.com"); - /* - * RFC 1035, section 2.3.4 - * A label may contain hyphens, but no two hyphens in a row. - * A label must not start nor end with a hyphen. - */ - // BUG982048 invalidEmailAddresses.put("2.3.4 Leading dash in domain", "email@-example.com"); - // BUG982048 invalidEmailAddresses.put("2.3.4 Trailing dash in domain", "email@example-.com"); - // BUG982048 invalidEmailAddresses.put("2.3.4 Multiple dashes in domain", "email@exa--mple.com"); - invalidEmailAddresses.put("2.3.4 Leading dash in bracketed domain", "email@[-example.com]"); - invalidEmailAddresses.put("2.3.4 Trailing dash in bracketed domain", "email@[example.com-]"); - invalidEmailAddresses.put("2.3.4 Multiple dashes in bracketed domain", "email@[exa--mple.com]"); + /* + * INVALID EMAIL ADDRESSES + */ - /* - * The contents of a bracketed domain can have a \ precede a character to escape it, and the following character - * must not be 10 (LF) or 13 (CR). - */ - invalidEmailAddresses.put("3.4.1 Invalid bracketed domain", "test@[\\".concat("\r").concat("example.com]")); - invalidEmailAddresses.put("3.4.1 Invalid bracketed domain", "test@[\\".concat("\n").concat("example.com]")); + /* + * RFC 2822, section 3.4.1 + * Email addresses consist of a local part, the "@" symbol, and the domain. + */ + public static String PLAIN_ADDRESS = "plainaddress"; + public static String MISSING_AMPERSAT = "email.example.com"; + public static String MISSING_LOCALPART = "@example.com"; + public static String MISSING_DOMAIN = "email@"; + public static String MULTIPLE_APERSAT = "email@example@example.com"; - /* - * RFC 2821, section 4.5.3.1 - * The maximum length of the local part is 64 characters. - */ - invalidEmailAddresses.put("RFC2821-4.5.3.1 Max localpart length is 64", - "emailuhpealgyxntsh5upl5gqn5a4ruqs7mw6wz21j6dn72amzwozqlyua4jx16rd@example.com"); + /* + * RFC 2822, section 3.4.1 + * No periods can start or end the local part. + * Two periods together is invalid. + */ + public static String LEADING_DOT = ".email@example.com"; + public static String TRAILING_DOT = "email.@example.com"; + public static String MULTIPLE_DOTS = "email..email@example.com"; - /* - * RFC 3696, section 2 - * The top level domain must be all alphabetic. - */ - invalidEmailAddresses.put("RFC3696-2 Encoded html", "Joe Smith "); - invalidEmailAddresses.put("RFC3696-2 Following text", "email@example.com (Joe Smith)"); - // BUG982048 invalidEmailAddresses.put("RFC3696-2 Invalid IP", "email@111.222.333.44444"); + /* + * RFC 2822, section 2.2 + * All email addresses are in 7-bit US ASCII. + */ + public static String NON_UNICODE_CHARACTERS = "あいうえお@example.com"; - /* - * RFC 2821, section 4.5.3.1 - * The maximum length of a "useful" email address is 255 characters. - */ - /* - * BUG982048 - invalidEmailAddresses.put("4.5.3.1 Max email length is 255", - "email@"+ - "Hk3yhCtbBRw3wCT76tL1ryAdfrIaaDszHqvZqnNrZPlNn3Wd7u."+ - "RfpxrueSghp9dkGTGwT9s0fyJL850Sned72RD3Mm5PpEh6QJwQ."+ - "3CeXyEHQEhXNOQdWhYVjGBLzlHz1sJfi4lfn7ighLXcxa5cMAK."+ - "jFXsG8BVsvkODKktTXJ70bQmDWtWQzuh3oz4twumVArDGEbzS1."+ - "slyaBcQqVgUdqXTBdbMY7YJxZwrzZQBBGjCl4e.com"); - */ - return invalidEmailAddresses; - } + /* + * RFC 2822, section 3.4.1 + * Unquoted local parts can consist of TEXT + * TEXT can contain: + * alphabetic + * numeric + * and symbols !#$%'*+-/=?^_`{|}~ + */ + public static String INVALID_UNQUOTED_COMMA = "test,user@example.com"; + public static String INVALID_UNQUOTED_LEFT_PARENTHESES = "test(user@example.com"; + public static String INVALID_UNQUOTED_RIGHT_PARENTHESES = "test)user@example.com"; /* - * An map of valid emails conforming to RFC2822 + * RFC 2822, section 3.4.1 + * The quoted local part starts with a quotation mark, ends with a quotation mark. */ - public static Map validEmailAddresses() - { - Map validEmailAddresses = new HashMap(); + public static String INVALID_SINGLE_QUOTING = "test\"user@example.com"; - /* - * RFC 2822, section 3.4.1 - * Email addresses consist of a local part, the "@" symbol, and the domain. - */ - validEmailAddresses.put("3.4.1 Basic email", "email@example.com"); + /* + * RFC 2822, section 4.4 + * If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted + * or quoted chunks separated by periods + */ + public static String INVALID_QUOTING_SEPARATION = "\"test\"user@example.com"; - /* - * RFC 2822, sections 3.4.1 and 4.4 - * The local part can be unquoted, quoted in its entirety, or quoted on a per-label basis. - * The quoted local part starts with a quotation mark, ends with a quotation mark. - */ - // BUG982048 validEmailAddresses.put("3.4.1 Basic quoted email", "\"email\"@example.com"); + /* + * RFC 2822, section 3.4.1 + * The contents of a quoted local part can not contain characters: + * 9 (TAB) + * 10 (LF) + * 13 (CR) + * 32 (space) + * 34 (") + * 91-94 ([, \, ], ^) + */ + public static String INVALID_QUOTED_COMMA = "\"test,user\"@example.com"; + public static String INVALID_QUOTED_BACKSLASH = "\"test\\user\"@example.com"; + public static String INVALID_QUOTED_LEFT_BRACKET = "\"test[user\"@example.com"; + public static String INVALID_QUOTED_RIGHT_BRACKET = "\"test]user\"@example.com"; + public static String INVALID_QUOTED_CARAT = "\"test^user\"@example.com"; + public static String INVALID_QUOTED_SPACE = "\"test user\"@example.com"; + public static String INVALID_QUOTED_QUOTE = "\"test\"user\"@example.com"; + // TODO: public static String Invalid_quoted_tab = "test.\"".concat("\t").concat("\".user@example.com"); - /* - * RFC 2822, section 3.4.1 - * TEXT can contain alphabetic, numeric, and these symbols: !#$%'*+-/=?^_`{|}~ - */ - validEmailAddresses.put("3.4.1 Allowed special characters in localpart", "email.!#$%'*+-/=?^_`{|}~.dot@example.com"); + /* + * RFC 2822, section 3.4.1 + * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). + */ + public static String INVALID_QUOTED_RETURN = "test.\"\\".concat("\r").concat("\".user@example.com"); + public static String INVALID_QUOTED_LINEFEED = "test.\"\\".concat("\n").concat("\".user@example.com"); - /* - RFC 2822, section 4.4 - If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted - or quoted chunks separated by periods. - */ - // BUG982048 validEmailAddresses.put("4.4 Quoted label with surrounding labels", "dot.\"email\".dot@example.com"); - // BUG982048 validEmailAddresses.put("4.4 Localpart with empty quote", "dot.\"\".dot@example.com"); + /* + * RFC 1035, section 2.3.4 + * A plain domain consists of labels separated with periods. No period can start or end a domain name. + * No two periods in succession can be in a domain name. + */ + public static String TRAILING_DOMAIN_DOT = "email@example.com."; + public static String LEADING_DOMAIN_DOT = "email@.example.com"; + public static String SUCCESSIVE_DOMAIN_DOTS = "email@example..com"; - /* - * RFC 2822, section 3.4.1 - * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). - * This supersedes the previous rule, allowing spaces and quotation marks in the email address as long as they - * are escaped. - */ - // BUG982048 validEmailAddresses.put("3.4.1 Quoted email with escaped special characters", "email.\"(),:;<>\\@\\[\\]\\\\\"@example.com"); - // BUG982048 validEmailAddresses.put("3.4.1 Quoted email with escaped quotes", "email.\"\\\"\"@example.com"); - // BUG982048 validEmailAddresses.put("3.4.1 Quoted email with space character", "\"special\\ email\"@example.com"); + /* + * RFC 2822, section 3.4.1 + * Bracketed domains must: + * start with [, end with ] + * not contain characters 9 (TAB), 10 (LF), 13 (CR), 32 (space), 91-94 ([, \, ], ^) + */ + public static String INCORRECTLY_BRACKETED_DOMAIN = "email@[example].com"; + public static String INVALID_DOMAIN_CHARACTER = "email@[ex^ample.com]"; + public static String INCORRECTLY_ESCAPED_DOMAIN = "email@[exa\\mple.com]"; - /* - * RFC 2822, section 3.4.1 - * The domain can be bracketed or plain. - */ - // BUG982048 validEmailAddresses.put("3.4.1 Email with bracketed domain", "email@[example.com]"); - // BUG982048 validEmailAddresses.put("3.4.1 Bracketed IPv6 domain", "email@[123.45.67.89]"); - // BUG982048 validEmailAddresses.put("3.4.1 Bracketed IPv6 domain", "email@[IPv6:2001:2d12:c4fe:5afe::1]"); + /* + * RFC 1035, section 2.3.4 + * The maximum length of a label is 63 characters. + */ + public static String DOMAIN_LABEL_LENGTH_EXCEEDED = + "email@IJUr9P6Y7Fx7rFy4sziQDT0qvSC7XKK6jrD0CNC41jorAKgFYIXLTN5ITJLohy58.com"; - /* - * RFC 1035, section 2.3.4 - * A plain domain consists of labels separated with periods. No period can start or end a domain name. - */ - validEmailAddresses.put("RFC1035-2.3.4 Localpart with multiple labels", "another.email@example.com"); - validEmailAddresses.put("RFC1035-2.3.4 Domain with multiple labels", "email@another.example.com"); + /* + * RFC 1035, section 2.3.4 + * A label may contain hyphens, but no two hyphens in a row. + * A label must not start nor end with a hyphen. + */ + public static String LEADING_DASH_DOMAIN = "email@-example.com"; + public static String TRAILING_DASH_DOMAIN = "email@example-.com"; + public static String MULTIPLE_DASHES_DOMAIN = "email@exa--mple.com"; + public static String LEADING_DASH_BRACKETED_DOMAIN = "email@[-example.com]"; + public static String TRAILING_DASH_BRACKETED_DOMAIN = "email@[example.com-]"; + public static String MULTIPLE_DASHES_BRACKETED_DOMAIN = "email@[exa--mple.com]"; /* - * RFC 1035, section 2.3.4 - * The maximum length of a label is 63 characters. + * The contents of a bracketed domain can have a \ precede a character to escape it, and the following character + * must not be 10 (LF) or 13 (CR). */ - validEmailAddresses.put("RFC1035-2.3.4 Domain label of 63 characters", - "email@B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8.com"); - validEmailAddresses.put("RFC1035-2.3.4 Localpart label of 63 characters", - "B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8@example.com"); + public static String INVALID_BRACKETED_DOMAIN_RETURN = "test@[\\".concat("\r").concat("example.com]"); + public static String INVALID_BRACKETED_DOMAIN_LINEFEED = "test@[\\".concat("\n").concat("example.com]"); - /* - * RFC 1035, section 2.3.4 - * A label may contain hyphens, but no two hyphens in a row. - */ - validEmailAddresses.put("RFC1035-2.3.4 Hyphenated domain label", "email@another-example.com"); - validEmailAddresses.put("RFC1035-2.3.4 Hyphenated localpart label", "my-email@example.com"); + /* + * RFC 2821, section 4.5.3.1 + * The maximum length of the local part is 64 characters. + */ + public static String LOCALPART_LENGTH_EXCEEDED = + "emailuhpealgyxntsh5upl5gqn5a4ruqs7mw6wz21j6dn72amzwozqlyua4jx16rd@example.com"; - /* - * RFC 2821, section 4.5.3.1 - * The maximum length of the local part is 64 characters. - */ - validEmailAddresses.put("RFC1035-2.3.4 Localpart length of 64 characters", - "B3NQyUsDdzODMoymfDdifn6Wztx2wrivm.80LEngHGl182frm6ifCPyv5SntbDg8@example.com"); + /* + * RFC 3696, section 2 + * The top level domain must be all alphabetic. + */ + public static String INVALID_ENCODED_HTML = "Joe Smith "; + public static String INVALID_FOLLOWING_TEXT = "email@example.com (Joe Smith)"; + public static String INVALID_IP_FORMAT = "email@111.222.333.44444"; - return validEmailAddresses; - } + /* + * RFC 2821, section 4.5.3.1 + * The maximum length of a "useful" email address is 255 characters. + */ + public static String MAX_EMAIL_LENGTH_EXCEEDED = + "email@"+ + "Hk3yhCtbBRw3wCT76tL1ryAdfrIaaDszHqvZqnNrZPlNn3Wd7u."+ + "RfpxrueSghp9dkGTGwT9s0fyJL850Sned72RD3Mm5PpEh6QJwQ."+ + "3CeXyEHQEhXNOQdWhYVjGBLzlHz1sJfi4lfn7ighLXcxa5cMAK."+ + "jFXsG8BVsvkODKktTXJ70bQmDWtWQzuh3oz4twumVArDGEbzS1."+ + "slyaBcQqVgUdqXTBdbMY7YJxZwrzZQBBGjCl4e.com"; } \ No newline at end of file diff --git a/functional-test/src/test/java/org/zanata/feature/account/RFC2822NegativeTest.java b/functional-test/src/test/java/org/zanata/feature/account/RFC2822NegativeTest.java new file mode 100644 index 0000000000..ea434e633d --- /dev/null +++ b/functional-test/src/test/java/org/zanata/feature/account/RFC2822NegativeTest.java @@ -0,0 +1,78 @@ +package org.zanata.feature.account; + +import org.hamcrest.Matchers; +import org.junit.ClassRule; +import org.junit.experimental.theories.DataPoint; +import org.junit.experimental.theories.Theories; +import org.junit.experimental.theories.Theory; +import org.junit.runner.RunWith; +import org.zanata.page.account.RegisterPage; +import org.zanata.util.RFC2822; +import org.zanata.util.ResetDatabaseRule; +import org.zanata.workflow.BasicWorkFlow; + +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * @author Damian Jansen djansen@redhat.com + */ +@RunWith(Theories.class) +public class RFC2822NegativeTest { + @DataPoint public static String PLAIN_ADDRESS = RFC2822.PLAIN_ADDRESS; + @DataPoint public static String MISSING_AMPERSAT = RFC2822.MISSING_AMPERSAT; + @DataPoint public static String MISSING_LOCALPART = RFC2822.MISSING_LOCALPART; + @DataPoint public static String MISSING_DOMAIN = RFC2822.MISSING_DOMAIN; + @DataPoint public static String MULTIPLE_APERSAT = RFC2822.MULTIPLE_APERSAT; + @DataPoint public static String LEADING_DOT = RFC2822.LEADING_DOT; + @DataPoint public static String TRAILING_DOT = RFC2822.TRAILING_DOT; + @DataPoint public static String MULTIPLE_DOTS = RFC2822.MULTIPLE_DOTS; + @DataPoint public static String INVALID_UNQUOTED_COMMA = RFC2822.INVALID_UNQUOTED_COMMA; + @DataPoint public static String INVALID_UNQUOTED_LEFT_PARENTHESES = RFC2822.INVALID_UNQUOTED_LEFT_PARENTHESES; + @DataPoint public static String INVALID_UNQUOTED_RIGHT_PARENTHESES = RFC2822.INVALID_UNQUOTED_RIGHT_PARENTHESES; + @DataPoint public static String INVALID_SINGLE_QUOTING = RFC2822.INVALID_SINGLE_QUOTING; + @DataPoint public static String INVALID_QUOTING_SEPARATION = RFC2822.INVALID_QUOTING_SEPARATION; + @DataPoint public static String INVALID_QUOTED_COMMA = RFC2822.INVALID_QUOTED_COMMA; + @DataPoint public static String INVALID_QUOTED_BACKSLASH = RFC2822.INVALID_QUOTED_BACKSLASH; + @DataPoint public static String INVALID_QUOTED_LEFT_BRACKET = RFC2822.INVALID_QUOTED_LEFT_BRACKET; + @DataPoint public static String INVALID_QUOTED_RIGHT_BRACKET = RFC2822.INVALID_QUOTED_RIGHT_BRACKET; + @DataPoint public static String INVALID_QUOTED_CARAT = RFC2822.INVALID_QUOTED_CARAT; + @DataPoint public static String INVALID_QUOTED_SPACE = RFC2822.INVALID_QUOTED_SPACE; + @DataPoint public static String INVALID_QUOTED_QUOTE = RFC2822.INVALID_QUOTED_QUOTE; + @DataPoint public static String INVALID_QUOTED_RETURN = RFC2822.INVALID_QUOTED_RETURN; + @DataPoint public static String INVALID_QUOTED_LINEFEED = RFC2822.INVALID_QUOTED_LINEFEED; + @DataPoint public static String TRAILING_DOMAIN_DOT = RFC2822.TRAILING_DOMAIN_DOT; + @DataPoint public static String LEADING_DOMAIN_DOT = RFC2822.LEADING_DOMAIN_DOT; + @DataPoint public static String SUCCESSIVE_DOMAIN_DOTS = RFC2822.SUCCESSIVE_DOMAIN_DOTS; + @DataPoint public static String INCORRECTLY_BRACKETED_DOMAIN = RFC2822.INCORRECTLY_BRACKETED_DOMAIN; + @DataPoint public static String INVALID_DOMAIN_CHARACTER = RFC2822.INVALID_DOMAIN_CHARACTER; + @DataPoint public static String INCORRECTLY_ESCAPED_DOMAIN = RFC2822.INCORRECTLY_ESCAPED_DOMAIN; + @DataPoint public static String DOMAIN_LABEL_LENGTH_EXCEEDED = RFC2822.DOMAIN_LABEL_LENGTH_EXCEEDED; + @DataPoint public static String LEADING_DASH_BRACKETED_DOMAIN = RFC2822.LEADING_DASH_BRACKETED_DOMAIN; + @DataPoint public static String TRAILING_DASH_BRACKETED_DOMAIN = RFC2822.TRAILING_DASH_BRACKETED_DOMAIN; + @DataPoint public static String MULTIPLE_DASHES_BRACKETED_DOMAIN = RFC2822.MULTIPLE_DASHES_BRACKETED_DOMAIN; + @DataPoint public static String INVALID_BRACKETED_DOMAIN_RETURN = RFC2822.INVALID_BRACKETED_DOMAIN_RETURN; + @DataPoint public static String INVALID_BRACKETED_DOMAIN_LINEFEED = RFC2822.INVALID_BRACKETED_DOMAIN_LINEFEED; + @DataPoint public static String LOCALPART_LENGTH_EXCEEDED = RFC2822.LOCALPART_LENGTH_EXCEEDED; + @DataPoint public static String INVALID_ENCODED_HTML = RFC2822.INVALID_ENCODED_HTML; + @DataPoint public static String INVALID_FOLLOWING_TEXT = RFC2822.INVALID_FOLLOWING_TEXT; + + // BUG982048 @DataPoint public static String INVALID_IP_FORMAT = RFC2822.INVALID_IP_FORMAT; + // BUG982048 @DataPoint public static String MAX_EMAIL_LENGTH_EXCEEDED = RFC2822.MAX_EMAIL_LENGTH_EXCEEDED; + // BUG982048 @DataPoint public static String NON_UNICODE_CHARACTERS = RFC2822.NON_UNICODE_CHARACTERS; + // BUG982048 @DataPoint public static String LEADING_DASH_DOMAIN = RFC2822.LEADING_DASH_DOMAIN; + // BUG982048 @DataPoint public static String TRAILING_DASH_DOMAIN = RFC2822.TRAILING_DASH_DOMAIN; + // BUG982048 @DataPoint public static String MULTIPLE_DASHES_DOMAIN = RFC2822.MULTIPLE_DASHES_DOMAIN; + + @ClassRule + public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(); + + @Theory + public void invalidEmailRejection(String emailAddress) + { + String errorMsg = "not a well-formed email address"; + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration(); + registerPage = registerPage.enterEmail(emailAddress).clickTerms(); + assertThat("Email validation errors are not shown", registerPage.waitForErrors(), Matchers.hasItem(errorMsg)); + } + +} diff --git a/functional-test/src/test/java/org/zanata/feature/account/RFC2822PositiveTest.java b/functional-test/src/test/java/org/zanata/feature/account/RFC2822PositiveTest.java new file mode 100644 index 0000000000..c9a1932986 --- /dev/null +++ b/functional-test/src/test/java/org/zanata/feature/account/RFC2822PositiveTest.java @@ -0,0 +1,56 @@ +package org.zanata.feature.account; + +import org.hamcrest.Matchers; +import org.junit.ClassRule; +import org.junit.experimental.theories.DataPoint; +import org.junit.experimental.theories.Theories; +import org.junit.experimental.theories.Theory; +import org.junit.runner.RunWith; +import org.zanata.page.account.RegisterPage; +import org.zanata.util.RFC2822; +import org.zanata.util.ResetDatabaseRule; +import org.zanata.workflow.BasicWorkFlow; + +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * @author Damian Jansen djansen@redhat.com + */ +@RunWith(Theories.class) +public class RFC2822PositiveTest { + + @DataPoint public static String BASIC_EMAIL = RFC2822.BASIC_EMAIL; + @DataPoint public static String SPECIAL_LOCALPART_CHARACTERS = RFC2822.SPECIAL_CHARACTERS_LOCALPART; + @DataPoint public static String LOCALPART_MULTIPLE_LABELS = RFC2822.LOCALPART_MULTIPLE_LABELS; + @DataPoint public static String DOMAIN_MULTIPLE_LABELS = RFC2822.DOMAIN_MULTIPLE_LABELS; + @DataPoint public static String DOMAIN_LABEL_MAX_CHARACTERS = RFC2822.DOMAIN_LABEL_MAX_CHARACTERS; + @DataPoint public static String LOCALPART_LABEL_MAX_CHARACTERS = RFC2822.LOCALPART_LABEL_MAX_CHARACTERS; + @DataPoint public static String HYPHENATED_DOMAIN_LABEL = RFC2822.HYPHENATED_DOMAIN_LABEL; + @DataPoint public static String HYPHENATED_LOCALPART_LABEL = RFC2822.HYPHENATED_LOCALPART_LABEL; + @DataPoint public static String LOCALPART_MAX_LENGTH = RFC2822.LOCALPART_MAX_LENGTH; + + // BUG982048 @DataPoint public static String BASIC_QUOTED_EMAIL = RFC2822.BASIC_QUOTED_EMAIL; + // BUG982048 @DataPoint public static String ENCLOSED_QUOTED_LABEL = RFC2822.ENCLOSED_QUOTED_LABEL; + // BUG982048 @DataPoint public static String LOCALPART_EMPTY_QUOTE = RFC2822.LOCALPART_WITH_EMPTY_QUOTE; + // BUG982048 @DataPoint public static String QUOTED_ESCAPED_SPECIAL_CHARACTERS = RFC2822.QUOTED_ESCAPED_SPECIAL_CHARACTERS; + // BUG982048 @DataPoint public static String QUOTED_ESCAPED_QUOTES = RFC2822.QUOTED_ESCAPED_QUOTES; + // BUG982048 @DataPoint public static String QUOTED_WITH_SPACE = RFC2822.QUOTED_WITH_SPACE; + // BUG982048 @DataPoint public static String BRACKETED_DOMAIN = RFC2822.BRACKETED_DOMAIN; + // BUG982048 @DataPoint public static String BRACKETED_IPV4_DOMAIN = RFC2822.BRACKETED_IPV4_DOMAIN; + // BUG982048 @DataPoint public static String BRACKETED_IPV6_DOMAIN = RFC2822.BRACKETED_IPV6_DOMAIN; + + @ClassRule + public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(); + + @Theory + public void validEmailAcceptance(String emailAddress) + { + String errorMsg = "not a well-formed email address"; + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration(); + registerPage = registerPage.enterEmail(emailAddress).clickTerms(); + assertThat("Email validation errors are not shown", registerPage.getErrors(), + Matchers.not(Matchers.hasItem(errorMsg))); + + } + +} diff --git a/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java b/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java index 2b03712090..4c90419e65 100644 --- a/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java +++ b/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java @@ -20,18 +20,22 @@ */ package org.zanata.feature.account; +import lombok.extern.slf4j.Slf4j; import org.hamcrest.Matchers; -import org.junit.*; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Ignore; +import org.junit.Test; import org.zanata.page.HomePage; import org.zanata.page.account.RegisterPage; -import org.zanata.util.ResetDatabaseRule; import org.zanata.util.RFC2822; -import org.zanata.workflow.AbstractWebWorkFlow; +import org.zanata.util.ResetDatabaseRule; +import org.zanata.workflow.BasicWorkFlow; -import java.util.*; +import java.util.HashMap; +import java.util.Map; import static org.hamcrest.MatcherAssert.assertThat; -import lombok.extern.slf4j.Slf4j; /** * @author Damian Jansen djansen@redhat.com @@ -40,7 +44,7 @@ public class RegisterDetailedTest { @ClassRule - public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(ResetDatabaseRule.Config.WithData, ResetDatabaseRule.Config.NoResetAfter); + public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(); Map fields; private HomePage homePage; @@ -59,7 +63,7 @@ public void before() fields.put("password", "testpassword"); fields.put("confirmpassword", "testpassword"); fields.put("captcha", "555"); // TODO: Expect captcha error, fix - homePage = new AbstractWebWorkFlow().goToHome(); + homePage = new BasicWorkFlow().goToHome(); } @Test @@ -92,62 +96,23 @@ public void usernameLengthValidation() assertThat("Size errors are shown for string too long", registerPage.getErrors(), Matchers.hasItem(errorMsg)); } - @Test - public void usernameCharacterValidation() - { - String errorMsg = "lowercase letters and digits (regex \"^[a-z\\d_]{3,20}$\")"; - fields.put("email", "character.test@example.com"); - for (Map.Entry entry : usernameCharacterValidationData().entrySet()) - { - log.info("Test " + entry.getKey() + ":" + entry.getValue()); - fields.put("username", entry.getValue()); - RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); - assertThat("Validation errors are shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); - } - } - @Test @Ignore("Captcha prevents test completion") public void usernamePreExisting() { String errorMsg = "This username is not available"; - fields.put("email", "exists.test@test.com"); - fields.put("username", "alreadyexists"); - RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); - registerPage.register(); - - fields.put("email", "exists2.test@test.com"); - registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); - assertThat("Username not available message is shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().enterUserName("admin"); + assertThat("Username not available message is shown", registerPage.waitForErrors(), Matchers.hasItem(errorMsg)); } @Test public void emailValidation() { String errorMsg = "not a well-formed email address"; + fields.put("email", RFC2822.PLAIN_ADDRESS); fields.put("username", "emailvalidation"); - for (Map.Entry entry : RFC2822.invalidEmailAddresses().entrySet()) - { - log.info("Test " + entry.getKey() + ":" + entry.getValue()); - fields.put("email", entry.getValue()); - RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); - assertThat("Email validation errors are shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); - } - } - - @Test - public void validEmailAcceptance() - { - String errorMsg = "not a well-formed email address"; - fields.put("username", "emailvalidation"); - for (Map.Entry entry : RFC2822.validEmailAddresses().entrySet()) - { - log.info("Test " + entry.getKey() + ":" + entry.getValue()); - fields.put("email", entry.getValue()); - RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); - assertThat("Email validation errors are not shown", registerPage.getErrors(), - Matchers.not(Matchers.hasItem(errorMsg))); - } + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields); + assertThat("Email validation errors are shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); } @Test @@ -158,7 +123,7 @@ public void rejectIncorrectCaptcha() fields.put("email", "rejectbadcaptcha@example.com"); fields.put("captcha", "9000"); - RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields).registerFailure(); + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields).registerFailure(); assertThat("The Captcha entry is rejected", registerPage.getErrors(), Matchers.contains(errorMsg)); } @@ -171,7 +136,7 @@ public void passwordsMatch() fields.put("password", "passwordsmatch"); fields.put("confirmpassword", "passwordsdonotmatch"); - RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields); assertThat("Passwords fail to match error is shown", registerPage.getErrors(), Matchers.contains(errorMsg)); } @@ -185,7 +150,7 @@ public void requiredFields() fields.put("password", ""); fields.put("confirmpassword", ""); - RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields); assertThat("Value is required shows for all fields", registerPage.getErrors(), Matchers.contains(errorMsg, errorMsg, errorMsg, errorMsg, errorMsg)); } @@ -193,68 +158,13 @@ public void requiredFields() /* Bugs */ - @Test(expected = AssertionError.class) - public void bug981082_inaccurateErrorMessage() - { - String errorMsg = "size must be between 3 and 20"; - fields.put("email", "bug981082test@test.com"); - fields.put("username", "mo"); - - RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); - assertThat("Size errors are shown for string too short", registerPage.getErrors(), Matchers.hasItem(errorMsg)); - - fields.put("username", "johndoeusernamevalidation"); - registerPage = registerPage.setFields(fields); - assertThat("Size errors are shown for string too long", registerPage.getErrors(), Matchers.hasItem(errorMsg)); - } - @Test(expected = AssertionError.class) public void bug981498_underscoreRules() { String errorMsg = "lowercase letters and digits (regex \"^[a-z\\d_]{3,20}$\")"; fields.put("email", "bug981498test@example.com"); fields.put("username", "______"); - RegisterPage registerPage = new AbstractWebWorkFlow().goToHome().goToRegistration().setFields(fields); + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields); assertThat("A username of all underscores is not valid", registerPage.getErrors(), Matchers.hasItem(errorMsg)); } - - /* - Returns a hash of invalid characters for a username - Hash is String reason for invalid 'character', String character - */ - private LinkedHashMap usernameCharacterValidationData() - { - LinkedHashMap inputData = new LinkedHashMap(100); - inputData.put("Invalid char |", "user|name"); - inputData.put("Invalid char /", "user/name"); - inputData.put("Invalid char ", "user\\name"); - inputData.put("Invalid char +", "user+name"); - inputData.put("Invalid char *", "user*name"); - inputData.put("Invalid char |", "user|name"); - inputData.put("Invalid char (", "user(name"); - inputData.put("Invalid char )", "user)name"); - inputData.put("Invalid char $", "user$name"); - inputData.put("Invalid char [", "user[name"); - inputData.put("Invalid char ]", "user]name"); - inputData.put("Invalid char :", "user:name"); - inputData.put("Invalid char ;", "user;name"); - inputData.put("Invalid char '", "user'name"); - inputData.put("Invalid char ,", "user,name"); - inputData.put("Invalid char ?", "user?name"); - inputData.put("Invalid char !", "user!name"); - inputData.put("Invalid char @", "user@name"); - inputData.put("Invalid char #", "user#name"); - inputData.put("Invalid char %", "user%name"); - inputData.put("Invalid char ^", "user^name"); - inputData.put("Invalid char =", "user=name"); - inputData.put("Invalid char .", "user.name"); - inputData.put("Invalid char {", "user{name"); - inputData.put("Invalid char }", "user}name"); - // Capital letters are prohibited - for (char c = 'A'; c <= 'Z'; c++) { - String letter = String.valueOf(c); - inputData.put("Invalid capital char ".concat(letter), "user".concat(letter).concat("name")); - } - return inputData; - } } diff --git a/functional-test/src/test/java/org/zanata/feature/account/RegisterTestSuite.java b/functional-test/src/test/java/org/zanata/feature/account/RegisterTestSuite.java new file mode 100644 index 0000000000..5e1f36d8be --- /dev/null +++ b/functional-test/src/test/java/org/zanata/feature/account/RegisterTestSuite.java @@ -0,0 +1,42 @@ +/* + * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the + * @author tags. See the copyright.txt file in the distribution for a full + * listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This software is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this software; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF + * site: http://www.fsf.org. + */ +package org.zanata.feature.account; + +import org.junit.ClassRule; +import org.junit.runner.RunWith; +import org.junit.runners.Suite; +import org.zanata.util.ResetDatabaseRule; + +/** + * @author Damian Jansen djansen@redhat.com + */ +@RunWith(Suite.class) +@Suite.SuiteClasses({ + RegisterDetailedTest.class, + UsernameValidationTest.class, + RFC2822PositiveTest.class, + RFC2822NegativeTest.class +}) +public class RegisterTestSuite +{ + @ClassRule + public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(); +} diff --git a/functional-test/src/test/java/org/zanata/feature/account/UsernameValidationTest.java b/functional-test/src/test/java/org/zanata/feature/account/UsernameValidationTest.java new file mode 100644 index 0000000000..129d3fe45b --- /dev/null +++ b/functional-test/src/test/java/org/zanata/feature/account/UsernameValidationTest.java @@ -0,0 +1,79 @@ +/* + * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the + * @author tags. See the copyright.txt file in the distribution for a full + * listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This software is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this software; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF + * site: http://www.fsf.org. + */ +package org.zanata.feature.account; + +import org.hamcrest.Matchers; +import org.junit.ClassRule; +import org.junit.experimental.theories.DataPoint; +import org.junit.experimental.theories.Theories; +import org.junit.experimental.theories.Theory; +import org.junit.runner.RunWith; +import org.zanata.page.account.RegisterPage; +import org.zanata.util.ResetDatabaseRule; +import org.zanata.workflow.BasicWorkFlow; + +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * @author Damian Jansen djansen@redhat.com + */ +@RunWith(Theories.class) +public class UsernameValidationTest +{ + @ClassRule + public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(); + + @DataPoint public static String INVALID_PIPE = "user|name"; + @DataPoint public static String INVALID_SLASH = "user/name"; + @DataPoint public static String INVALID_BACKSLASH = "user\\name"; + @DataPoint public static String INVALID_PLUS = "user+name"; + @DataPoint public static String INVALID_ASTERISK = "user*name"; + @DataPoint public static String INVALID_LEFT_PARENTHESES = "user(name"; + @DataPoint public static String INVALID_RIGHT_PARENTHESES = "user)name"; + @DataPoint public static String INVALID_DOLLAR = "user$name"; + @DataPoint public static String INVALID_LEFT_BRACKET = "user[name"; + @DataPoint public static String INVALID_RIGHT_BRACKET = "user]name"; + @DataPoint public static String INVALID_COLON = "user:name"; + @DataPoint public static String INVALID_SEMICOLON = "user;name"; + @DataPoint public static String INVALID_APOSTROPHE = "user'name"; + @DataPoint public static String INVALID_COMMA = "user,name"; + @DataPoint public static String INVALID_QUESTION_MARK = "user?name"; + @DataPoint public static String INVALID_EXCLAMATION_MARK = "user!name"; + @DataPoint public static String INVALID_AMPERSAT = "user@name"; + @DataPoint public static String INVALID_HASH = "user#name"; + @DataPoint public static String INVALID_PERCENT = "user%name"; + @DataPoint public static String INVALID_CARAT = "user^name"; + @DataPoint public static String INVALID_EQUALS = "user=name"; + @DataPoint public static String INVALID_PERIOD = "user.name"; + @DataPoint public static String INVALID_LEFT_BRACE = "user{name"; + @DataPoint public static String INVALID_RIGHT_BRACE = "user}name"; + @DataPoint public static String INVALID_CAPITAL_A = "userAname"; + @DataPoint public static String INVALID_CAPITAL_Z = "userZname"; + + @Theory + public void usernameCharacterValidation(String username) + { + String errorMsg = "lowercase letters and digits (regex \"^[a-z\\d_]{3,20}$\")"; + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration(); + registerPage = registerPage.enterUserName(username).clickTerms(); + assertThat("Validation errors are shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); + } +} diff --git a/functional-test/src/test/java/org/zanata/feature/glossary/GlossaryTestSuite.java b/functional-test/src/test/java/org/zanata/feature/glossary/GlossaryTestSuite.java index 37bc3b4289..eebdb08baf 100644 --- a/functional-test/src/test/java/org/zanata/feature/glossary/GlossaryTestSuite.java +++ b/functional-test/src/test/java/org/zanata/feature/glossary/GlossaryTestSuite.java @@ -22,5 +22,5 @@ public class GlossaryTestSuite { @ClassRule - public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(ResetDatabaseRule.Config.NoResetAfter, ResetDatabaseRule.Config.WithData); + public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(ResetDatabaseRule.Config.WithData); } From 2724c49dff05091130c30bcee7b816eaedabd369 Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 11 Jul 2013 16:04:03 +1000 Subject: [PATCH 15/26] make editor request all states when no states checked in UI --- .../webtrans/shared/rpc/GetTransUnitList.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java index 00f3645aea..9a906c8743 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java @@ -33,6 +33,15 @@ private GetTransUnitList(GetTransUnitActionContext context) offset = context.getOffset(); count = context.getCount(); phrase = context.getFindMessage(); + setIncludeStatesFrom(context); + setIncludeAllStateIfNoneSelected(); + filterHasError = context.isFilterHasError(); + targetTransUnitId = context.getTargetTransUnitId(); + validationIds = context.getValidationIds(); + } + + private void setIncludeStatesFrom(GetTransUnitActionContext context) + { // @formatter :off filterStates = ContentStateGroup.builder() .includeNew(context.isFilterUntranslated()) @@ -42,9 +51,14 @@ private GetTransUnitList(GetTransUnitActionContext context) .includeRejected(context.isFilterRejected()) .build(); // @formatter :on - filterHasError = context.isFilterHasError(); - targetTransUnitId = context.getTargetTransUnitId(); - validationIds = context.getValidationIds(); + } + + private void setIncludeAllStateIfNoneSelected() + { + if (filterStates.hasNoStates()) + { + filterStates = ContentStateGroup.builder().addAll().build(); + } } public static GetTransUnitList newAction(GetTransUnitActionContext context) From 8436bd248150d83e2bd41df3d21a580b4f154918 Mon Sep 17 00:00:00 2001 From: David Mason Date: Fri, 12 Jul 2013 13:46:46 +1000 Subject: [PATCH 16/26] prevent server returning all states when none are requested --- .../java/org/zanata/search/FilterConstraints.java | 13 +------------ .../webtrans/client/service/NavigationService.java | 1 - .../webtrans/shared/rpc/GetTransUnitList.java | 3 +-- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java b/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java index d5803bb1da..6b15dc1a0f 100644 --- a/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java +++ b/zanata-war/src/main/java/org/zanata/search/FilterConstraints.java @@ -144,18 +144,7 @@ public Builder checkInTarget(boolean check) public Builder includeStates(ContentStateGroup states) { - //FIXME this behaviour is too surprising. - // It exists because the editor UI should show all states when either - // all or none of the states are checked. This logic should just happen - // in the editor backend *before* sending a request to the server. - if (states.hasNoStates()) - { - this.states.addAll(); - } - else - { - this.states.fromStates(states); - } + this.states.fromStates(states); return this; } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/service/NavigationService.java b/zanata-war/src/main/java/org/zanata/webtrans/client/service/NavigationService.java index 610b6b98e6..0a872a0128 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/service/NavigationService.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/service/NavigationService.java @@ -55,7 +55,6 @@ import org.zanata.webtrans.client.resources.TableEditorMessages; import org.zanata.webtrans.client.rpc.CachingDispatchAsync; import org.zanata.webtrans.shared.auth.EditorClientId; -import org.zanata.webtrans.shared.model.DocumentInfo; import org.zanata.webtrans.shared.model.TransUnit; import org.zanata.webtrans.shared.model.TransUnitId; import org.zanata.webtrans.shared.rpc.GetTransUnitList; diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java index 9a906c8743..b992cc336d 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java @@ -144,8 +144,7 @@ public List getValidationIds() public boolean isAcceptAllStatus() { - //all filter options are checked or unchecked - return filterStates.hasNoStates() && !filterHasError || filterStates.hasAllStates() && filterHasError; + return filterStates.hasAllStates() && !filterHasError; } @Override From 65824b2234bcffde82e7871bc0b5ef6bc3fb4943 Mon Sep 17 00:00:00 2001 From: David Mason Date: Fri, 12 Jul 2013 14:01:54 +1000 Subject: [PATCH 17/26] prevent TextFlowDAO returning all states when none are requested --- .../main/java/org/zanata/dao/TextFlowDAO.java | 14 ++++++++----- .../java/org/zanata/dao/TextFlowDAOTest.java | 21 ++++++------------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java b/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java index e3f613cbbf..70b1308c12 100644 --- a/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java +++ b/zanata-war/src/main/java/org/zanata/dao/TextFlowDAO.java @@ -160,19 +160,23 @@ public List getNavigationByDocumentId(Long documentId, HLocale hLocal } /** - * This will build a SQL query condition in where clause. - * If all status are equal (i.e. all true or all false), it's treated as accept all and it will return '1'. + * Build a SQL query condition that is true only for text flows with one of the given states. * - * @param includedStates + * @param includedStates states of targets that should return true * @param hTextFlowTargetTableAlias alias being used for the target table in the current query - * @return '1' if accept all status or a SQL condition clause with target content state conditions in parentheses '()' joined by 'or' + * @return a valid SQL query condition that is wrapped in parentheses if necessary */ protected static String buildContentStateCondition(ContentStateGroup includedStates, String hTextFlowTargetTableAlias) { - if (includedStates.hasAllStates() || includedStates.hasNoStates()) + if (includedStates.hasAllStates()) { return "1"; } + if (includedStates.hasNoStates()) + { + return "0"; + } + StringBuilder builder = new StringBuilder(); builder.append("("); List conditions = Lists.newArrayList(); diff --git a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java index f9a5d27d2d..43d97cfa48 100644 --- a/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java +++ b/zanata-war/src/test/java/org/zanata/dao/TextFlowDAOTest.java @@ -10,7 +10,6 @@ import org.dbunit.operation.DatabaseOperation; import org.hamcrest.Matchers; -import org.hibernate.Query; import org.hibernate.Session; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -26,7 +25,7 @@ @Slf4j public class TextFlowDAOTest extends ZanataDbunitJpaTest { - + private static final boolean PRINT_TEST_DATA = false; private TextFlowDAO dao; @Override @@ -43,7 +42,10 @@ protected void prepareDBUnitOperations() public void setup() { dao = new TextFlowDAO((Session) getEm().getDelegate()); -// printTestData(); + if (PRINT_TEST_DATA) + { + printTestData(); + } } private void printTestData() @@ -179,13 +181,12 @@ public void canBuildAcceptAllQuery() assertThat("Conditional that accepts all should be '1'", contentStateCondition, is("1")); } - // FIXME the 'none == all' logic should be limited to the editor @Test public void canBuildAcceptAllQueryWhenNoStatesSelected() { String contentStateCondition = TextFlowDAO.buildContentStateCondition( ContentStateGroup.builder().removeAll().build(), "tft"); - assertThat("Conditional that accepts all should be '1'", contentStateCondition, is("1")); + assertThat("Conditional that accepts none should be '0'", contentStateCondition, is("0")); } @Test @@ -285,14 +286,4 @@ public void testGetTextFlowByDocumentIdWithConstraint() assertThat(result, Matchers.hasSize(1)); } - // What is this testing? I can't tell if it is ensuring that no exception is thrown, - // or if it is just half-written and useless. - @Test - public void queryTest1() - { - String queryString = "from HTextFlow tf left join tf.targets tft with (index(tft) = 3) " + - "where (exists (from HTextFlowTarget where textFlow = tf and content0 like '%mssg%'))"; - Query query = getSession().createQuery(queryString); - List result = query.list(); - } } From c910766ab3f6c79cdd48581001f135af4680ac1b Mon Sep 17 00:00:00 2001 From: David Mason Date: Fri, 12 Jul 2013 14:55:13 +1000 Subject: [PATCH 18/26] move rule for requesting all states when none are checked into context class --- .../service/GetTransUnitActionContext.java | 29 ++++++++++++++++++- .../webtrans/shared/rpc/GetTransUnitList.java | 23 ++------------- .../shared/rpc/GetTransUnitsNavigation.java | 6 +--- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/service/GetTransUnitActionContext.java b/zanata-war/src/main/java/org/zanata/webtrans/client/service/GetTransUnitActionContext.java index 322c2b5839..abf063d4f3 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/service/GetTransUnitActionContext.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/service/GetTransUnitActionContext.java @@ -23,6 +23,7 @@ import java.util.List; +import org.zanata.webtrans.shared.model.ContentStateGroup; import org.zanata.webtrans.shared.model.DocumentInfo; import org.zanata.webtrans.shared.model.TransUnitId; import org.zanata.webtrans.shared.model.ValidationId; @@ -292,7 +293,33 @@ public boolean acceptAll() && filterUntranslated == filterHasError && filterHasError == filterApproved && filterApproved == filterRejected; - + return messageFilterAcceptAll && Strings.isNullOrEmpty(findMessage); } + + public ContentStateGroup getCurrentFilterStates() + { + return filterStatesFromCheckboxStates(getCheckboxStates()); + } + + private ContentStateGroup getCheckboxStates() + { + ContentStateGroup checkboxStates = ContentStateGroup.builder() + .includeNew(filterUntranslated) + .includeFuzzy(filterNeedReview) + .includeTranslated(filterTranslated) + .includeApproved(filterApproved) + .includeRejected(filterRejected) + .build(); + return checkboxStates; + } + + private static ContentStateGroup filterStatesFromCheckboxStates(ContentStateGroup filterStates) + { + if (filterStates.hasNoStates()) + { + filterStates = ContentStateGroup.builder().addAll().build(); + } + return filterStates; + } } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java index b992cc336d..35e77ba64c 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitList.java @@ -33,32 +33,15 @@ private GetTransUnitList(GetTransUnitActionContext context) offset = context.getOffset(); count = context.getCount(); phrase = context.getFindMessage(); - setIncludeStatesFrom(context); - setIncludeAllStateIfNoneSelected(); + setIncludeStates(context.getCurrentFilterStates()); filterHasError = context.isFilterHasError(); targetTransUnitId = context.getTargetTransUnitId(); validationIds = context.getValidationIds(); } - private void setIncludeStatesFrom(GetTransUnitActionContext context) + private void setIncludeStates(ContentStateGroup contentStateGroup) { - // @formatter :off - filterStates = ContentStateGroup.builder() - .includeNew(context.isFilterUntranslated()) - .includeFuzzy(context.isFilterNeedReview()) - .includeTranslated(context.isFilterTranslated()) - .includeApproved(context.isFilterApproved()) - .includeRejected(context.isFilterRejected()) - .build(); - // @formatter :on - } - - private void setIncludeAllStateIfNoneSelected() - { - if (filterStates.hasNoStates()) - { - filterStates = ContentStateGroup.builder().addAll().build(); - } + filterStates = ContentStateGroup.builder().fromStates(contentStateGroup).build(); } public static GetTransUnitList newAction(GetTransUnitActionContext context) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java index 1f8cb9a861..0e494d5efc 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/shared/rpc/GetTransUnitsNavigation.java @@ -50,11 +50,7 @@ public GetTransUnitsNavigation(GetTransUnitActionContext context) this(context.getDocument().getId().getId(), context.getFindMessage(), ContentStateGroup.builder() - .includeNew(context.isFilterUntranslated()) - .includeFuzzy(context.isFilterNeedReview()) - .includeTranslated(context.isFilterTranslated()) - .includeApproved(context.isFilterApproved()) - .includeRejected(context.isFilterRejected()) + .fromStates(context.getCurrentFilterStates()) .build()); } From 189970abf372f47227c43e38466d7bd7da16dacf Mon Sep 17 00:00:00 2001 From: Damian Jansen Date: Fri, 12 Jul 2013 16:06:11 +1000 Subject: [PATCH 19/26] Improve the RFC2822 files to be more descriptive and clean Split email addresses into valid and non-valid. Use an enum for the address entries. Improve docs. --- .../main/java/org/zanata/util/RFC2822.java | 287 ------------------ .../rfc2822/InvalidEmailAddressRFC2822.java | 204 +++++++++++++ .../rfc2822/ValidEmailAddressRFC2822.java | 134 ++++++++ .../account/InvalidEmailAddressTest.java | 79 +++++ .../feature/account/RFC2822NegativeTest.java | 78 ----- .../feature/account/RFC2822PositiveTest.java | 56 ---- .../feature/account/RegisterDetailedTest.java | 4 +- .../feature/account/RegisterTestSuite.java | 4 +- .../account/ValidEmailAddressTest.java | 57 ++++ 9 files changed, 478 insertions(+), 425 deletions(-) delete mode 100644 functional-test/src/main/java/org/zanata/util/RFC2822.java create mode 100644 functional-test/src/main/java/org/zanata/util/rfc2822/InvalidEmailAddressRFC2822.java create mode 100644 functional-test/src/main/java/org/zanata/util/rfc2822/ValidEmailAddressRFC2822.java create mode 100644 functional-test/src/test/java/org/zanata/feature/account/InvalidEmailAddressTest.java delete mode 100644 functional-test/src/test/java/org/zanata/feature/account/RFC2822NegativeTest.java delete mode 100644 functional-test/src/test/java/org/zanata/feature/account/RFC2822PositiveTest.java create mode 100644 functional-test/src/test/java/org/zanata/feature/account/ValidEmailAddressTest.java diff --git a/functional-test/src/main/java/org/zanata/util/RFC2822.java b/functional-test/src/main/java/org/zanata/util/RFC2822.java deleted file mode 100644 index f30784ec5f..0000000000 --- a/functional-test/src/main/java/org/zanata/util/RFC2822.java +++ /dev/null @@ -1,287 +0,0 @@ -/* - * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the - * @author tags. See the copyright.txt file in the distribution for a full - * listing of individual contributors. - * - * This is free software; you can redistribute it and/or modify it under the - * terms of the GNU Lesser General Public License as published by the Free - * Software Foundation; either version 2.1 of the License, or (at your option) - * any later version. - * - * This software is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS - * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more - * details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this software; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF - * site: http://www.fsf.org. - */ -package org.zanata.util; - -import java.util.HashMap; -import java.util.Map; - -/** - * @author Damian Jansen djansen@redhat.com - */ -public class RFC2822 { - - /* - * Synopsis: - * The functions of this class contain valid and invalid email addresses, as stipulated in the - * RFC2822 Internet Message Format standard, or referred to standards. - * - * Definitions - * localpart: the section of an address preceding the @ symbol - * domain: the section of an address following the @ symbol - * label: section of localpart or domain between the start, @ symbol, period or end (also referred to as "atom") - * e.g. me, myself, example, com in me.myself@example.com - * quote / quoting: a section of the localpart contained within quotation marks - * - * Untested: - * RFC2822, section 3.4.1 - * The contents of a bracketed domain can have a \ precede a character to escape it, - * and the following character must not be 10 (LF) or 13 (CR). - * This allows spaces in the domain as long as they are escaped. - * - * RFC 2821, section 4.5.3.1 - * The maximum length of a "useful" email address is 255 characters. - * - * RFC 3696 - * The maximum allowable length of an email address is 320 characters. - */ - - /* - * VALID EMAIL ADDRESSES - */ - /* - * RFC 2822, section 3.4.1 - * Email addresses consist of a local part, the "@" symbol, and the domain. - */ - public static String BASIC_EMAIL = "email@example.com"; - - /* - * RFC 2822, sections 3.4.1 and 4.4 - * The local part can be unquoted, quoted in its entirety, or quoted on a per-label basis. - * The quoted local part starts with a quotation mark, ends with a quotation mark. - */ - // BUG982048 - public static String BASIC_QUOTED_EMAIL = "\"email\"@example.com"; - - /* - * RFC 2822, section 3.4.1 - * TEXT can contain alphabetic, numeric, and these symbols: !#$%'*+-/=?^_`{|}~ - */ - public static String SPECIAL_CHARACTERS_LOCALPART = "email.!#$%'*+-/=?^_`{|}~.dot@example.com"; - - /* - RFC 2822, section 4.4 - If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted - or quoted chunks separated by periods. - */ - public static String ENCLOSED_QUOTED_LABEL = "dot.\"email\".dot@example.com"; - public static String LOCALPART_WITH_EMPTY_QUOTE = "dot.\"\".dot@example.com"; - - /* - * RFC 2822, section 3.4.1 - * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). - * This supersedes the previous rule, allowing spaces and quotation marks in the email address as long as they - * are escaped. - */ - public static String QUOTED_ESCAPED_SPECIAL_CHARACTERS = "email.\"(),:;<>\\@\\[\\]\\\\\"@example.com"; - public static String QUOTED_ESCAPED_QUOTES = "email.\"\\\"\"@example.com"; - public static String QUOTED_WITH_SPACE = "\"special\\ email\"@example.com"; - - /* - * RFC 2822, section 3.4.1 - * The domain can be bracketed or plain. - */ - public static String BRACKETED_DOMAIN = "email@[example.com]"; - public static String BRACKETED_IPV4_DOMAIN = "email@[123.45.67.89]"; - public static String BRACKETED_IPV6_DOMAIN = "email@[IPv6:2001:2d12:c4fe:5afe::1]"; - - /* - * RFC 1035, section 2.3.4 - * A plain domain consists of labels separated with periods. No period can start or end a domain name. - */ - public static String LOCALPART_MULTIPLE_LABELS = "another.email@example.com"; - public static String DOMAIN_MULTIPLE_LABELS = "email@another.example.com"; - - /* - * RFC 1035, section 2.3.4 - * The maximum length of a label is 63 characters. - */ - public static String DOMAIN_LABEL_MAX_CHARACTERS = - "email@B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8.com"; - public static String LOCALPART_LABEL_MAX_CHARACTERS = - "B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8@example.com"; - - /* - * RFC 1035, section 2.3.4 - * A label may contain hyphens, but no two hyphens in a row. - */ - public static String HYPHENATED_DOMAIN_LABEL = "email@another-example.com"; - public static String HYPHENATED_LOCALPART_LABEL = "my-email@example.com"; - - /* - * RFC 2821, section 4.5.3.1 - * The maximum length of the local part is 64 characters. - */ - public static String LOCALPART_MAX_LENGTH = - "B3NQyUsDdzODMoymfDdifn6Wztx2wrivm.80LEngHGl182frm6ifCPyv5SntbDg8@example.com"; - - - /* - * INVALID EMAIL ADDRESSES - */ - - /* - * RFC 2822, section 3.4.1 - * Email addresses consist of a local part, the "@" symbol, and the domain. - */ - public static String PLAIN_ADDRESS = "plainaddress"; - public static String MISSING_AMPERSAT = "email.example.com"; - public static String MISSING_LOCALPART = "@example.com"; - public static String MISSING_DOMAIN = "email@"; - public static String MULTIPLE_APERSAT = "email@example@example.com"; - - /* - * RFC 2822, section 3.4.1 - * No periods can start or end the local part. - * Two periods together is invalid. - */ - public static String LEADING_DOT = ".email@example.com"; - public static String TRAILING_DOT = "email.@example.com"; - public static String MULTIPLE_DOTS = "email..email@example.com"; - - /* - * RFC 2822, section 2.2 - * All email addresses are in 7-bit US ASCII. - */ - public static String NON_UNICODE_CHARACTERS = "あいうえお@example.com"; - - /* - * RFC 2822, section 3.4.1 - * Unquoted local parts can consist of TEXT - * TEXT can contain: - * alphabetic - * numeric - * and symbols !#$%'*+-/=?^_`{|}~ - */ - public static String INVALID_UNQUOTED_COMMA = "test,user@example.com"; - public static String INVALID_UNQUOTED_LEFT_PARENTHESES = "test(user@example.com"; - public static String INVALID_UNQUOTED_RIGHT_PARENTHESES = "test)user@example.com"; - - /* - * RFC 2822, section 3.4.1 - * The quoted local part starts with a quotation mark, ends with a quotation mark. - */ - public static String INVALID_SINGLE_QUOTING = "test\"user@example.com"; - - /* - * RFC 2822, section 4.4 - * If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted - * or quoted chunks separated by periods - */ - public static String INVALID_QUOTING_SEPARATION = "\"test\"user@example.com"; - - /* - * RFC 2822, section 3.4.1 - * The contents of a quoted local part can not contain characters: - * 9 (TAB) - * 10 (LF) - * 13 (CR) - * 32 (space) - * 34 (") - * 91-94 ([, \, ], ^) - */ - public static String INVALID_QUOTED_COMMA = "\"test,user\"@example.com"; - public static String INVALID_QUOTED_BACKSLASH = "\"test\\user\"@example.com"; - public static String INVALID_QUOTED_LEFT_BRACKET = "\"test[user\"@example.com"; - public static String INVALID_QUOTED_RIGHT_BRACKET = "\"test]user\"@example.com"; - public static String INVALID_QUOTED_CARAT = "\"test^user\"@example.com"; - public static String INVALID_QUOTED_SPACE = "\"test user\"@example.com"; - public static String INVALID_QUOTED_QUOTE = "\"test\"user\"@example.com"; - // TODO: public static String Invalid_quoted_tab = "test.\"".concat("\t").concat("\".user@example.com"); - - /* - * RFC 2822, section 3.4.1 - * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). - */ - public static String INVALID_QUOTED_RETURN = "test.\"\\".concat("\r").concat("\".user@example.com"); - public static String INVALID_QUOTED_LINEFEED = "test.\"\\".concat("\n").concat("\".user@example.com"); - - /* - * RFC 1035, section 2.3.4 - * A plain domain consists of labels separated with periods. No period can start or end a domain name. - * No two periods in succession can be in a domain name. - */ - public static String TRAILING_DOMAIN_DOT = "email@example.com."; - public static String LEADING_DOMAIN_DOT = "email@.example.com"; - public static String SUCCESSIVE_DOMAIN_DOTS = "email@example..com"; - - /* - * RFC 2822, section 3.4.1 - * Bracketed domains must: - * start with [, end with ] - * not contain characters 9 (TAB), 10 (LF), 13 (CR), 32 (space), 91-94 ([, \, ], ^) - */ - public static String INCORRECTLY_BRACKETED_DOMAIN = "email@[example].com"; - public static String INVALID_DOMAIN_CHARACTER = "email@[ex^ample.com]"; - public static String INCORRECTLY_ESCAPED_DOMAIN = "email@[exa\\mple.com]"; - - /* - * RFC 1035, section 2.3.4 - * The maximum length of a label is 63 characters. - */ - public static String DOMAIN_LABEL_LENGTH_EXCEEDED = - "email@IJUr9P6Y7Fx7rFy4sziQDT0qvSC7XKK6jrD0CNC41jorAKgFYIXLTN5ITJLohy58.com"; - - /* - * RFC 1035, section 2.3.4 - * A label may contain hyphens, but no two hyphens in a row. - * A label must not start nor end with a hyphen. - */ - public static String LEADING_DASH_DOMAIN = "email@-example.com"; - public static String TRAILING_DASH_DOMAIN = "email@example-.com"; - public static String MULTIPLE_DASHES_DOMAIN = "email@exa--mple.com"; - public static String LEADING_DASH_BRACKETED_DOMAIN = "email@[-example.com]"; - public static String TRAILING_DASH_BRACKETED_DOMAIN = "email@[example.com-]"; - public static String MULTIPLE_DASHES_BRACKETED_DOMAIN = "email@[exa--mple.com]"; - - /* - * The contents of a bracketed domain can have a \ precede a character to escape it, and the following character - * must not be 10 (LF) or 13 (CR). - */ - public static String INVALID_BRACKETED_DOMAIN_RETURN = "test@[\\".concat("\r").concat("example.com]"); - public static String INVALID_BRACKETED_DOMAIN_LINEFEED = "test@[\\".concat("\n").concat("example.com]"); - - /* - * RFC 2821, section 4.5.3.1 - * The maximum length of the local part is 64 characters. - */ - public static String LOCALPART_LENGTH_EXCEEDED = - "emailuhpealgyxntsh5upl5gqn5a4ruqs7mw6wz21j6dn72amzwozqlyua4jx16rd@example.com"; - - /* - * RFC 3696, section 2 - * The top level domain must be all alphabetic. - */ - public static String INVALID_ENCODED_HTML = "Joe Smith "; - public static String INVALID_FOLLOWING_TEXT = "email@example.com (Joe Smith)"; - public static String INVALID_IP_FORMAT = "email@111.222.333.44444"; - - /* - * RFC 2821, section 4.5.3.1 - * The maximum length of a "useful" email address is 255 characters. - */ - public static String MAX_EMAIL_LENGTH_EXCEEDED = - "email@"+ - "Hk3yhCtbBRw3wCT76tL1ryAdfrIaaDszHqvZqnNrZPlNn3Wd7u."+ - "RfpxrueSghp9dkGTGwT9s0fyJL850Sned72RD3Mm5PpEh6QJwQ."+ - "3CeXyEHQEhXNOQdWhYVjGBLzlHz1sJfi4lfn7ighLXcxa5cMAK."+ - "jFXsG8BVsvkODKktTXJ70bQmDWtWQzuh3oz4twumVArDGEbzS1."+ - "slyaBcQqVgUdqXTBdbMY7YJxZwrzZQBBGjCl4e.com"; -} \ No newline at end of file diff --git a/functional-test/src/main/java/org/zanata/util/rfc2822/InvalidEmailAddressRFC2822.java b/functional-test/src/main/java/org/zanata/util/rfc2822/InvalidEmailAddressRFC2822.java new file mode 100644 index 0000000000..ded69fe894 --- /dev/null +++ b/functional-test/src/main/java/org/zanata/util/rfc2822/InvalidEmailAddressRFC2822.java @@ -0,0 +1,204 @@ +/* + * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the + * @author tags. See the copyright.txt file in the distribution for a full + * listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This software is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this software; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF + * site: http://www.fsf.org. + */ +package org.zanata.util.rfc2822; + +/** + * @author Damian Jansen djansen@redhat.com + * @see RFC2822 Internet Message Format Standard + * Synopsis: + * The functions of this class contain invalid email addresses, as stipulated in the + * RFC2822 Internet Message Format standard, or referred to standards. + * + * Definitions + * localpart: the section of an address preceding the @ symbol + * domain: the section of an address following the @ symbol + * label: section of localpart or domain between the start, @ symbol, period or end (also referred to as "atom") + * e.g. me, myself, example, com in me.myself@example.com + * quote / quoting: a section of the localpart contained within quotation marks + * + * Untested: + * + * RFC 2821, section 4.5.3.1 + * The maximum length of a "useful" email address is 255 characters. + * + * RFC 3696 + * The maximum allowable length of an email address is 320 characters. + */ +public enum InvalidEmailAddressRFC2822 { + + /** + * Email addresses consist of a local part, the "@" symbol, and the domain. + * @see "RFC 2822, section 3.4.1" + */ + PLAIN_ADDRESS("plainaddress"), + MISSING_AMPERSAT("email.example.com"), + MISSING_LOCALPART("@example.com"), + MISSING_DOMAIN("email@"), + MULTIPLE_APERSAT("email@example@example.com"), + + /** + * No periods can start or end the local part. + * Two periods together is invalid. + * @see "RFC 2822, section 3.4.1" + */ + LEADING_DOT(".email@example.com"), + TRAILING_DOT("email.@example.com"), + MULTIPLE_DOTS("email..email@example.com"), + + /** + * All email addresses are in 7-bit US ASCII. + * @see "RFC 2822, section 2.2" + */ + NON_UNICODE_CHARACTERS("あいうえお@example.com"), + + /** + * Unquoted local parts can consist of TEXT + * TEXT can contain: + * alphabetic + * numeric + * and symbols !#$%'*+-/=?^_`{|}~ + * @see "RFC 2822, section 3.4.1" + */ + INVALID_UNQUOTED_COMMA("test,user@example.com"), + INVALID_UNQUOTED_LEFT_PARENTHESES("test(user@example.com"), + INVALID_UNQUOTED_RIGHT_PARENTHESES("test)user@example.com"), + + /** + * The quoted local part starts with a quotation mark, ends with a quotation mark. + * @see "RFC 2822, section 3.4.1" + */ + INVALID_SINGLE_QUOTING("test\"user@example.com"), + + /** + * If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted + * or quoted chunks separated by periods + * @see "RFC 2822, section 4.4" + */ + INVALID_QUOTING_SEPARATION("\"test\"user@example.com"), + + /** + * The contents of a quoted local part can not contain characters: + * 9 (TAB) + * 10 (LF) + * 13 (CR) + * 32 (space) + * 34 (") + * 91-94 ([, \, ], ^) + * @see "RFC 2822, section 3.4.1" + */ + INVALID_QUOTED_COMMA("\"test,user\"@example.com"), + INVALID_QUOTED_BACKSLASH("\"test\\user\"@example.com"), + INVALID_QUOTED_LEFT_BRACKET("\"test[user\"@example.com"), + INVALID_QUOTED_RIGHT_BRACKET("\"test]user\"@example.com"), + INVALID_QUOTED_CARAT("\"test^user\"@example.com"), + INVALID_QUOTED_SPACE("\"test user\"@example.com"), + INVALID_QUOTED_QUOTE("\"test\"user\"@example.com"), + INVALID_QUOTED_TAB("test.\"".concat("\t").concat("\".user@example.com")), + + /** + * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). + * @see "RFC 2822, section 3.4.1" + */ + INVALID_QUOTED_RETURN("test.\"\\".concat("\r").concat("\".user@example.com")), + INVALID_QUOTED_LINEFEED("test.\"\\".concat("\n").concat("\".user@example.com")), + + /** + * A plain domain consists of labels separated with periods. No period can start or end a domain name. + * No two periods in succession can be in a domain name. + * @see "RFC 1035, section 2.3.4" + */ + TRAILING_DOMAIN_DOT("email@example.com."), + LEADING_DOMAIN_DOT("email@.example.com"), + SUCCESSIVE_DOMAIN_DOTS("email@example..com"), + + /** + * Bracketed domains must: + * start with [, end with ] + * not contain characters 9 (TAB), 10 (LF), 13 (CR), 32 (space), 91-94 ([, \, ], ^) + * @see "RFC 2822, section 3.4.1" + */ + INCORRECTLY_BRACKETED_DOMAIN("email@[example].com"), + INVALID_DOMAIN_CHARACTER("email@[ex^ample.com]"), + INCORRECTLY_ESCAPED_DOMAIN("email@[exa\\mple.com]"), + + /** + * The maximum length of a label is 63 characters. + * @see "RFC 1035, section 2.3.4" + */ + DOMAIN_LABEL_LENGTH_EXCEEDED("email@IJUr9P6Y7Fx7rFy4sziQDT0qvSC7XKK6jrD0CNC41jorAKgFYIXLTN5ITJLohy58.com"), + + /** + * A label may contain hyphens, but no two hyphens in a row. + * A label must not start nor end with a hyphen. + * @see "RFC 1035, section 2.3.4" + */ + LEADING_DASH_DOMAIN("email@-example.com"), + TRAILING_DASH_DOMAIN("email@example-.com"), + MULTIPLE_DASHES_DOMAIN("email@exa--mple.com"), + LEADING_DASH_BRACKETED_DOMAIN("email@[-example.com]"), + TRAILING_DASH_BRACKETED_DOMAIN("email@[example.com-]"), + MULTIPLE_DASHES_BRACKETED_DOMAIN("email@[exa--mple.com]"), + + /** + * The contents of a bracketed domain can have a \ precede a character to escape it, and the following character + * must not be 10 (LF) or 13 (CR). + * @see "RFC2822, section 3.4.1" + */ + INVALID_BRACKETED_DOMAIN_RETURN("test@[\\".concat("\r").concat("example.com]")), + INVALID_BRACKETED_DOMAIN_LINEFEED("test@[\\".concat("\n").concat("example.com]")), + + /** + * The maximum length of the local part is 64 characters. + * @see "RFC 2821, section 4.5.3.1" + */ + LOCALPART_LENGTH_EXCEEDED("emailuhpealgyxntsh5upl5gqn5a4ruqs7mw6wz21j6dn72amzwozqlyua4jx16rd@example.com"), + + /** + * The top level domain must be all alphabetic. + * @see "RFC 3696, section 2" + */ + INVALID_ENCODED_HTML("Joe Smith "), + INVALID_FOLLOWING_TEXT("email@example.com (Joe Smith)"), + INVALID_IP_FORMAT("email@111.222.333.44444"), + + /** + * The maximum length of a "useful" email address is 255 characters. + * @see "RFC 2821, section 4.5.3.1" + */ + MAX_EMAIL_LENGTH_EXCEEDED("email@"+ + "Hk3yhCtbBRw3wCT76tL1ryAdfrIaaDszHqvZqnNrZPlNn3Wd7u."+ + "RfpxrueSghp9dkGTGwT9s0fyJL850Sned72RD3Mm5PpEh6QJwQ."+ + "3CeXyEHQEhXNOQdWhYVjGBLzlHz1sJfi4lfn7ighLXcxa5cMAK."+ + "jFXsG8BVsvkODKktTXJ70bQmDWtWQzuh3oz4twumVArDGEbzS1."+ + "slyaBcQqVgUdqXTBdbMY7YJxZwrzZQBBGjCl4e.com"); + + private final String address; + + private InvalidEmailAddressRFC2822(String address) + { + this.address = address; + } + + public String toString() + { + return address; + } +} \ No newline at end of file diff --git a/functional-test/src/main/java/org/zanata/util/rfc2822/ValidEmailAddressRFC2822.java b/functional-test/src/main/java/org/zanata/util/rfc2822/ValidEmailAddressRFC2822.java new file mode 100644 index 0000000000..e153c9049c --- /dev/null +++ b/functional-test/src/main/java/org/zanata/util/rfc2822/ValidEmailAddressRFC2822.java @@ -0,0 +1,134 @@ +/* + * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the + * @author tags. See the copyright.txt file in the distribution for a full + * listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This software is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this software; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF + * site: http://www.fsf.org. + */ +package org.zanata.util.rfc2822; + +/** + * @author Damian Jansen djansen@redhat.com + * @see RFC2822 Internet Message Format Standard + * Synopsis: + * The functions of this class contain valid email addresses, as stipulated in the + * RFC2822 Internet Message Format standard, or referred to standards. + * + * Definitions + * localpart: the section of an address preceding the @ symbol + * domain: the section of an address following the @ symbol + * label: section of localpart or domain between the start, @ symbol, period or end (also referred to as "atom") + * e.g. me, myself, example, com in me.myself@example.com + * quote / quoting: a section of the localpart contained within quotation marks + * + * Untested: + * RFC2822, section 3.4.1 + * The contents of a bracketed domain can have a \ precede a character to escape it, + * and the following character must not be 10 (LF) or 13 (CR). + * This allows spaces in the domain as long as they are escaped. + * + * RFC 2821, section 4.5.3.1 + * The maximum length of a "useful" email address is 255 characters. + * + * RFC 3696 + * The maximum allowable length of an email address is 320 characters. + */ +public enum ValidEmailAddressRFC2822 { + + /** + * Email addresses consist of a local part, the "@" symbol, and the domain. + * @see "RFC 2822, section 3.4.1" + */ + BASIC_EMAIL("email@example.com"), + + /** + * The local part can be unquoted, quoted in its entirety, or quoted on a per-label basis. + * The quoted local part starts with a quotation mark, ends with a quotation mark. + * @see "RFC 2822, sections 3.4.1 and 4.4" + */ + BASIC_QUOTED_EMAIL("\"email\"@example.com"), + + /** + * TEXT can contain alphabetic, numeric, and these symbols: !#$%'*+-/=?^_`{|}~ + * @see "RFC 2822, section 3.4.1" + */ + SPECIAL_CHARACTERS_LOCALPART("email.!#$%'*+-/=?^_`{|}~.dot@example.com"), + + /** + * If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted + * or quoted chunks separated by periods. + * @see "RFC 2822, section 4.4" + */ + ENCLOSED_QUOTED_LABEL("dot.\"email\".dot@example.com"), + LOCALPART_WITH_EMPTY_QUOTE("dot.\"\".dot@example.com"), + + /** + * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). + * This supersedes the previous rule, allowing spaces and quotation marks in the email address as long as they + * are escaped. + * @see "RFC 2822, section 3.4.1" + */ + QUOTED_ESCAPED_SPECIAL_CHARACTERS("email.\"(),:;<>\\@\\[\\]\\\\\"@example.com"), + QUOTED_ESCAPED_QUOTES("email.\"\\\"\"@example.com"), + QUOTED_WITH_SPACE("\"special\\ email\"@example.com"), + + /** + * The domain can be bracketed or plain. + * @see "RFC 2822, section 3.4.1" + */ + BRACKETED_DOMAIN("email@[example.com]"), + BRACKETED_IPV4_DOMAIN("email@[123.45.67.89]"), + BRACKETED_IPV6_DOMAIN("email@[IPv6:2001:2d12:c4fe:5afe::1]"), + + /** + * A plain domain consists of labels separated with periods. No period can start or end a domain name. + * @see "RFC 1035, section 2.3.4" + */ + LOCALPART_MULTIPLE_LABELS("another.email@example.com"), + DOMAIN_MULTIPLE_LABELS("email@another.example.com"), + + /** + * The maximum length of a label is 63 characters. + * @see "RFC 1035, section 2.3.4" + */ + DOMAIN_LABEL_MAX_CHARACTERS("email@B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8.com"), + LOCALPART_LABEL_MAX_CHARACTERS("B3NQyUsDdzODMoymfDdifn6Wztx2wrivm80LEngHGl182frm6ifCPyv5SntbDg8@example.com"), + + /** + * A label may contain hyphens, but no two hyphens in a row. + * @see "RFC 1035, section 2.3.4" + */ + HYPHENATED_DOMAIN_LABEL("email@another-example.com"), + HYPHENATED_LOCALPART_LABEL("my-email@example.com"), + + /** + * The maximum length of the local part is 64 characters. + * @see "RFC 2821, section 4.5.3.1" + */ + LOCALPART_MAX_LENGTH("B3NQyUsDdzODMoymfDdifn6Wztx2wrivm.80LEngHGl182frm6ifCPyv5SntbDg8@example.com"); + + private final String address; + + private ValidEmailAddressRFC2822(String address) + { + this.address = address; + } + + public String toString() + { + return address; + } +} \ No newline at end of file diff --git a/functional-test/src/test/java/org/zanata/feature/account/InvalidEmailAddressTest.java b/functional-test/src/test/java/org/zanata/feature/account/InvalidEmailAddressTest.java new file mode 100644 index 0000000000..d307916cee --- /dev/null +++ b/functional-test/src/test/java/org/zanata/feature/account/InvalidEmailAddressTest.java @@ -0,0 +1,79 @@ +package org.zanata.feature.account; + +import org.hamcrest.Matchers; +import org.junit.ClassRule; +import org.junit.experimental.theories.DataPoint; +import org.junit.experimental.theories.Theories; +import org.junit.experimental.theories.Theory; +import org.junit.runner.RunWith; +import org.zanata.page.account.RegisterPage; +import org.zanata.util.ResetDatabaseRule; +import org.zanata.util.rfc2822.InvalidEmailAddressRFC2822; +import org.zanata.workflow.BasicWorkFlow; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.zanata.util.rfc2822.InvalidEmailAddressRFC2822.*; + +/** + * @author Damian Jansen djansen@redhat.com + */ +@RunWith(Theories.class) +public class InvalidEmailAddressTest { + @ClassRule + public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(); + + @DataPoint public static InvalidEmailAddressRFC2822 TEST_PLAIN_ADDRESS = PLAIN_ADDRESS; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_MISSING_AMPERSAT = MISSING_AMPERSAT; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_MISSING_LOCALPART = MISSING_LOCALPART; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_MISSING_DOMAIN = MISSING_DOMAIN; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_MULTIPLE_APERSAT = MULTIPLE_APERSAT; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_LEADING_DOT = LEADING_DOT; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_TRAILING_DOT = TRAILING_DOT; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_MULTIPLE_DOTS = MULTIPLE_DOTS; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_UNQUOTED_COMMA = INVALID_UNQUOTED_COMMA; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_UNQUOTED_LEFT_PARENTHESES = INVALID_UNQUOTED_LEFT_PARENTHESES; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_UNQUOTED_RIGHT_PARENTHESES = INVALID_UNQUOTED_RIGHT_PARENTHESES; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_SINGLE_QUOTING = INVALID_SINGLE_QUOTING; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTING_SEPARATION = INVALID_QUOTING_SEPARATION; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTED_COMMA = INVALID_QUOTED_COMMA; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTED_BACKSLASH = INVALID_QUOTED_BACKSLASH; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTED_LEFT_BRACKET = INVALID_QUOTED_LEFT_BRACKET; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTED_RIGHT_BRACKET = INVALID_QUOTED_RIGHT_BRACKET; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTED_CARAT = INVALID_QUOTED_CARAT; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTED_SPACE = INVALID_QUOTED_SPACE; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTED_QUOTE = INVALID_QUOTED_QUOTE; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTED_RETURN = INVALID_QUOTED_RETURN; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_QUOTED_LINEFEED = INVALID_QUOTED_LINEFEED; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_TRAILING_DOMAIN_DOT = TRAILING_DOMAIN_DOT; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_LEADING_DOMAIN_DOT = LEADING_DOMAIN_DOT; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_SUCCESSIVE_DOMAIN_DOTS = SUCCESSIVE_DOMAIN_DOTS; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INCORRECTLY_BRACKETED_DOMAIN = INCORRECTLY_BRACKETED_DOMAIN; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_DOMAIN_CHARACTER = INVALID_DOMAIN_CHARACTER; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INCORRECTLY_ESCAPED_DOMAIN = INCORRECTLY_ESCAPED_DOMAIN; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_DOMAIN_LABEL_LENGTH_EXCEEDED = DOMAIN_LABEL_LENGTH_EXCEEDED; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_LEADING_DASH_BRACKETED_DOMAIN = LEADING_DASH_BRACKETED_DOMAIN; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_TRAILING_DASH_BRACKETED_DOMAIN = TRAILING_DASH_BRACKETED_DOMAIN; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_MULTIPLE_DASHES_BRACKETED_DOMAIN = MULTIPLE_DASHES_BRACKETED_DOMAIN; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_BRACKETED_DOMAIN_RETURN = INVALID_BRACKETED_DOMAIN_RETURN; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_BRACKETED_DOMAIN_LINEFEED = INVALID_BRACKETED_DOMAIN_LINEFEED; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_LOCALPART_LENGTH_EXCEEDED = LOCALPART_LENGTH_EXCEEDED; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_ENCODED_HTML = INVALID_ENCODED_HTML; + @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_FOLLOWING_TEXT = INVALID_FOLLOWING_TEXT; + + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_IP_FORMAT = EmailAddressRFC2822.INVALID_IP_FORMAT; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_MAX_EMAIL_LENGTH_EXCEEDED = EmailAddressRFC2822.MAX_EMAIL_LENGTH_EXCEEDED; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_NON_UNICODE_CHARACTERS = EmailAddressRFC2822.NON_UNICODE_CHARACTERS; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_LEADING_DASH_DOMAIN = EmailAddressRFC2822.LEADING_DASH_DOMAIN; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_TRAILING_DASH_DOMAIN = EmailAddressRFC2822.TRAILING_DASH_DOMAIN; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_MULTIPLE_DASHES_DOMAIN = EmailAddressRFC2822.MULTIPLE_DASHES_DOMAIN; + + @Theory + public void invalidEmailRejection(InvalidEmailAddressRFC2822 emailAddress) + { + String errorMsg = "not a well-formed email address"; + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration(); + registerPage = registerPage.enterEmail(emailAddress.toString()).clickTerms(); + assertThat("Email validation errors are not shown", registerPage.waitForErrors(), Matchers.hasItem(errorMsg)); + } + +} diff --git a/functional-test/src/test/java/org/zanata/feature/account/RFC2822NegativeTest.java b/functional-test/src/test/java/org/zanata/feature/account/RFC2822NegativeTest.java deleted file mode 100644 index ea434e633d..0000000000 --- a/functional-test/src/test/java/org/zanata/feature/account/RFC2822NegativeTest.java +++ /dev/null @@ -1,78 +0,0 @@ -package org.zanata.feature.account; - -import org.hamcrest.Matchers; -import org.junit.ClassRule; -import org.junit.experimental.theories.DataPoint; -import org.junit.experimental.theories.Theories; -import org.junit.experimental.theories.Theory; -import org.junit.runner.RunWith; -import org.zanata.page.account.RegisterPage; -import org.zanata.util.RFC2822; -import org.zanata.util.ResetDatabaseRule; -import org.zanata.workflow.BasicWorkFlow; - -import static org.hamcrest.MatcherAssert.assertThat; - -/** - * @author Damian Jansen djansen@redhat.com - */ -@RunWith(Theories.class) -public class RFC2822NegativeTest { - @DataPoint public static String PLAIN_ADDRESS = RFC2822.PLAIN_ADDRESS; - @DataPoint public static String MISSING_AMPERSAT = RFC2822.MISSING_AMPERSAT; - @DataPoint public static String MISSING_LOCALPART = RFC2822.MISSING_LOCALPART; - @DataPoint public static String MISSING_DOMAIN = RFC2822.MISSING_DOMAIN; - @DataPoint public static String MULTIPLE_APERSAT = RFC2822.MULTIPLE_APERSAT; - @DataPoint public static String LEADING_DOT = RFC2822.LEADING_DOT; - @DataPoint public static String TRAILING_DOT = RFC2822.TRAILING_DOT; - @DataPoint public static String MULTIPLE_DOTS = RFC2822.MULTIPLE_DOTS; - @DataPoint public static String INVALID_UNQUOTED_COMMA = RFC2822.INVALID_UNQUOTED_COMMA; - @DataPoint public static String INVALID_UNQUOTED_LEFT_PARENTHESES = RFC2822.INVALID_UNQUOTED_LEFT_PARENTHESES; - @DataPoint public static String INVALID_UNQUOTED_RIGHT_PARENTHESES = RFC2822.INVALID_UNQUOTED_RIGHT_PARENTHESES; - @DataPoint public static String INVALID_SINGLE_QUOTING = RFC2822.INVALID_SINGLE_QUOTING; - @DataPoint public static String INVALID_QUOTING_SEPARATION = RFC2822.INVALID_QUOTING_SEPARATION; - @DataPoint public static String INVALID_QUOTED_COMMA = RFC2822.INVALID_QUOTED_COMMA; - @DataPoint public static String INVALID_QUOTED_BACKSLASH = RFC2822.INVALID_QUOTED_BACKSLASH; - @DataPoint public static String INVALID_QUOTED_LEFT_BRACKET = RFC2822.INVALID_QUOTED_LEFT_BRACKET; - @DataPoint public static String INVALID_QUOTED_RIGHT_BRACKET = RFC2822.INVALID_QUOTED_RIGHT_BRACKET; - @DataPoint public static String INVALID_QUOTED_CARAT = RFC2822.INVALID_QUOTED_CARAT; - @DataPoint public static String INVALID_QUOTED_SPACE = RFC2822.INVALID_QUOTED_SPACE; - @DataPoint public static String INVALID_QUOTED_QUOTE = RFC2822.INVALID_QUOTED_QUOTE; - @DataPoint public static String INVALID_QUOTED_RETURN = RFC2822.INVALID_QUOTED_RETURN; - @DataPoint public static String INVALID_QUOTED_LINEFEED = RFC2822.INVALID_QUOTED_LINEFEED; - @DataPoint public static String TRAILING_DOMAIN_DOT = RFC2822.TRAILING_DOMAIN_DOT; - @DataPoint public static String LEADING_DOMAIN_DOT = RFC2822.LEADING_DOMAIN_DOT; - @DataPoint public static String SUCCESSIVE_DOMAIN_DOTS = RFC2822.SUCCESSIVE_DOMAIN_DOTS; - @DataPoint public static String INCORRECTLY_BRACKETED_DOMAIN = RFC2822.INCORRECTLY_BRACKETED_DOMAIN; - @DataPoint public static String INVALID_DOMAIN_CHARACTER = RFC2822.INVALID_DOMAIN_CHARACTER; - @DataPoint public static String INCORRECTLY_ESCAPED_DOMAIN = RFC2822.INCORRECTLY_ESCAPED_DOMAIN; - @DataPoint public static String DOMAIN_LABEL_LENGTH_EXCEEDED = RFC2822.DOMAIN_LABEL_LENGTH_EXCEEDED; - @DataPoint public static String LEADING_DASH_BRACKETED_DOMAIN = RFC2822.LEADING_DASH_BRACKETED_DOMAIN; - @DataPoint public static String TRAILING_DASH_BRACKETED_DOMAIN = RFC2822.TRAILING_DASH_BRACKETED_DOMAIN; - @DataPoint public static String MULTIPLE_DASHES_BRACKETED_DOMAIN = RFC2822.MULTIPLE_DASHES_BRACKETED_DOMAIN; - @DataPoint public static String INVALID_BRACKETED_DOMAIN_RETURN = RFC2822.INVALID_BRACKETED_DOMAIN_RETURN; - @DataPoint public static String INVALID_BRACKETED_DOMAIN_LINEFEED = RFC2822.INVALID_BRACKETED_DOMAIN_LINEFEED; - @DataPoint public static String LOCALPART_LENGTH_EXCEEDED = RFC2822.LOCALPART_LENGTH_EXCEEDED; - @DataPoint public static String INVALID_ENCODED_HTML = RFC2822.INVALID_ENCODED_HTML; - @DataPoint public static String INVALID_FOLLOWING_TEXT = RFC2822.INVALID_FOLLOWING_TEXT; - - // BUG982048 @DataPoint public static String INVALID_IP_FORMAT = RFC2822.INVALID_IP_FORMAT; - // BUG982048 @DataPoint public static String MAX_EMAIL_LENGTH_EXCEEDED = RFC2822.MAX_EMAIL_LENGTH_EXCEEDED; - // BUG982048 @DataPoint public static String NON_UNICODE_CHARACTERS = RFC2822.NON_UNICODE_CHARACTERS; - // BUG982048 @DataPoint public static String LEADING_DASH_DOMAIN = RFC2822.LEADING_DASH_DOMAIN; - // BUG982048 @DataPoint public static String TRAILING_DASH_DOMAIN = RFC2822.TRAILING_DASH_DOMAIN; - // BUG982048 @DataPoint public static String MULTIPLE_DASHES_DOMAIN = RFC2822.MULTIPLE_DASHES_DOMAIN; - - @ClassRule - public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(); - - @Theory - public void invalidEmailRejection(String emailAddress) - { - String errorMsg = "not a well-formed email address"; - RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration(); - registerPage = registerPage.enterEmail(emailAddress).clickTerms(); - assertThat("Email validation errors are not shown", registerPage.waitForErrors(), Matchers.hasItem(errorMsg)); - } - -} diff --git a/functional-test/src/test/java/org/zanata/feature/account/RFC2822PositiveTest.java b/functional-test/src/test/java/org/zanata/feature/account/RFC2822PositiveTest.java deleted file mode 100644 index c9a1932986..0000000000 --- a/functional-test/src/test/java/org/zanata/feature/account/RFC2822PositiveTest.java +++ /dev/null @@ -1,56 +0,0 @@ -package org.zanata.feature.account; - -import org.hamcrest.Matchers; -import org.junit.ClassRule; -import org.junit.experimental.theories.DataPoint; -import org.junit.experimental.theories.Theories; -import org.junit.experimental.theories.Theory; -import org.junit.runner.RunWith; -import org.zanata.page.account.RegisterPage; -import org.zanata.util.RFC2822; -import org.zanata.util.ResetDatabaseRule; -import org.zanata.workflow.BasicWorkFlow; - -import static org.hamcrest.MatcherAssert.assertThat; - -/** - * @author Damian Jansen djansen@redhat.com - */ -@RunWith(Theories.class) -public class RFC2822PositiveTest { - - @DataPoint public static String BASIC_EMAIL = RFC2822.BASIC_EMAIL; - @DataPoint public static String SPECIAL_LOCALPART_CHARACTERS = RFC2822.SPECIAL_CHARACTERS_LOCALPART; - @DataPoint public static String LOCALPART_MULTIPLE_LABELS = RFC2822.LOCALPART_MULTIPLE_LABELS; - @DataPoint public static String DOMAIN_MULTIPLE_LABELS = RFC2822.DOMAIN_MULTIPLE_LABELS; - @DataPoint public static String DOMAIN_LABEL_MAX_CHARACTERS = RFC2822.DOMAIN_LABEL_MAX_CHARACTERS; - @DataPoint public static String LOCALPART_LABEL_MAX_CHARACTERS = RFC2822.LOCALPART_LABEL_MAX_CHARACTERS; - @DataPoint public static String HYPHENATED_DOMAIN_LABEL = RFC2822.HYPHENATED_DOMAIN_LABEL; - @DataPoint public static String HYPHENATED_LOCALPART_LABEL = RFC2822.HYPHENATED_LOCALPART_LABEL; - @DataPoint public static String LOCALPART_MAX_LENGTH = RFC2822.LOCALPART_MAX_LENGTH; - - // BUG982048 @DataPoint public static String BASIC_QUOTED_EMAIL = RFC2822.BASIC_QUOTED_EMAIL; - // BUG982048 @DataPoint public static String ENCLOSED_QUOTED_LABEL = RFC2822.ENCLOSED_QUOTED_LABEL; - // BUG982048 @DataPoint public static String LOCALPART_EMPTY_QUOTE = RFC2822.LOCALPART_WITH_EMPTY_QUOTE; - // BUG982048 @DataPoint public static String QUOTED_ESCAPED_SPECIAL_CHARACTERS = RFC2822.QUOTED_ESCAPED_SPECIAL_CHARACTERS; - // BUG982048 @DataPoint public static String QUOTED_ESCAPED_QUOTES = RFC2822.QUOTED_ESCAPED_QUOTES; - // BUG982048 @DataPoint public static String QUOTED_WITH_SPACE = RFC2822.QUOTED_WITH_SPACE; - // BUG982048 @DataPoint public static String BRACKETED_DOMAIN = RFC2822.BRACKETED_DOMAIN; - // BUG982048 @DataPoint public static String BRACKETED_IPV4_DOMAIN = RFC2822.BRACKETED_IPV4_DOMAIN; - // BUG982048 @DataPoint public static String BRACKETED_IPV6_DOMAIN = RFC2822.BRACKETED_IPV6_DOMAIN; - - @ClassRule - public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(); - - @Theory - public void validEmailAcceptance(String emailAddress) - { - String errorMsg = "not a well-formed email address"; - RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration(); - registerPage = registerPage.enterEmail(emailAddress).clickTerms(); - assertThat("Email validation errors are not shown", registerPage.getErrors(), - Matchers.not(Matchers.hasItem(errorMsg))); - - } - -} diff --git a/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java b/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java index 4c90419e65..2a4b3f0aef 100644 --- a/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java +++ b/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java @@ -28,7 +28,7 @@ import org.junit.Test; import org.zanata.page.HomePage; import org.zanata.page.account.RegisterPage; -import org.zanata.util.RFC2822; +import org.zanata.util.rfc2822.InvalidEmailAddressRFC2822; import org.zanata.util.ResetDatabaseRule; import org.zanata.workflow.BasicWorkFlow; @@ -109,7 +109,7 @@ public void usernamePreExisting() public void emailValidation() { String errorMsg = "not a well-formed email address"; - fields.put("email", RFC2822.PLAIN_ADDRESS); + fields.put("email", InvalidEmailAddressRFC2822.PLAIN_ADDRESS.toString()); fields.put("username", "emailvalidation"); RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields); assertThat("Email validation errors are shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); diff --git a/functional-test/src/test/java/org/zanata/feature/account/RegisterTestSuite.java b/functional-test/src/test/java/org/zanata/feature/account/RegisterTestSuite.java index 5e1f36d8be..8201fa4cd6 100644 --- a/functional-test/src/test/java/org/zanata/feature/account/RegisterTestSuite.java +++ b/functional-test/src/test/java/org/zanata/feature/account/RegisterTestSuite.java @@ -32,8 +32,8 @@ @Suite.SuiteClasses({ RegisterDetailedTest.class, UsernameValidationTest.class, - RFC2822PositiveTest.class, - RFC2822NegativeTest.class + ValidEmailAddressTest.class, + InvalidEmailAddressTest.class }) public class RegisterTestSuite { diff --git a/functional-test/src/test/java/org/zanata/feature/account/ValidEmailAddressTest.java b/functional-test/src/test/java/org/zanata/feature/account/ValidEmailAddressTest.java new file mode 100644 index 0000000000..17f8a170d3 --- /dev/null +++ b/functional-test/src/test/java/org/zanata/feature/account/ValidEmailAddressTest.java @@ -0,0 +1,57 @@ +package org.zanata.feature.account; + +import org.hamcrest.Matchers; +import org.junit.ClassRule; +import org.junit.experimental.theories.DataPoint; +import org.junit.experimental.theories.Theories; +import org.junit.experimental.theories.Theory; +import org.junit.runner.RunWith; +import org.zanata.page.account.RegisterPage; +import org.zanata.util.ResetDatabaseRule; +import org.zanata.util.rfc2822.ValidEmailAddressRFC2822; +import org.zanata.workflow.BasicWorkFlow; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.zanata.util.rfc2822.ValidEmailAddressRFC2822.*; + +/** + * @author Damian Jansen djansen@redhat.com + */ +@RunWith(Theories.class) +public class ValidEmailAddressTest { + + @ClassRule + public static ResetDatabaseRule resetDatabaseRule = new ResetDatabaseRule(); + + @DataPoint public static ValidEmailAddressRFC2822 TEST_BASIC_EMAIL = BASIC_EMAIL; + @DataPoint public static ValidEmailAddressRFC2822 TEST_SPECIAL_LOCALPART_CHARACTERS = SPECIAL_CHARACTERS_LOCALPART; + @DataPoint public static ValidEmailAddressRFC2822 TEST_LOCALPART_MULTIPLE_LABELS = LOCALPART_MULTIPLE_LABELS; + @DataPoint public static ValidEmailAddressRFC2822 TEST_DOMAIN_MULTIPLE_LABELS = DOMAIN_MULTIPLE_LABELS; + @DataPoint public static ValidEmailAddressRFC2822 TEST_DOMAIN_LABEL_MAX_CHARACTERS = DOMAIN_LABEL_MAX_CHARACTERS; + @DataPoint public static ValidEmailAddressRFC2822 TEST_LOCALPART_LABEL_MAX_CHARACTERS = LOCALPART_LABEL_MAX_CHARACTERS; + @DataPoint public static ValidEmailAddressRFC2822 TEST_HYPHENATED_DOMAIN_LABEL = HYPHENATED_DOMAIN_LABEL; + @DataPoint public static ValidEmailAddressRFC2822 TEST_HYPHENATED_LOCALPART_LABEL = HYPHENATED_LOCALPART_LABEL; + @DataPoint public static ValidEmailAddressRFC2822 TEST_LOCALPART_MAX_LENGTH = LOCALPART_MAX_LENGTH; + + // BUG982048 @DataPoint public static ValidEmailAddressRFC2822 TEST_BASIC_QUOTED_EMAIL = BASIC_QUOTED_EMAIL; + // BUG982048 @DataPoint public static ValidEmailAddressRFC2822 TEST_ENCLOSED_QUOTED_LABEL = ENCLOSED_QUOTED_LABEL; + // BUG982048 @DataPoint public static ValidEmailAddressRFC2822 TEST_LOCALPART_EMPTY_QUOTE = LOCALPART_WITH_EMPTY_QUOTE; + // BUG982048 @DataPoint public static ValidEmailAddressRFC2822 TEST_QUOTED_ESCAPED_SPECIAL_CHARACTERS = QUOTED_ESCAPED_SPECIAL_CHARACTERS; + // BUG982048 @DataPoint public static ValidEmailAddressRFC2822 TEST_QUOTED_ESCAPED_QUOTES = QUOTED_ESCAPED_QUOTES; + // BUG982048 @DataPoint public static ValidEmailAddressRFC2822 TEST_QUOTED_WITH_SPACE = QUOTED_WITH_SPACE; + // BUG982048 @DataPoint public static ValidEmailAddressRFC2822 TEST_BRACKETED_DOMAIN = BRACKETED_DOMAIN; + // BUG982048 @DataPoint public static ValidEmailAddressRFC2822 TEST_BRACKETED_IPV4_DOMAIN = BRACKETED_IPV4_DOMAIN; + // BUG982048 @DataPoint public static ValidEmailAddressRFC2822 TEST_BRACKETED_IPV6_DOMAIN = BRACKETED_IPV6_DOMAIN; + + @Theory + public void validEmailAcceptance(ValidEmailAddressRFC2822 emailAddress) + { + String errorMsg = "not a well-formed email address"; + RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration(); + registerPage = registerPage.enterEmail(emailAddress.toString()).clickTerms(); + assertThat("Email validation errors are not shown", registerPage.getErrors(), + Matchers.not(Matchers.hasItem(errorMsg))); + + } + +} From 41783d7964577c0dd65728ff14553c7a378b1bc4 Mon Sep 17 00:00:00 2001 From: Damian Jansen Date: Mon, 15 Jul 2013 13:36:54 +1000 Subject: [PATCH 20/26] Fix some line length and function calls Function calls (specifically new BasicWorkFlow) replaced. Wrapped some long lines. --- .../main/java/org/zanata/page/HomePage.java | 2 +- .../rfc2822/InvalidEmailAddressRFC2822.java | 22 ++++++++++--------- .../rfc2822/ValidEmailAddressRFC2822.java | 21 ++++++++++-------- .../account/InvalidEmailAddressTest.java | 12 +++++----- .../feature/account/RegisterDetailedTest.java | 16 +++++++------- 5 files changed, 39 insertions(+), 34 deletions(-) diff --git a/functional-test/src/main/java/org/zanata/page/HomePage.java b/functional-test/src/main/java/org/zanata/page/HomePage.java index b58226ae46..2442ffe3c1 100644 --- a/functional-test/src/main/java/org/zanata/page/HomePage.java +++ b/functional-test/src/main/java/org/zanata/page/HomePage.java @@ -112,7 +112,7 @@ public AdministrationPage goToAdministration() public RegisterPage goToRegistration() { - getDriver().findElement(By.linkText("More")).click(); + getDriver().findElement(By.id("systemCol")).click(); WebElement registerLink = getDriver().findElement(By.id("Register")); registerLink.click(); return new RegisterPage(getDriver()); diff --git a/functional-test/src/main/java/org/zanata/util/rfc2822/InvalidEmailAddressRFC2822.java b/functional-test/src/main/java/org/zanata/util/rfc2822/InvalidEmailAddressRFC2822.java index ded69fe894..9aab26ff2a 100644 --- a/functional-test/src/main/java/org/zanata/util/rfc2822/InvalidEmailAddressRFC2822.java +++ b/functional-test/src/main/java/org/zanata/util/rfc2822/InvalidEmailAddressRFC2822.java @@ -24,14 +24,15 @@ * @author Damian Jansen djansen@redhat.com * @see RFC2822 Internet Message Format Standard * Synopsis: - * The functions of this class contain invalid email addresses, as stipulated in the + * This enumeration represents a collection of invalid email addresses, as stipulated in the * RFC2822 Internet Message Format standard, or referred to standards. * * Definitions * localpart: the section of an address preceding the @ symbol * domain: the section of an address following the @ symbol - * label: section of localpart or domain between the start, @ symbol, period or end (also referred to as "atom") - * e.g. me, myself, example, com in me.myself@example.com + * label: section of localpart or domain between the start, @ symbol, period or + * end (also referred to as "atom") + * e.g. me, myself, example, com in me.myself@example.com * quote / quoting: a section of the localpart contained within quotation marks * * Untested: @@ -88,8 +89,8 @@ public enum InvalidEmailAddressRFC2822 { INVALID_SINGLE_QUOTING("test\"user@example.com"), /** - * If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted - * or quoted chunks separated by periods + * If an email is using the obsolete quoting on a per-label basis, then the email + * address consists of unquoted or quoted chunks separated by periods * @see "RFC 2822, section 4.4" */ INVALID_QUOTING_SEPARATION("\"test\"user@example.com"), @@ -114,15 +115,16 @@ public enum InvalidEmailAddressRFC2822 { INVALID_QUOTED_TAB("test.\"".concat("\t").concat("\".user@example.com")), /** - * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). + * If the quoted local part has a backslash, the following character is escaped and must not be + * 10 (LF), 13 (CR). * @see "RFC 2822, section 3.4.1" */ INVALID_QUOTED_RETURN("test.\"\\".concat("\r").concat("\".user@example.com")), INVALID_QUOTED_LINEFEED("test.\"\\".concat("\n").concat("\".user@example.com")), /** - * A plain domain consists of labels separated with periods. No period can start or end a domain name. - * No two periods in succession can be in a domain name. + * A plain domain consists of labels separated with periods. No period can start or end a + * domain name. No two periods in succession can be in a domain name. * @see "RFC 1035, section 2.3.4" */ TRAILING_DOMAIN_DOT("email@example.com."), @@ -158,8 +160,8 @@ public enum InvalidEmailAddressRFC2822 { MULTIPLE_DASHES_BRACKETED_DOMAIN("email@[exa--mple.com]"), /** - * The contents of a bracketed domain can have a \ precede a character to escape it, and the following character - * must not be 10 (LF) or 13 (CR). + * The contents of a bracketed domain can have a \ precede a character to escape it, and the + * following character must not be 10 (LF) or 13 (CR). * @see "RFC2822, section 3.4.1" */ INVALID_BRACKETED_DOMAIN_RETURN("test@[\\".concat("\r").concat("example.com]")), diff --git a/functional-test/src/main/java/org/zanata/util/rfc2822/ValidEmailAddressRFC2822.java b/functional-test/src/main/java/org/zanata/util/rfc2822/ValidEmailAddressRFC2822.java index e153c9049c..8fd1b7d4b5 100644 --- a/functional-test/src/main/java/org/zanata/util/rfc2822/ValidEmailAddressRFC2822.java +++ b/functional-test/src/main/java/org/zanata/util/rfc2822/ValidEmailAddressRFC2822.java @@ -24,14 +24,15 @@ * @author Damian Jansen djansen@redhat.com * @see RFC2822 Internet Message Format Standard * Synopsis: - * The functions of this class contain valid email addresses, as stipulated in the + * This enumeration represents a collection of valid email addresses, as stipulated in the * RFC2822 Internet Message Format standard, or referred to standards. * * Definitions * localpart: the section of an address preceding the @ symbol * domain: the section of an address following the @ symbol - * label: section of localpart or domain between the start, @ symbol, period or end (also referred to as "atom") - * e.g. me, myself, example, com in me.myself@example.com + * label: section of localpart or domain between the start, @ symbol, period or + * end (also referred to as "atom") + * e.g. me, myself, example, com in me.myself@example.com * quote / quoting: a section of the localpart contained within quotation marks * * Untested: @@ -68,17 +69,18 @@ public enum ValidEmailAddressRFC2822 { SPECIAL_CHARACTERS_LOCALPART("email.!#$%'*+-/=?^_`{|}~.dot@example.com"), /** - * If an email is using the obsolete quoting on a per-label basis, then the email address consists of unquoted - * or quoted chunks separated by periods. + * If an email is using the obsolete quoting on a per-label basis, then the email address + * consists of unquoted or quoted chunks separated by periods. * @see "RFC 2822, section 4.4" */ ENCLOSED_QUOTED_LABEL("dot.\"email\".dot@example.com"), LOCALPART_WITH_EMPTY_QUOTE("dot.\"\".dot@example.com"), /** - * If the quoted local part has a backslash, the following character is escaped and must not be 10 (LF), 13 (CR). - * This supersedes the previous rule, allowing spaces and quotation marks in the email address as long as they - * are escaped. + * If the quoted local part has a backslash, the following character is escaped and must not + * be 10 (LF), 13 (CR). + * This supersedes the previous rule, allowing spaces and quotation marks in the email address + * as long as they are escaped. * @see "RFC 2822, section 3.4.1" */ QUOTED_ESCAPED_SPECIAL_CHARACTERS("email.\"(),:;<>\\@\\[\\]\\\\\"@example.com"), @@ -94,7 +96,8 @@ public enum ValidEmailAddressRFC2822 { BRACKETED_IPV6_DOMAIN("email@[IPv6:2001:2d12:c4fe:5afe::1]"), /** - * A plain domain consists of labels separated with periods. No period can start or end a domain name. + * A plain domain consists of labels separated with periods. No period can start or end + * a domain name. * @see "RFC 1035, section 2.3.4" */ LOCALPART_MULTIPLE_LABELS("another.email@example.com"), diff --git a/functional-test/src/test/java/org/zanata/feature/account/InvalidEmailAddressTest.java b/functional-test/src/test/java/org/zanata/feature/account/InvalidEmailAddressTest.java index d307916cee..e79ffe043e 100644 --- a/functional-test/src/test/java/org/zanata/feature/account/InvalidEmailAddressTest.java +++ b/functional-test/src/test/java/org/zanata/feature/account/InvalidEmailAddressTest.java @@ -60,12 +60,12 @@ public class InvalidEmailAddressTest { @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_ENCODED_HTML = INVALID_ENCODED_HTML; @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_FOLLOWING_TEXT = INVALID_FOLLOWING_TEXT; - // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_IP_FORMAT = EmailAddressRFC2822.INVALID_IP_FORMAT; - // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_MAX_EMAIL_LENGTH_EXCEEDED = EmailAddressRFC2822.MAX_EMAIL_LENGTH_EXCEEDED; - // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_NON_UNICODE_CHARACTERS = EmailAddressRFC2822.NON_UNICODE_CHARACTERS; - // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_LEADING_DASH_DOMAIN = EmailAddressRFC2822.LEADING_DASH_DOMAIN; - // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_TRAILING_DASH_DOMAIN = EmailAddressRFC2822.TRAILING_DASH_DOMAIN; - // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_MULTIPLE_DASHES_DOMAIN = EmailAddressRFC2822.MULTIPLE_DASHES_DOMAIN; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_INVALID_IP_FORMAT = INVALID_IP_FORMAT; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_MAX_EMAIL_LENGTH_EXCEEDED = MAX_EMAIL_LENGTH_EXCEEDED; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_NON_UNICODE_CHARACTERS = NON_UNICODE_CHARACTERS; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_LEADING_DASH_DOMAIN = LEADING_DASH_DOMAIN; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_TRAILING_DASH_DOMAIN = TRAILING_DASH_DOMAIN; + // BUG982048 @DataPoint public static InvalidEmailAddressRFC2822 TEST_MULTIPLE_DASHES_DOMAIN = MULTIPLE_DASHES_DOMAIN; @Theory public void invalidEmailRejection(InvalidEmailAddressRFC2822 emailAddress) diff --git a/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java b/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java index 2a4b3f0aef..b59dcc24e3 100644 --- a/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java +++ b/functional-test/src/test/java/org/zanata/feature/account/RegisterDetailedTest.java @@ -70,13 +70,13 @@ public void before() @Ignore("Captcha prevents test completion") public void registerSuccessful() { - RegisterPage registerPage = homePage.goToRegistration(); - registerPage = registerPage.setFields(fields); + RegisterPage registerPage = homePage.goToRegistration().setFields(fields); assertThat("No errors are shown", registerPage.getErrors().size(), Matchers.equalTo(0)); registerPage.register(); } @Test + @Ignore("Unstable length matching ") public void usernameLengthValidation() { String errorMsg = "size must be between 3 and 20"; @@ -101,7 +101,7 @@ public void usernameLengthValidation() public void usernamePreExisting() { String errorMsg = "This username is not available"; - RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().enterUserName("admin"); + RegisterPage registerPage = homePage.goToRegistration().enterUserName("admin"); assertThat("Username not available message is shown", registerPage.waitForErrors(), Matchers.hasItem(errorMsg)); } @@ -111,7 +111,7 @@ public void emailValidation() String errorMsg = "not a well-formed email address"; fields.put("email", InvalidEmailAddressRFC2822.PLAIN_ADDRESS.toString()); fields.put("username", "emailvalidation"); - RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields); + RegisterPage registerPage = homePage.goToRegistration().setFields(fields); assertThat("Email validation errors are shown", registerPage.getErrors(), Matchers.hasItem(errorMsg)); } @@ -123,7 +123,7 @@ public void rejectIncorrectCaptcha() fields.put("email", "rejectbadcaptcha@example.com"); fields.put("captcha", "9000"); - RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields).registerFailure(); + RegisterPage registerPage = homePage.goToRegistration().setFields(fields).registerFailure(); assertThat("The Captcha entry is rejected", registerPage.getErrors(), Matchers.contains(errorMsg)); } @@ -136,7 +136,7 @@ public void passwordsMatch() fields.put("password", "passwordsmatch"); fields.put("confirmpassword", "passwordsdonotmatch"); - RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields); + RegisterPage registerPage = homePage.goToRegistration().setFields(fields); assertThat("Passwords fail to match error is shown", registerPage.getErrors(), Matchers.contains(errorMsg)); } @@ -150,7 +150,7 @@ public void requiredFields() fields.put("password", ""); fields.put("confirmpassword", ""); - RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields); + RegisterPage registerPage = homePage.goToRegistration().setFields(fields); assertThat("Value is required shows for all fields", registerPage.getErrors(), Matchers.contains(errorMsg, errorMsg, errorMsg, errorMsg, errorMsg)); } @@ -164,7 +164,7 @@ public void bug981498_underscoreRules() String errorMsg = "lowercase letters and digits (regex \"^[a-z\\d_]{3,20}$\")"; fields.put("email", "bug981498test@example.com"); fields.put("username", "______"); - RegisterPage registerPage = new BasicWorkFlow().goToHome().goToRegistration().setFields(fields); + RegisterPage registerPage = homePage.goToRegistration().setFields(fields); assertThat("A username of all underscores is not valid", registerPage.getErrors(), Matchers.hasItem(errorMsg)); } } From ce224e3f9345c1812f6a239499d274971bd1eea1 Mon Sep 17 00:00:00 2001 From: Alex Eng Date: Mon, 15 Jul 2013 14:43:33 +1000 Subject: [PATCH 21/26] fix paging bug in editor: https://bugzilla.redhat.com/show_bug.cgi?id=983786 --- .../org/zanata/webtrans/client/ui/Pager.java | 31 ++++++++++++++----- .../zanata/webtrans/client/ui/Pager.ui.xml | 7 ++++- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/Pager.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/Pager.java index d792e745a5..cd3a35edd1 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/Pager.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/Pager.java @@ -3,6 +3,7 @@ import org.zanata.webtrans.client.resources.Resources; import org.zanata.webtrans.client.resources.WebTransMessages; +import com.allen_sauer.gwt.log.client.Log; import com.google.gwt.core.client.GWT; import com.google.gwt.event.dom.client.BlurEvent; import com.google.gwt.event.dom.client.BlurHandler; @@ -42,7 +43,6 @@ interface Styles extends CssResource String disabled(); } - @UiField InlineLabel firstPage, lastPage, nextPage, prevPage; @@ -158,7 +158,7 @@ public void setValue(Integer value) @Override public void setValue(Integer value, boolean fireEvents) { - if (value != this.currentPage) + if (value != this.currentPage && (value > 0 && value <= pageCount)) { this.currentPage = value; if (fireEvents) @@ -183,23 +183,40 @@ public void onClick(ClickEvent event) { if (event.getSource() == firstPage) { - setValue(1); + if (isButtonEnabled(firstPage)) + { + setValue(1); + } } else if (event.getSource() == lastPage) { - setValue(pageCount); + if (isButtonEnabled(lastPage)) + { + setValue(pageCount); + } } else if (event.getSource() == nextPage) { - setValue(currentPage + 1); + if (isButtonEnabled(nextPage)) + { + setValue(currentPage + 1); + } } else if (event.getSource() == prevPage) { - setValue(currentPage - 1); + if (isButtonEnabled(prevPage)) + { + setValue(currentPage - 1); + } } } }; + private boolean isButtonEnabled(InlineLabel button) + { + return button.getStyleName().contains(style.enabled()); + } + @Override public HandlerRegistration addFocusHandler(FocusHandler handler) { @@ -214,7 +231,7 @@ public HandlerRegistration addBlurHandler(BlurHandler handler) private void setEnabled(InlineLabel button, boolean enabled) { - if(enabled) + if (enabled) { button.removeStyleName(style.disabled()); button.addStyleName(style.enabled()); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/Pager.ui.xml b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/Pager.ui.xml index 96c0ab97e9..7c7fd3be74 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/Pager.ui.xml +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/Pager.ui.xml @@ -6,7 +6,7 @@ .nav-button { - font-size:16px; + font-size:1.1em; } .rootContainer { @@ -23,6 +23,11 @@ { opacity:1; cursor:pointer; + transition:0.2s; + } + + .enabled:hover { + color:#0085cc; } .disabled From b8841539c28ba260c907f0d09e5c8dec0c1ada32 Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Mon, 15 Jul 2013 17:46:40 +1000 Subject: [PATCH 22/26] Update xom version for server --- pom.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pom.xml b/pom.xml index 72d7079998..6a56d6f8e3 100644 --- a/pom.xml +++ b/pom.xml @@ -505,6 +505,16 @@ test-jar test + + + xom + xom + 1.2.5 + chrome + :0 ${project.build.directory}/webdriver.log @@ -402,7 +403,11 @@ -Dzanata.sample.projects.basedir=${project.build.testOutputDirectory}/sample-projects -Dcargo.debug.jvm.args : If not set by default will listen to port 8787. Need to set to empty on jenkins - -Dinclude.test.patterns : by default is **/*TestSuite.java. Can be used to control what test to run. + -Dinclude.test.patterns=test filter pattern. Can be used to control what test to run. Default is **/*TestSuite.java. + -Dwebdriver.type=run tests in htmlUnit, chrome or firefox. For chrome, see also webdriver.chrome.* Default is htmlUnit. + -Dwebdriver.display=display to run test browser in, for Xnest or otherwise. Default is :0. + -Dwebdriver.chrome.bin=full path to chrome binary. + -Dwebdriver.chrome.driver=full path to chromedriver binary. ========================================================== diff --git a/functional-test/src/main/java/org/zanata/page/WebDriverFactory.java b/functional-test/src/main/java/org/zanata/page/WebDriverFactory.java index b08a1ef61f..2d120d3e59 100644 --- a/functional-test/src/main/java/org/zanata/page/WebDriverFactory.java +++ b/functional-test/src/main/java/org/zanata/page/WebDriverFactory.java @@ -29,6 +29,7 @@ import java.io.IOException; import java.util.concurrent.TimeUnit; +import com.google.common.collect.ImmutableMap; import org.openqa.selenium.WebDriver; import org.openqa.selenium.chrome.ChromeDriverService; import org.openqa.selenium.firefox.FirefoxBinary; @@ -105,11 +106,11 @@ private WebDriver configureChromeDriver() driverService = new ChromeDriverService.Builder() .usingDriverExecutable(new File(PropertiesHolder.properties.getProperty("webdriver.chrome.driver"))) .usingAnyFreePort() + .withEnvironment(ImmutableMap.of("DISPLAY", PropertiesHolder.properties.getProperty("webdriver.display"))) .withLogFile(new File(PropertiesHolder.properties.getProperty("webdriver.log"))) .build(); DesiredCapabilities capabilities = DesiredCapabilities.chrome(); capabilities.setCapability("chrome.binary", PropertiesHolder.properties.getProperty("webdriver.chrome.bin")); -// System.setProperty("webdriver.chrome.driver", properties.getProperty("webdriver.chrome.driver")); try { driverService.start(); @@ -134,11 +135,13 @@ private WebDriver configureFirefoxDriver() { firefoxBinary = new FirefoxBinary(); } - //we timeout the connection in 10 seconds -// firefoxBinary.setTimeout(SECONDS.toMillis(10)); - + /* + * TODO: Evaluate current timeout + * Timeout the connection in 30 seconds + * firefoxBinary.setTimeout(TimeUnit.SECONDS.toMillis(30)); + */ + firefoxBinary.setEnvironmentProperty("DISPLAY", PropertiesHolder.properties.getProperty("webdriver.display")); return new FirefoxDriver(firefoxBinary, makeFirefoxProfile()); -// return new FirefoxDriver(); } private FirefoxProfile makeFirefoxProfile() @@ -149,12 +152,16 @@ private FirefoxProfile makeFirefoxProfile() // TODO - look at FirefoxDriver.getProfile(). } final FirefoxProfile firefoxProfile = new FirefoxProfile(); - // firefoxProfile.setPreference("browser.safebrowsing.malware.enabled", - // false); // disables connection to sb-ssl.google.com + + /* + * TODO: Evaluate need for this + * Disable unnecessary connection to sb-ssl.google.com + * firefoxProfile.setPreference("browser.safebrowsing.malware.enabled", false); + */ + firefoxProfile.setAlwaysLoadNoFocusLib(true); firefoxProfile.setEnableNativeEvents(true); firefoxProfile.setAcceptUntrustedCertificates(true); -// firefoxProfile.setPort(8000); return firefoxProfile; } diff --git a/functional-test/src/test/resources/setup.properties b/functional-test/src/test/resources/setup.properties index dbc26a0322..072e2be2d1 100644 --- a/functional-test/src/test/resources/setup.properties +++ b/functional-test/src/test/resources/setup.properties @@ -6,7 +6,7 @@ webdriver.chrome.driver=${webdriver.chrome.driver} #this decides what web driver type we intended to use. webdriver.type=${webdriver.type} webdriver.log=${webdriver.log} - +webdriver.display=${webdriver.display} zanata.instance.url=${zanata.instance.url} zanata.sample.projects.basedir=${zanata.sample.projects.basedir} zanata.apikey=${zanata.apikey} From 49e1ee4df6a4a98a1dd6ab0bdd433665691d6144 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Tue, 16 Jul 2013 11:44:28 +1000 Subject: [PATCH 26/26] add wait until logic around dynamic field --- .../administration/ManageUserAccountPage.java | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/functional-test/src/main/java/org/zanata/page/administration/ManageUserAccountPage.java b/functional-test/src/main/java/org/zanata/page/administration/ManageUserAccountPage.java index 0368fb4cc1..1c005b80b9 100644 --- a/functional-test/src/main/java/org/zanata/page/administration/ManageUserAccountPage.java +++ b/functional-test/src/main/java/org/zanata/page/administration/ManageUserAccountPage.java @@ -28,6 +28,7 @@ import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; import org.zanata.page.AbstractPage; +import com.google.common.base.Predicate; /** * @author Damian Jansen djansen@redhat.com @@ -36,9 +37,6 @@ public class ManageUserAccountPage extends AbstractPage { - @FindBy(id = "userdetailForm:usernameField:username") - private WebElement usernameField; - @FindBy(id = "userdetailForm:passwordField:password") private WebElement passwordField; @@ -56,6 +54,9 @@ public class ManageUserAccountPage extends AbstractPage private Map roleMap; + // username field will trigger ajax call and become stale + private By usernameBy = By.id("userdetailForm:usernameField:username"); + public ManageUserAccountPage(WebDriver driver) { super(driver); @@ -67,9 +68,18 @@ public ManageUserAccountPage(WebDriver driver) roleMap.put("user", "4"); } - public ManageUserAccountPage enterUsername(String username) + public ManageUserAccountPage enterUsername(final String username) { - usernameField.sendKeys(username); + waitForTenSec().until(new Predicate() + { + @Override + public boolean apply(WebDriver input) + { + WebElement usernameField = input.findElement(usernameBy); + usernameField.sendKeys(username); + return input.findElement(usernameBy).getAttribute("value").equals(username); + } + }); return new ManageUserAccountPage(getDriver()); } @@ -117,7 +127,15 @@ public ManageUserPage cancelEditUser() public ManageUserAccountPage clearFields() { - usernameField.clear(); + waitForTenSec().until(new Predicate() + { + @Override + public boolean apply(WebDriver input) + { + input.findElement(usernameBy).clear(); + return input.findElement(usernameBy).getAttribute("value").isEmpty(); + } + }); passwordField.clear(); passwordConfirmField.clear(); return new ManageUserAccountPage(getDriver());