From 6069eeb9df11149e4c9ce4ea105ace5148d33af2 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 18 May 2015 15:39:51 +1000 Subject: [PATCH 01/10] fix potential bug to handle null argument better --- .../rest/client/StatisticsResourceClient.java | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/zanata-rest-client/src/main/java/org/zanata/rest/client/StatisticsResourceClient.java b/zanata-rest-client/src/main/java/org/zanata/rest/client/StatisticsResourceClient.java index 7bc6a071..19db011d 100644 --- a/zanata-rest-client/src/main/java/org/zanata/rest/client/StatisticsResourceClient.java +++ b/zanata-rest-client/src/main/java/org/zanata/rest/client/StatisticsResourceClient.java @@ -22,6 +22,8 @@ package org.zanata.rest.client; import java.net.URI; +import java.util.ArrayList; +import java.util.List; import javax.ws.rs.DefaultValue; import org.zanata.rest.dto.stats.ContainerTranslationStatistics; @@ -59,10 +61,20 @@ public ContainerTranslationStatistics getStatistics(String projectSlug, .queryParam("detail", String.valueOf(includeDetails)) .queryParam("word", String.valueOf(includeWordStats)) .queryParams(asMultivaluedMap("locale", - Lists.newArrayList(locales))); + toLocaleList(locales))); return webResource.get(ContainerTranslationStatistics.class); } + private static List toLocaleList(String[] locales) { + List localesList; + if (locales == null) { + localesList = Lists.newArrayList(); + } else { + localesList = Lists.newArrayList(locales); + } + return localesList; + } + @Override public ContainerTranslationStatistics getStatistics(String projectSlug, String iterationSlug, String docId, @@ -77,7 +89,7 @@ public ContainerTranslationStatistics getStatistics(String projectSlug, .path(docId) .queryParam("word", String.valueOf(includeWordStats)) .queryParams(asMultivaluedMap("locale", - Lists.newArrayList(locales))); + toLocaleList(locales))); return webResource.get(ContainerTranslationStatistics.class); } From 39020544893010ad211002fa0fb7b0685f8e9712 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 18 May 2015 15:40:35 +1000 Subject: [PATCH 02/10] rhbz1215274 - allow specify minimum accepted translation percentage --- .../org/zanata/client/ClientToServerTest.java | 2 +- .../client/commands/PushPullCommand.java | 3 + .../client/commands/pull/PullCommand.java | 105 +++++++++++++++++- .../client/commands/pull/PullOptions.java | 2 + .../client/commands/pull/PullOptionsImpl.java | 11 ++ .../client/commands/push/PushCommand.java | 2 +- .../org/zanata/maven/AbstractPullMojo.java | 12 ++ 7 files changed, 131 insertions(+), 6 deletions(-) diff --git a/zanata-cli/src/test/java/org/zanata/client/ClientToServerTest.java b/zanata-cli/src/test/java/org/zanata/client/ClientToServerTest.java index 4a5fbb0e..3b420f3a 100644 --- a/zanata-cli/src/test/java/org/zanata/client/ClientToServerTest.java +++ b/zanata-cli/src/test/java/org/zanata/client/ClientToServerTest.java @@ -47,7 +47,7 @@ public void testDisableSSLCertOption() { String project = "iok"; String version = "6.4"; client.processArgs(command, "--url", url, "--project", project, - "--project-version", version, "--disable-ssl-cert"); + "--project-version", version, "--disable-ssl-cert", "--details"); } @Test diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java index ac620ad0..ae5e2e6e 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java @@ -44,6 +44,7 @@ import org.zanata.client.exceptions.ConfigException; import org.zanata.rest.client.RestClientFactory; import org.zanata.rest.client.SourceDocResourceClient; +import org.zanata.rest.client.StatisticsResourceClient; import org.zanata.rest.client.TransDocResourceClient; import org.zanata.rest.dto.resource.Resource; import org.zanata.rest.dto.resource.ResourceMeta; @@ -67,6 +68,7 @@ public abstract class PushPullCommand extends private String modulePrefix; protected SourceDocResourceClient sourceDocResourceClient; protected TransDocResourceClient transDocResourceClient; + protected final StatisticsResourceClient statsClient; public PushPullCommand(O opts, RestClientFactory clientFactory) { super(opts, clientFactory); @@ -80,6 +82,7 @@ public PushPullCommand(O opts, RestClientFactory clientFactory) { transDocResourceClient = getClientFactory().getTransDocResourceClient(opts.getProj(), opts.getProjectVersion()); + statsClient = getClientFactory().getStatisticsClient(); } public PushPullCommand(O opts) { diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java index 9bceaa14..f9aa5114 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java @@ -28,8 +28,12 @@ import org.zanata.rest.client.RestClientFactory; import org.zanata.rest.dto.resource.Resource; import org.zanata.rest.dto.resource.TranslationsResource; +import org.zanata.rest.dto.stats.ContainerTranslationStatistics; +import org.zanata.rest.dto.stats.TranslationStatistics; import org.zanata.util.HashUtil; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; import com.sun.jersey.api.client.ClientResponse; /** @@ -123,12 +127,16 @@ public static void logOptions(Logger logger, PullOptions opts) { logger.info("Pulling target documents (translations) only"); logger.info("Target-language base directory (translations): {}", opts.getTransDir()); + logger.info("Minimum accepted translation percentage: {}%", + opts.getMinDocPercent()); } else { logger.info("Pulling source and target (translation) documents"); logger.info("Source-language directory (originals): {}", opts.getSrcDir()); logger.info("Target-language base directory (translations): {}", opts.getTransDir()); + logger.info("Minimum accepted translation percentage: {}%", + opts.getMinDocPercent()); } } @@ -137,10 +145,10 @@ public void run() throws Exception { logOptions(); LocaleList locales = getOpts().getLocaleMapList(); - if (locales == null && (getOpts().getPullType() != PushPullType.Source)) + if (locales == null && (getOpts().getPullType() != PushPullType.Source)) { throw new ConfigException("no locales specified"); - PullStrategy strat = createStrategy( - getOpts()); + } + PullStrategy strat = createStrategy(getOpts()); if (strat.isTransOnly() && getOpts().getPullType() == PushPullType.Source) { @@ -198,13 +206,19 @@ && getOpts().getPullType() == PushPullType.Source) { + "': existing source-language files may be overwritten/deleted"); confirmWithUser("This will overwrite/delete any existing documents and translations in the above directories.\n"); } else { - confirmWithUser("This will overwrite/delete any existing translations in the above directory.\n"); + confirmWithUser( + "This will overwrite/delete any existing translations in the above directory.\n"); } if (getOpts().getPurgeCache()) { eTagCache.clear(); } + Map> statsMap = null; + if (pullTarget && getOpts().getMinDocPercent() > 0) { + statsMap = getDocsTranslatedPercent(); + } + for (String qualifiedDocName : docsToPull) { try { Resource doc = null; @@ -229,6 +243,14 @@ && getOpts().getPullType() == PushPullType.Source) { File transFile = strat.getTransFileToWrite(localDocName, locMapping); + + if (!shouldPullThisLocale(statsMap, localDocName, locale)) { + log.info( + "{} is skipped due to insufficient translation percentage in locale {}", + transFile, locMapping.getLocalLocale()); + continue; + } + ETagCacheEntry eTagCacheEntry = eTagCache.findEntry(localDocName, locale.getId()); @@ -330,6 +352,59 @@ && getOpts().getPullType() == PushPullType.Source) { } + private boolean shouldPullThisLocale( + Map> statsMap, + String localDocName, LocaleId serverLocale) { + int minDocPercent = getOpts().getMinDocPercent(); + if (log.isDebugEnabled() && statsMap != null) { + log.debug("{} for locale {} is translated {}%", localDocName, + serverLocale, statsMap.get(localDocName).get(serverLocale) + .translatedPercent); + } + return statsMap == null + || statsMap.get(localDocName).get(serverLocale) + .isAboveThreshold(minDocPercent); + } + + private Map> getDocsTranslatedPercent() { + ContainerTranslationStatistics statistics = + getDetailStatisticsForProjectVersion(); + List statsPerDoc = + statistics.getDetailedStats(); + ImmutableMap.Builder> docIdToStatsBuilder = + ImmutableMap.builder(); + for (ContainerTranslationStatistics docStats : statsPerDoc) { + String docId = docStats.getId(); + List statsPerLocale = docStats.getStats(); + ImmutableMap.Builder localeToStatsBuilder = + ImmutableMap.builder(); + + for (TranslationStatistics statsForSingleLocale : statsPerLocale) { + // TODO pahuang server statistics API should return locale with + // alias + TranslatedPercent translatedPercent = + new TranslatedPercent(statsForSingleLocale.getTotal(), + statsForSingleLocale.getTranslatedOnly(), + statsForSingleLocale.getApproved()); + + localeToStatsBuilder.put( + new LocaleId(statsForSingleLocale.getLocale()), + translatedPercent); + } + Map localeStats = + localeToStatsBuilder.build(); + docIdToStatsBuilder.put(docId, localeStats); + } + return docIdToStatsBuilder.build(); + } + + @VisibleForTesting + protected ContainerTranslationStatistics getDetailStatisticsForProjectVersion() { + return statsClient + .getStatistics(getOpts().getProj(), + getOpts().getProjectVersion(), true, false, null); + } + /** * Returns a list with all documents before fromDoc removed. * @@ -401,4 +476,26 @@ private void writeTargetDoc(PullStrategy strat, String localDocName, } } + private static class TranslatedPercent { + private final int translatedPercent; + private final long total; + private final long translated; + private final long approved; + + public TranslatedPercent(long total, long translated, long approved) { + this.total = total; + this.translated = translated; + this.approved = approved; + translatedPercent = (int) ((translated + approved) * 100 / total); + } + + public boolean isAboveThreshold(int minimumPercent) { + if (minimumPercent == 100) { + return total == translated + approved; + } else { + return translatedPercent >= minimumPercent; + } + } + } + } diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptions.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptions.java index 2494c143..eb8ea04d 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptions.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptions.java @@ -43,4 +43,6 @@ public interface PullOptions extends PushPullOptions { boolean getUseCache(); boolean isContinueAfterError(); + + int getMinDocPercent(); } diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java index 37766379..c83d0e07 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java @@ -50,6 +50,7 @@ public class PullOptionsImpl extends AbstractPushPullOptionsImpl private boolean useCache = DEFAULT_USE_CACHE; private boolean purgeCache = DEFAULT_PURGE_CACHE; private boolean continueAfterError = DEFAULT_CONTINUE_AFTER_ERROR; + private int minDocPercent = 0; @Override public ZanataCommand initCommand() { @@ -165,6 +166,16 @@ public boolean isContinueAfterError() { return continueAfterError; } + @Override + public int getMinDocPercent() { + return this.minDocPercent; + } + + @Option(name = "--min-doc-percent", aliases = "-m", usage = "When specified, will only pull down translation files that have translated or better statistics above the specified percentage") + public void setMinDocPercent(int minDocPercent) { + this.minDocPercent = minDocPercent; + } + @Override public boolean isAuthRequired() { return false; diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/push/PushCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/push/PushCommand.java index 77697154..4bd49df2 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/push/PushCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/push/PushCommand.java @@ -63,7 +63,7 @@ public class PushCommand extends PushPullCommand { private CopyTransClient copyTransClient; private AsyncProcessClient asyncProcessClient; - public static interface TranslationResourcesVisitor { + public interface TranslationResourcesVisitor { void visit(LocaleMapping locale, TranslationsResource targetDoc); } diff --git a/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java b/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java index 4aeee35a..ebe188d0 100644 --- a/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java +++ b/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java @@ -107,6 +107,13 @@ public abstract class AbstractPullMojo extends */ private boolean continueAfterError = false; + /** + * When specified, will only pull down translation files that have translated or better statistics above the specified percentage. + * + * @parameter expression="$zanata.minDocPercent}" default-value="0" + */ + private int minDocPercent = 0; + /** * */ @@ -174,4 +181,9 @@ public String getCommandName() { public boolean isAuthRequired() { return false; } + + @Override + public int getMinDocPercent() { + return minDocPercent; + } } From 8c2c71f1ff516d555038831b4d48e384c1c9baaf Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 18 May 2015 16:42:10 +1000 Subject: [PATCH 03/10] rhbz1215274 - refactor: extract method to make it easier to test --- .../client/commands/pull/PullCommand.java | 169 ++++++++++-------- 1 file changed, 92 insertions(+), 77 deletions(-) diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java index f9aa5114..41a46164 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java @@ -34,6 +34,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; import com.sun.jersey.api.client.ClientResponse; /** @@ -237,91 +238,25 @@ && getOpts().getPullType() == PushPullType.Source) { } if (pullTarget) { + List skippedLocales = Lists.newArrayList(); for (LocaleMapping locMapping : locales) { LocaleId locale = new LocaleId(locMapping.getLocale()); - String eTag = null; File transFile = strat.getTransFileToWrite(localDocName, locMapping); - if (!shouldPullThisLocale(statsMap, localDocName, locale)) { - log.info( - "{} is skipped due to insufficient translation percentage in locale {}", - transFile, locMapping.getLocalLocale()); - continue; - } - - ETagCacheEntry eTagCacheEntry = - eTagCache.findEntry(localDocName, - locale.getId()); - - if (getOpts().getUseCache() && eTagCacheEntry != null) { - // Check the last updated date on the file matches - // what's in the cache - // only then use the cached ETag - if (transFile.exists() - && Long.toString(transFile.lastModified()) - .equals(eTagCacheEntry - .getLocalFileTime())) { - eTag = eTagCacheEntry.getServerETag(); - } - } - - ClientResponse transResponse = - transDocResourceClient.getTranslations(docUri, - locale, strat.getExtensions(), - createSkeletons, eTag); - - // ignore 404 (no translation yet for specified - // document) - if (transResponse.getClientResponseStatus() == ClientResponse.Status.NOT_FOUND) { - if (!createSkeletons) { - log.info( - "No translations found in locale {} for document {}", - locale, localDocName); - } else { - // Write the skeleton - writeTargetDoc(strat, localDocName, locMapping, - doc, null, - transResponse.getHeaders() - .getFirst(HttpHeaders.ETAG)); - } - } else if (transResponse.getClientResponseStatus() == ClientResponse.Status.NOT_MODIFIED) { - // 304 NOT MODIFIED (the document can stay the same) - log.info( - "No changes in translations for locale {} and document {}", - locale, localDocName); - - // Check the file's MD5 matches what's stored in the - // cache. If not, it needs to be fetched again (with - // no etag) - String fileChecksum = - HashUtil.getMD5Checksum(transFile); - if (!fileChecksum.equals(eTagCacheEntry - .getLocalFileMD5())) { - transResponse = - transDocResourceClient.getTranslations( - docUri, locale, - strat.getExtensions(), - createSkeletons, null); - ClientUtil.checkResult(transResponse); - // rewrite the target document - writeTargetDoc(strat, localDocName, locMapping, - doc, transResponse.getEntity(TranslationsResource.class), - transResponse.getHeaders() - .getFirst(HttpHeaders.ETAG)); - } + if (shouldPullThisLocale(statsMap, localDocName, locale)) { + pullDocForLocale(strat, doc, localDocName, docUri, + createSkeletons, locMapping, transFile); } else { - ClientUtil.checkResult(transResponse); - TranslationsResource targetDoc = - transResponse.getEntity(TranslationsResource.class); - - // Write the target document - writeTargetDoc(strat, localDocName, locMapping, - doc, targetDoc, - transResponse.getHeaders() - .getFirst(HttpHeaders.ETAG)); + skippedLocales.add(locale); } + + } + if (!skippedLocales.isEmpty()) { + log.info( + "Translation file for document {} for locales {} are skipped due to insufficient translate percentage", + localDocName, skippedLocales); } // write the cache @@ -352,6 +287,86 @@ && getOpts().getPullType() == PushPullType.Source) { } + @VisibleForTesting + protected void pullDocForLocale(PullStrategy strat, Resource doc, + String localDocName, String docUri, boolean createSkeletons, + LocaleMapping locMapping, + File transFile) throws IOException { + LocaleId locale = new LocaleId(locMapping.getLocale()); + String eTag = null; + ETagCacheEntry eTagCacheEntry = + eTagCache.findEntry(localDocName, + locale.getId()); + + if (getOpts().getUseCache() && eTagCacheEntry != null) { + // Check the last updated date on the file matches + // what's in the cache + // only then use the cached ETag + if (transFile.exists() + && Long.toString(transFile.lastModified()) + .equals(eTagCacheEntry + .getLocalFileTime())) { + eTag = eTagCacheEntry.getServerETag(); + } + } + + ClientResponse transResponse = + transDocResourceClient.getTranslations(docUri, + locale, strat.getExtensions(), + createSkeletons, eTag); + + // ignore 404 (no translation yet for specified + // document) + if (transResponse.getClientResponseStatus() == ClientResponse.Status.NOT_FOUND) { + if (!createSkeletons) { + log.info( + "No translations found in locale {} for document {}", + locale, localDocName); + } else { + // Write the skeleton + writeTargetDoc(strat, localDocName, locMapping, + doc, null, + transResponse.getHeaders() + .getFirst(HttpHeaders.ETAG)); + } + } else if (transResponse.getClientResponseStatus() == ClientResponse.Status.NOT_MODIFIED) { + // 304 NOT MODIFIED (the document can stay the same) + log.info( + "No changes in translations for locale {} and document {}", + locale, localDocName); + + // Check the file's MD5 matches what's stored in the + // cache. If not, it needs to be fetched again (with + // no etag) + String fileChecksum = + HashUtil.getMD5Checksum(transFile); + if (!fileChecksum.equals(eTagCacheEntry + .getLocalFileMD5())) { + transResponse = + transDocResourceClient.getTranslations( + docUri, locale, + strat.getExtensions(), + createSkeletons, null); + ClientUtil.checkResult(transResponse); + // rewrite the target document + writeTargetDoc(strat, localDocName, locMapping, + doc, transResponse.getEntity(TranslationsResource.class), + transResponse.getHeaders() + .getFirst(HttpHeaders.ETAG)); + } + } else { + ClientUtil.checkResult(transResponse); + TranslationsResource targetDoc = + transResponse.getEntity(TranslationsResource.class); + + // Write the target document + writeTargetDoc(strat, localDocName, locMapping, + doc, targetDoc, + transResponse.getHeaders() + .getFirst(HttpHeaders.ETAG)); + } + } + private boolean shouldPullThisLocale( Map> statsMap, String localDocName, LocaleId serverLocale) { From 5895ecda5c7b70a9167fd693a29e8b67beaa0b10 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Tue, 19 May 2015 11:42:37 +1000 Subject: [PATCH 04/10] rhbz1215274 - add test for minimum doc percent in pull command --- .../zanata/client/commands/BasicOptions.java | 12 +- .../client/commands/ConfigurableOptions.java | 25 +- .../client/commands/PushPullOptions.java | 4 +- .../client/commands/pull/PullCommandTest.java | 286 ++++++++++++++++++ 4 files changed, 306 insertions(+), 21 deletions(-) create mode 100644 zanata-client-commands/src/test/java/org/zanata/client/commands/pull/PullCommandTest.java diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/BasicOptions.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/BasicOptions.java index 322bd346..3cf317ef 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/BasicOptions.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/BasicOptions.java @@ -49,9 +49,9 @@ public interface BasicOptions { boolean isQuietSet(); - public boolean isInteractiveMode(); + boolean isInteractiveMode(); - public void setInteractiveMode(boolean interactiveMode); + void setInteractiveMode(boolean interactiveMode); /** * Used to generate the command line interface and its usage help. This name @@ -60,7 +60,7 @@ public interface BasicOptions { * * @return */ - public String getCommandName(); + String getCommandName(); /** * Used to generate CLI usage help. This description should preferably match @@ -68,9 +68,9 @@ public interface BasicOptions { * * @return */ - public String getCommandDescription(); + String getCommandDescription(); - public @Nonnull List getCommandHooks(); + @Nonnull List getCommandHooks(); - public void setCommandHooks(@Nonnull List commandHooks); + void setCommandHooks(@Nonnull List commandHooks); } diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/ConfigurableOptions.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/ConfigurableOptions.java index af94e989..acb688fb 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/ConfigurableOptions.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/ConfigurableOptions.java @@ -18,37 +18,37 @@ public interface ConfigurableOptions extends BasicOptions { /** * API key for accessing the REST API. Defaults to the value in zanata.ini. */ - public String getKey(); + String getKey(); - public void setKey(String key); + void setKey(String key); /** * Base URL for the server. Defaults to the value in zanata.xml. */ - public URL getUrl(); + URL getUrl(); - public void setUrl(URL url); + void setUrl(URL url); /** * Client configuration file. */ - public File getUserConfig(); + File getUserConfig(); - public void setUserConfig(File userConfig); + void setUserConfig(File userConfig); /** * Username for accessing the REST API. Defaults to the value in zanata.ini. */ - public String getUsername(); + String getUsername(); - public void setUsername(String username); + void setUsername(String username); /** * Enable HTTP message logging. */ - public boolean getLogHttp(); + boolean getLogHttp(); - public void setLogHttp(boolean traceLogging); + void setLogHttp(boolean traceLogging); /** * Disable SSL certificate verification when connecting to Zanata host by @@ -56,13 +56,12 @@ public interface ConfigurableOptions extends BasicOptions { */ boolean isDisableSSLCert(); - public - void setDisableSSLCert(boolean disableSSLCert); + void setDisableSSLCert(boolean disableSSLCert); /** * Use to disable check for presence of username and API key before running command. * * @return true if this command should fail when username or API key is absent. */ - public boolean isAuthRequired(); + boolean isAuthRequired(); } diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullOptions.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullOptions.java index dbd8d417..bd0763ab 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullOptions.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullOptions.java @@ -55,7 +55,7 @@ public interface PushPullOptions extends ConfigurableProjectOptions { File getTransDir(); - public String getFromDoc(); + String getFromDoc(); /** * This name should represent the exact parameter as it would be entered on @@ -63,7 +63,7 @@ public interface PushPullOptions extends ConfigurableProjectOptions { * parameter to the argument. This is so that the argument can be appended * directly to the parameter name. */ - public String buildFromDocArgument(String argValue); + String buildFromDocArgument(String argValue); boolean getEnableModules(); diff --git a/zanata-client-commands/src/test/java/org/zanata/client/commands/pull/PullCommandTest.java b/zanata-client-commands/src/test/java/org/zanata/client/commands/pull/PullCommandTest.java new file mode 100644 index 00000000..6d5b8359 --- /dev/null +++ b/zanata-client-commands/src/test/java/org/zanata/client/commands/pull/PullCommandTest.java @@ -0,0 +1,286 @@ +/* + * Copyright 2015, 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.client.commands.pull; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +import java.io.File; +import java.io.IOException; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.zanata.client.config.LocaleList; +import org.zanata.client.config.LocaleMapping; +import org.zanata.common.LocaleId; +import org.zanata.common.ProjectType; +import org.zanata.common.TransUnitCount; +import org.zanata.rest.StringSet; +import org.zanata.rest.client.RestClientFactory; +import org.zanata.rest.client.SourceDocResourceClient; +import org.zanata.rest.client.StatisticsResourceClient; +import org.zanata.rest.client.TransDocResourceClient; +import org.zanata.rest.dto.resource.Resource; +import org.zanata.rest.dto.stats.ContainerTranslationStatistics; +import org.zanata.rest.dto.stats.TranslationStatistics; + +import com.google.common.collect.Lists; + +public class PullCommandTest { + public static final StringSet EXTENSIONS = new StringSet("comment"); + @Mock + private RestClientFactory restClientFactory; + private PullOptionsImpl opts; + private final String projectSlug = "project"; + private final String versionSlug = "master"; + @Mock + private SourceDocResourceClient sourceClient; + @Mock + private TransDocResourceClient transClient; + @Mock + private StatisticsResourceClient statsClient; + private LocaleList locales; + private PullCommand pullCommand; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + opts = new PullOptionsImpl(); + opts.setProj(projectSlug); + opts.setProjectVersion(versionSlug); + opts.setProjectType(ProjectType.Properties.name().toLowerCase()); + opts.setBatchMode(true); + + when( + restClientFactory.getSourceDocResourceClient(projectSlug, + versionSlug)).thenReturn( + sourceClient); + when(restClientFactory.getTransDocResourceClient(projectSlug, + versionSlug)).thenReturn(transClient); + when(restClientFactory.getStatisticsClient()).thenReturn(statsClient); + + locales = new LocaleList(); + opts.setLocaleMapList(locales); + pullCommand = new PullCommand(opts, restClientFactory); + } + + @Test + public void pullSourceOnlyWillIgnoreMinimumPercent() throws Exception { + locales.add(new LocaleMapping("zh")); + locales.add(new LocaleMapping("de")); + opts.setDryRun(true); + // Given: pull-type is source only and minimum-doc-percent is set to 80 + opts.setPullType("source"); + opts.setMinDocPercent(80); + when(sourceClient + .getResource("file1", EXTENSIONS)).thenReturn( + new Resource()); + pullCommand = new PullCommand(opts, restClientFactory) { + @Override + protected List + getQualifiedDocNamesForCurrentModuleFromServer() { + return Lists.newArrayList("file1"); + } + }; + + // When: + pullCommand.run(); + + // Then: + verifyZeroInteractions(statsClient, transClient); + } + + @Test + public void pullTransOnlyWillIgnoreMinimumPercentIfItIsZero() + throws Exception { + locales.add(new LocaleMapping("zh")); + locales.add(new LocaleMapping("de")); + opts.setDryRun(true); + // Given: pull-type is trans only and minimum-doc-percent is set to 0 + opts.setPullType("trans"); + opts.setMinDocPercent(0); + + pullCommand = new PullCommand(opts, restClientFactory) { + @Override + protected List + getQualifiedDocNamesForCurrentModuleFromServer() { + return Lists.newArrayList("file1"); + } + + @Override + protected void pullDocForLocale(PullStrategy strat, Resource doc, + String localDocName, String docUri, + boolean createSkeletons, + LocaleMapping locMapping, File transFile) + throws IOException { + // pretend we are pulling + transClient.getTranslations(docUri, + new LocaleId(locMapping.getLocale()), EXTENSIONS, + createSkeletons, null); + } + }; + + // When: + pullCommand.run(); + + // Then: + verifyZeroInteractions(statsClient); + verify(transClient).getTranslations("file1", new LocaleId("zh"), + EXTENSIONS, false, + null); + verify(transClient).getTranslations("file1", new LocaleId("de"), + EXTENSIONS, false, + null); + } + + @Test + public void pullTransOnlyWillUseMinimumPercentIfItIsNotZero() + throws Exception { + locales.add(new LocaleMapping("zh")); + locales.add(new LocaleMapping("de")); + opts.setDryRun(true); + // Given: pull-type is trans only and minimum-doc-percent is set to 80 + opts.setPullType("trans"); + opts.setMinDocPercent(80); + + ContainerTranslationStatistics statistics = + new ContainerTranslationStatistics(); + ContainerTranslationStatistics docStats = + new ContainerTranslationStatistics(); + docStats.setId("file1"); + statistics.addDetailedStats(docStats); + // zh has 100 approved + TranslationStatistics zhLocaleStats = + new TranslationStatistics(new TransUnitCount(100, 0, 0, 0, 0), + "zh"); + // de has 39 approved, 21 fuzzy and 40 translated so 79% translated + TranslationStatistics deLocaleStats = + new TranslationStatistics(new TransUnitCount(39, 21, 0, 40, 0), + "de"); + docStats.addStats(zhLocaleStats); + docStats.addStats(deLocaleStats); + + when(statsClient + .getStatistics(projectSlug, versionSlug, true, false, null)) + .thenReturn(statistics); + + pullCommand = new PullCommand(opts, restClientFactory) { + @Override + protected List + getQualifiedDocNamesForCurrentModuleFromServer() { + return Lists.newArrayList("file1"); + } + + @Override + protected void pullDocForLocale(PullStrategy strat, Resource doc, + String localDocName, String docUri, + boolean createSkeletons, + LocaleMapping locMapping, File transFile) + throws IOException { + // pretend we are pulling + transClient.getTranslations(docUri, + new LocaleId(locMapping.getLocale()), EXTENSIONS, + createSkeletons, null); + } + }; + + // When: + pullCommand.run(); + + // Then: translation for "de" will not be pulled + verify(statsClient).getStatistics(projectSlug, versionSlug, true, + false, null); + verify(transClient).getTranslations("file1", new LocaleId("zh"), + EXTENSIONS, false, + null); + verifyNoMoreInteractions(transClient); + } + + @Test + public void whenMinimumPercentIsSetTo100ItWillUseTotalNumber() + throws Exception { + locales.add(new LocaleMapping("zh")); + locales.add(new LocaleMapping("de")); + opts.setDryRun(true); + // Given: pull-type is trans only and minimum-doc-percent is set to 100 + opts.setPullType("trans"); + opts.setMinDocPercent(100); + + ContainerTranslationStatistics statistics = + new ContainerTranslationStatistics(); + ContainerTranslationStatistics docStats = + new ContainerTranslationStatistics(); + docStats.setId("file1"); + statistics.addDetailedStats(docStats); + // zh has 100000 approved + TranslationStatistics zhLocaleStats = + new TranslationStatistics(new TransUnitCount(100000, 0, 0, 0, 0), + "zh"); + // de has 99999 approved, 1 untranslated so 99.999% translated + TranslationStatistics deLocaleStats = + new TranslationStatistics(new TransUnitCount(99999, 0, 1, 0, 0), + "de"); + docStats.addStats(zhLocaleStats); + docStats.addStats(deLocaleStats); + + when(statsClient + .getStatistics(projectSlug, versionSlug, true, false, null)) + .thenReturn(statistics); + + pullCommand = new PullCommand(opts, restClientFactory) { + @Override + protected List + getQualifiedDocNamesForCurrentModuleFromServer() { + return Lists.newArrayList("file1"); + } + + @Override + protected void pullDocForLocale(PullStrategy strat, Resource doc, + String localDocName, String docUri, + boolean createSkeletons, + LocaleMapping locMapping, File transFile) + throws IOException { + // pretend we are pulling + transClient.getTranslations(docUri, + new LocaleId(locMapping.getLocale()), EXTENSIONS, + createSkeletons, null); + } + }; + + // When: + pullCommand.run(); + + // Then: translation for "de" will not be pulled + verify(statsClient).getStatistics(projectSlug, versionSlug, true, + false, null); + verify(transClient).getTranslations("file1", new LocaleId("zh"), + EXTENSIONS, false, + null); + verifyNoMoreInteractions(transClient); + } + +} From ac033b281eca30b9c34f6b3fcee695fa3bb3636a Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Wed, 20 May 2015 14:08:11 +1000 Subject: [PATCH 05/10] rhbz1215274 - refactor --- .../client/commands/pull/PullCommand.java | 25 +++++++++++++------ .../client/commands/pull/PullOptionsImpl.java | 6 ++++- .../org/zanata/maven/AbstractPullMojo.java | 7 +++++- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java index 41a46164..74d3c9ba 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java @@ -33,6 +33,7 @@ import org.zanata.util.HashUtil; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Optional; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.sun.jersey.api.client.ClientResponse; @@ -215,9 +216,15 @@ && getOpts().getPullType() == PushPullType.Source) { eTagCache.clear(); } - Map> statsMap = null; + // this stats map will have docId as key, the value is another map with + // localeId as key and translated percent as value. It's optional if we + // require statistics to determine which file to pull. In cases where + // statistics is not required, i.e. pull source only or minimum percent + // is set to 0, this will be Optional.absence(). + Optional>> optionalStats = + Optional.absent(); if (pullTarget && getOpts().getMinDocPercent() > 0) { - statsMap = getDocsTranslatedPercent(); + optionalStats = Optional.of(getDocsTranslatedPercent()); } for (String qualifiedDocName : docsToPull) { @@ -245,7 +252,7 @@ && getOpts().getPullType() == PushPullType.Source) { strat.getTransFileToWrite(localDocName, locMapping); - if (shouldPullThisLocale(statsMap, localDocName, locale)) { + if (shouldPullThisLocale(optionalStats, localDocName, locale)) { pullDocForLocale(strat, doc, localDocName, docUri, createSkeletons, locMapping, transFile); } else { @@ -368,16 +375,16 @@ protected void pullDocForLocale(PullStrategy strat, Resource doc, } private boolean shouldPullThisLocale( - Map> statsMap, + Optional>> optionalStats, String localDocName, LocaleId serverLocale) { int minDocPercent = getOpts().getMinDocPercent(); - if (log.isDebugEnabled() && statsMap != null) { + if (log.isDebugEnabled() && optionalStats.isPresent()) { log.debug("{} for locale {} is translated {}%", localDocName, - serverLocale, statsMap.get(localDocName).get(serverLocale) + serverLocale, optionalStats.get().get(localDocName).get(serverLocale) .translatedPercent); } - return statsMap == null - || statsMap.get(localDocName).get(serverLocale) + return !optionalStats.isPresent() + || optionalStats.get().get(localDocName).get(serverLocale) .isAboveThreshold(minDocPercent); } @@ -505,6 +512,8 @@ public TranslatedPercent(long total, long translated, long approved) { } public boolean isAboveThreshold(int minimumPercent) { + // if minimum percent is 100, we will compare exact number so that + // rounding issue won't affect the result if (minimumPercent == 100) { return total == translated + approved; } else { diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java index c83d0e07..9ac3a02b 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java @@ -26,6 +26,7 @@ import org.zanata.client.commands.BooleanValueHandler; import org.zanata.client.commands.ZanataCommand; import org.zanata.client.commands.PushPullType; +import com.google.common.base.Preconditions; /** * @author Sean Flanigan = 0 && minDocPercent <= 100, + "--min-doc-percent should be an integer from 0 to 100"); this.minDocPercent = minDocPercent; } diff --git a/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java b/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java index ebe188d0..ed8bdd91 100644 --- a/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java +++ b/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java @@ -27,6 +27,7 @@ import org.zanata.client.commands.pull.PullOptions; import org.zanata.client.commands.pull.RawPullCommand; +import com.google.common.base.Preconditions; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; /** @@ -108,7 +109,8 @@ public abstract class AbstractPullMojo extends private boolean continueAfterError = false; /** - * When specified, will only pull down translation files that have translated or better statistics above the specified percentage. + * Accepts integer from 0 to 100. Only pull translation documents + * which are at least PERCENT % completed. * * @parameter expression="$zanata.minDocPercent}" default-value="0" */ @@ -119,6 +121,9 @@ public abstract class AbstractPullMojo extends */ public AbstractPullMojo() { super(); + Preconditions + .checkArgument(minDocPercent >= 0 && minDocPercent <= 100, + "zanata.minDocPercent should be an integer from 0 to 100"); } public PushPullCommand initCommand() { From 019a1ae9d658d922661c54caa7eaf304735c6b07 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 22 May 2015 13:48:19 +1000 Subject: [PATCH 06/10] rhbz1215274 - enable min doc percent for file type project --- .../client/commands/PushPullCommand.java | 113 ++++++++++++++++++ .../client/commands/pull/PullCommand.java | 94 +-------------- .../client/commands/pull/RawPullCommand.java | 86 ++++++++----- 3 files changed, 169 insertions(+), 124 deletions(-) diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java index ae5e2e6e..f26a95f4 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java @@ -30,18 +30,25 @@ import java.net.URI; import java.util.ArrayList; import java.util.List; +import java.util.Map; import javax.xml.bind.JAXBContext; import javax.xml.bind.JAXBException; import javax.xml.bind.Marshaller; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Optional; +import com.google.common.collect.ImmutableMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.zanata.client.commands.pull.PullCommand; +import org.zanata.client.commands.pull.PullOptions; import org.zanata.client.config.LocaleList; import org.zanata.client.config.LocaleMapping; import org.zanata.client.etag.ETagCache; import org.zanata.client.etag.ETagCacheReaderWriter; import org.zanata.client.exceptions.ConfigException; +import org.zanata.common.LocaleId; import org.zanata.rest.client.RestClientFactory; import org.zanata.rest.client.SourceDocResourceClient; import org.zanata.rest.client.StatisticsResourceClient; @@ -49,6 +56,8 @@ import org.zanata.rest.dto.resource.Resource; import org.zanata.rest.dto.resource.ResourceMeta; import org.zanata.rest.dto.resource.TranslationsResource; +import org.zanata.rest.dto.stats.ContainerTranslationStatistics; +import org.zanata.rest.dto.stats.TranslationStatistics; import org.zanata.util.PathUtil; /** @@ -248,4 +257,108 @@ protected void storeETagCache() { } } + protected Map> getDocsTranslatedPercent() { + ContainerTranslationStatistics statistics = + getDetailStatisticsForProjectVersion(); + List statsPerDoc = + statistics.getDetailedStats(); + ImmutableMap.Builder> docIdToStatsBuilder = + ImmutableMap.builder(); + for (ContainerTranslationStatistics docStats : statsPerDoc) { + String docId = docStats.getId(); + List statsPerLocale = docStats.getStats(); + ImmutableMap.Builder localeToStatsBuilder = + ImmutableMap.builder(); + + for (TranslationStatistics statsForSingleLocale : statsPerLocale) { + // TODO server statistics API should return locale with alias + TranslatedPercent translatedPercent = + new TranslatedPercent(statsForSingleLocale.getTotal(), + statsForSingleLocale.getTranslatedOnly(), + statsForSingleLocale.getApproved()); + + localeToStatsBuilder.put( + new LocaleId(statsForSingleLocale.getLocale()), + translatedPercent); + } + Map localeStats = + localeToStatsBuilder.build(); + docIdToStatsBuilder.put(docId, localeStats); + } + return docIdToStatsBuilder.build(); + } + + @VisibleForTesting + protected ContainerTranslationStatistics getDetailStatisticsForProjectVersion() { + return statsClient + .getStatistics(getOpts().getProj(), + getOpts().getProjectVersion(), true, false, null); + } + + /** + * this stats map will have docId as key, the value is another map with + * localeId as key and translated percent as value. + * It's optional if we require statistics to determine which file to pull. + * In cases where statistics is not required, + * i.e. pull source only or minimum percent is set to 0, this will be + * Optional.absence(). + * + * @param pullTarget whether we need to pull translation target + * @return either detailed document statistics or optional.absence() + */ + protected Optional>> prepareStatsIfApplicable( + boolean pullTarget) { + + Optional>> optionalStats = + Optional.absent(); + O opts = getOpts(); + if (pullTarget && opts instanceof PullOptions && + ((PullOptions) opts).getMinDocPercent() > 0) { + optionalStats = Optional.of(getDocsTranslatedPercent()); + } + return optionalStats; + } + + protected boolean shouldPullThisLocale( + Optional>> optionalStats, + String localDocName, LocaleId serverLocale) { + int minDocPercent = ((PullOptions) getOpts()).getMinDocPercent(); + if (log.isDebugEnabled() && optionalStats.isPresent()) { + log.debug("{} for locale {} is translated {}%", localDocName, + serverLocale, optionalStats.get() + .get(localDocName).get(serverLocale) + .getTranslatedPercent()); + } + return !optionalStats.isPresent() + || optionalStats.get().get(localDocName).get(serverLocale) + .isAboveThreshold(minDocPercent); + } + + protected static class TranslatedPercent { + protected final int translatedPercent; + private final long total; + private final long translated; + private final long approved; + + public TranslatedPercent(long total, long translated, long approved) { + this.total = total; + this.translated = translated; + this.approved = approved; + translatedPercent = (int) ((translated + approved) * 100 / total); + } + + public boolean isAboveThreshold(int minimumPercent) { + // if minimum percent is 100, we will compare exact number so that + // rounding issue won't affect the result + if (minimumPercent == 100) { + return total == translated + approved; + } else { + return translatedPercent >= minimumPercent; + } + } + + public int getTranslatedPercent() { + return translatedPercent; + } + } } diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java index 74d3c9ba..9f054e5c 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java @@ -28,13 +28,10 @@ import org.zanata.rest.client.RestClientFactory; import org.zanata.rest.dto.resource.Resource; import org.zanata.rest.dto.resource.TranslationsResource; -import org.zanata.rest.dto.stats.ContainerTranslationStatistics; -import org.zanata.rest.dto.stats.TranslationStatistics; import org.zanata.util.HashUtil; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.sun.jersey.api.client.ClientResponse; @@ -215,17 +212,9 @@ && getOpts().getPullType() == PushPullType.Source) { if (getOpts().getPurgeCache()) { eTagCache.clear(); } - - // this stats map will have docId as key, the value is another map with - // localeId as key and translated percent as value. It's optional if we - // require statistics to determine which file to pull. In cases where - // statistics is not required, i.e. pull source only or minimum percent - // is set to 0, this will be Optional.absence(). Optional>> optionalStats = - Optional.absent(); - if (pullTarget && getOpts().getMinDocPercent() > 0) { - optionalStats = Optional.of(getDocsTranslatedPercent()); - } + prepareStatsIfApplicable(pullTarget); + for (String qualifiedDocName : docsToPull) { try { @@ -262,7 +251,7 @@ && getOpts().getPullType() == PushPullType.Source) { } if (!skippedLocales.isEmpty()) { log.info( - "Translation file for document {} for locales {} are skipped due to insufficient translate percentage", + "Translation file for document {} for locales {} are skipped due to insufficient completed percentage", localDocName, skippedLocales); } @@ -374,59 +363,6 @@ protected void pullDocForLocale(PullStrategy strat, Resource doc, } } - private boolean shouldPullThisLocale( - Optional>> optionalStats, - String localDocName, LocaleId serverLocale) { - int minDocPercent = getOpts().getMinDocPercent(); - if (log.isDebugEnabled() && optionalStats.isPresent()) { - log.debug("{} for locale {} is translated {}%", localDocName, - serverLocale, optionalStats.get().get(localDocName).get(serverLocale) - .translatedPercent); - } - return !optionalStats.isPresent() - || optionalStats.get().get(localDocName).get(serverLocale) - .isAboveThreshold(minDocPercent); - } - - private Map> getDocsTranslatedPercent() { - ContainerTranslationStatistics statistics = - getDetailStatisticsForProjectVersion(); - List statsPerDoc = - statistics.getDetailedStats(); - ImmutableMap.Builder> docIdToStatsBuilder = - ImmutableMap.builder(); - for (ContainerTranslationStatistics docStats : statsPerDoc) { - String docId = docStats.getId(); - List statsPerLocale = docStats.getStats(); - ImmutableMap.Builder localeToStatsBuilder = - ImmutableMap.builder(); - - for (TranslationStatistics statsForSingleLocale : statsPerLocale) { - // TODO pahuang server statistics API should return locale with - // alias - TranslatedPercent translatedPercent = - new TranslatedPercent(statsForSingleLocale.getTotal(), - statsForSingleLocale.getTranslatedOnly(), - statsForSingleLocale.getApproved()); - - localeToStatsBuilder.put( - new LocaleId(statsForSingleLocale.getLocale()), - translatedPercent); - } - Map localeStats = - localeToStatsBuilder.build(); - docIdToStatsBuilder.put(docId, localeStats); - } - return docIdToStatsBuilder.build(); - } - - @VisibleForTesting - protected ContainerTranslationStatistics getDetailStatisticsForProjectVersion() { - return statsClient - .getStatistics(getOpts().getProj(), - getOpts().getProjectVersion(), true, false, null); - } - /** * Returns a list with all documents before fromDoc removed. * @@ -498,28 +434,4 @@ private void writeTargetDoc(PullStrategy strat, String localDocName, } } - private static class TranslatedPercent { - private final int translatedPercent; - private final long total; - private final long translated; - private final long approved; - - public TranslatedPercent(long total, long translated, long approved) { - this.total = total; - this.translated = translated; - this.approved = approved; - translatedPercent = (int) ((translated + approved) * 100 / total); - } - - public boolean isAboveThreshold(int minimumPercent) { - // if minimum percent is 100, we will compare exact number so that - // rounding issue won't affect the result - if (minimumPercent == 100) { - return total == translated + approved; - } else { - return translatedPercent >= minimumPercent; - } - } - } - } diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java index 4bec6af9..1a8e532a 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java @@ -23,9 +23,11 @@ import java.io.IOException; import java.io.InputStream; import java.util.List; +import java.util.Map; import java.util.SortedSet; import java.util.TreeSet; +import com.google.common.collect.Lists; import org.apache.commons.io.FilenameUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,7 +36,6 @@ import org.zanata.client.config.LocaleList; import org.zanata.client.config.LocaleMapping; import org.zanata.client.exceptions.ConfigException; -import org.zanata.common.DocumentType; import org.zanata.common.LocaleId; import org.zanata.rest.client.ClientUtil; import org.zanata.rest.client.FileResourceClient; @@ -45,8 +46,6 @@ import com.google.common.base.Optional; import com.sun.jersey.api.client.ClientResponse; -import javax.ws.rs.core.HttpHeaders; - /** * * @author David Mason, >> optionalStats = + prepareStatsIfApplicable(pullTarget); + for (String qualifiedDocName : docsToPull) { // TODO add filtering by file type? e.g. pull all dtd documents // only. @@ -188,40 +190,23 @@ public void run() throws IOException { FileResource.FILETYPE_TRANSLATED_APPROVED; } + List skippedLocales = Lists.newArrayList(); for (LocaleMapping locMapping : locales) { LocaleId locale = new LocaleId(locMapping.getLocale()); - ClientResponse response = - fileResourceClient.downloadTranslationFile(getOpts() - .getProj(), getOpts() - .getProjectVersion(), locale.getId(), - fileExtension, qualifiedDocName); - if (response.getClientResponseStatus() == ClientResponse.Status.NOT_FOUND) { - log.info( - "No translation document file found in locale {} for document [{}]", - locale, qualifiedDocName); + + if (shouldPullThisLocale(optionalStats, localDocName, locale)) { + pullDocForLocale(strat, qualifiedDocName, localDocName, + fileExtension, + locMapping, locale); } else { - ClientUtil.checkResult(response); - InputStream transDoc = - (InputStream) response - .getEntity(InputStream.class); - if (transDoc != null) { - try { - String fileName = - ClientUtil.getFileNameFromHeader( - response.getHeaders()); - String targetFileExt = FilenameUtils - .getExtension(fileName); - - Optional translationFileExtension = - Optional.fromNullable(targetFileExt); - - strat.writeTransFile(localDocName, - locMapping, transDoc, translationFileExtension); - } finally { - transDoc.close(); - } - } + skippedLocales.add(locale); } + + } + if (!skippedLocales.isEmpty()) { + log.info( + "Translation file for document {} for locales {} are skipped due to insufficient completed percentage", + localDocName, skippedLocales); } } } catch (IOException | RuntimeException e) { @@ -233,4 +218,39 @@ public void run() throws IOException { } } } + + private void pullDocForLocale(RawPullStrategy strat, + String qualifiedDocName, String localDocName, String fileExtension, + LocaleMapping locMapping, LocaleId locale) throws IOException { + ClientResponse response = + fileResourceClient.downloadTranslationFile(getOpts() + .getProj(), getOpts() + .getProjectVersion(), locale.getId(), + fileExtension, qualifiedDocName); + if (response.getClientResponseStatus() == ClientResponse.Status.NOT_FOUND) { + log.info( + "No translation document file found in locale {} for document [{}]", + locale, qualifiedDocName); + } else { + ClientUtil.checkResult(response); + InputStream transDoc = response.getEntity(InputStream.class); + if (transDoc != null) { + try { + String fileName = + ClientUtil.getFileNameFromHeader( + response.getHeaders()); + String targetFileExt = FilenameUtils + .getExtension(fileName); + + Optional translationFileExtension = + Optional.fromNullable(targetFileExt); + + strat.writeTransFile(localDocName, + locMapping, transDoc, translationFileExtension); + } finally { + transDoc.close(); + } + } + } + } } From 31d5eed1ed4f5129ab9a5587e540947adba984cc Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Tue, 26 May 2015 14:00:22 +1000 Subject: [PATCH 07/10] rhbz1215274 - fix issues - total number can be zero - maven argument missing a bracket - optimize stats to only include asking locales - use double for percentage calculation --- .../client/commands/PushPullCommand.java | 33 ++++++++++++++----- .../client/commands/pull/PullCommand.java | 2 +- .../client/commands/pull/PullOptionsImpl.java | 4 ++- .../client/commands/pull/RawPullCommand.java | 2 +- .../client/commands/pull/PullCommandTest.java | 8 ++--- .../org/zanata/maven/AbstractPullMojo.java | 7 ++-- 6 files changed, 37 insertions(+), 19 deletions(-) diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java index f26a95f4..d8d30568 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java @@ -27,6 +27,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.StringWriter; +import java.math.BigInteger; import java.net.URI; import java.util.ArrayList; import java.util.List; @@ -257,9 +258,10 @@ protected void storeETagCache() { } } - protected Map> getDocsTranslatedPercent() { + protected Map> getDocsTranslatedPercent( + LocaleList locales) { ContainerTranslationStatistics statistics = - getDetailStatisticsForProjectVersion(); + getDetailStatisticsForProjectVersion(locales); List statsPerDoc = statistics.getDetailedStats(); ImmutableMap.Builder> docIdToStatsBuilder = @@ -289,10 +291,15 @@ protected Map> getDocsTranslatedPercent } @VisibleForTesting - protected ContainerTranslationStatistics getDetailStatisticsForProjectVersion() { + protected ContainerTranslationStatistics getDetailStatisticsForProjectVersion( + LocaleList locales) { + String[] localesOnServer = new String[locales.size()]; + for (int i = 0; i < locales.size(); i++) { + localesOnServer[i] = locales.get(i).getLocale(); + } return statsClient .getStatistics(getOpts().getProj(), - getOpts().getProjectVersion(), true, false, null); + getOpts().getProjectVersion(), true, false, localesOnServer); } /** @@ -304,17 +311,18 @@ protected ContainerTranslationStatistics getDetailStatisticsForProjectVersion() * Optional.absence(). * * @param pullTarget whether we need to pull translation target + * @param locales * @return either detailed document statistics or optional.absence() */ protected Optional>> prepareStatsIfApplicable( - boolean pullTarget) { + boolean pullTarget, LocaleList locales) { Optional>> optionalStats = Optional.absent(); O opts = getOpts(); if (pullTarget && opts instanceof PullOptions && ((PullOptions) opts).getMinDocPercent() > 0) { - optionalStats = Optional.of(getDocsTranslatedPercent()); + optionalStats = Optional.of(getDocsTranslatedPercent(locales)); } return optionalStats; } @@ -335,7 +343,7 @@ protected boolean shouldPullThisLocale( } protected static class TranslatedPercent { - protected final int translatedPercent; + private final double translatedPercent; private final long total; private final long translated; private final long approved; @@ -344,7 +352,14 @@ public TranslatedPercent(long total, long translated, long approved) { this.total = total; this.translated = translated; this.approved = approved; - translatedPercent = (int) ((translated + approved) * 100 / total); + if (total == 0) { + // in some case the document has no content, we want to pull the + // translation file regardless + translatedPercent = 100; + } else { + translatedPercent = (translated + approved) * 100.0 / total; + + } } public boolean isAboveThreshold(int minimumPercent) { @@ -357,7 +372,7 @@ public boolean isAboveThreshold(int minimumPercent) { } } - public int getTranslatedPercent() { + public double getTranslatedPercent() { return translatedPercent; } } diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java index 9f054e5c..ff871163 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java @@ -213,7 +213,7 @@ && getOpts().getPullType() == PushPullType.Source) { eTagCache.clear(); } Optional>> optionalStats = - prepareStatsIfApplicable(pullTarget); + prepareStatsIfApplicable(pullTarget, locales); for (String qualifiedDocName : docsToPull) { diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java index 9ac3a02b..a32f2377 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java @@ -172,7 +172,9 @@ public int getMinDocPercent() { return this.minDocPercent; } - @Option(name = "--min-doc-percent", metaVar = "PERCENT", usage = "Accepts integer from 0 to 100. Only pull translation documents which are at least PERCENT % completed.") + @Option(name = "--min-doc-percent", metaVar = "PERCENT", + usage = "Accepts integer from 0 to 100. Only pull translation documents which are at least PERCENT % completed.\n" + + "Please note specifying this option may cause longer time to pull for a large project") public void setMinDocPercent(int minDocPercent) { Preconditions .checkArgument(minDocPercent >= 0 && minDocPercent <= 100, diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java index 1a8e532a..08d5c273 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java @@ -146,7 +146,7 @@ public void run() throws IOException { } Optional>> optionalStats = - prepareStatsIfApplicable(pullTarget); + prepareStatsIfApplicable(pullTarget, locales); for (String qualifiedDocName : docsToPull) { // TODO add filtering by file type? e.g. pull all dtd documents diff --git a/zanata-client-commands/src/test/java/org/zanata/client/commands/pull/PullCommandTest.java b/zanata-client-commands/src/test/java/org/zanata/client/commands/pull/PullCommandTest.java index 6d5b8359..4f18e919 100644 --- a/zanata-client-commands/src/test/java/org/zanata/client/commands/pull/PullCommandTest.java +++ b/zanata-client-commands/src/test/java/org/zanata/client/commands/pull/PullCommandTest.java @@ -185,7 +185,7 @@ public void pullTransOnlyWillUseMinimumPercentIfItIsNotZero() docStats.addStats(deLocaleStats); when(statsClient - .getStatistics(projectSlug, versionSlug, true, false, null)) + .getStatistics(projectSlug, versionSlug, true, false, new String[] {"zh", "de"})) .thenReturn(statistics); pullCommand = new PullCommand(opts, restClientFactory) { @@ -213,7 +213,7 @@ protected void pullDocForLocale(PullStrategy strat, Resource doc, // Then: translation for "de" will not be pulled verify(statsClient).getStatistics(projectSlug, versionSlug, true, - false, null); + false, new String[] {"zh", "de"}); verify(transClient).getTranslations("file1", new LocaleId("zh"), EXTENSIONS, false, null); @@ -248,7 +248,7 @@ public void whenMinimumPercentIsSetTo100ItWillUseTotalNumber() docStats.addStats(deLocaleStats); when(statsClient - .getStatistics(projectSlug, versionSlug, true, false, null)) + .getStatistics(projectSlug, versionSlug, true, false, new String[] {"zh", "de"})) .thenReturn(statistics); pullCommand = new PullCommand(opts, restClientFactory) { @@ -276,7 +276,7 @@ protected void pullDocForLocale(PullStrategy strat, Resource doc, // Then: translation for "de" will not be pulled verify(statsClient).getStatistics(projectSlug, versionSlug, true, - false, null); + false, new String[] {"zh", "de"}); verify(transClient).getTranslations("file1", new LocaleId("zh"), EXTENSIONS, false, null); diff --git a/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java b/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java index ed8bdd91..820e904b 100644 --- a/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java +++ b/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java @@ -109,10 +109,11 @@ public abstract class AbstractPullMojo extends private boolean continueAfterError = false; /** - * Accepts integer from 0 to 100. Only pull translation documents - * which are at least PERCENT % completed. + * Accepts integer from 0 to 100. Only pull translation documents which are + * at least PERCENT % completed. Please note specifying this option may + * cause longer time to pull for a large project. * - * @parameter expression="$zanata.minDocPercent}" default-value="0" + * @parameter expression="${zanata.minDocPercent}" default-value="0" */ private int minDocPercent = 0; From 243afc8722b3cc8500d885070eccbed543cc0d79 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Tue, 26 May 2015 14:31:53 +1000 Subject: [PATCH 08/10] rhbz1215274 - log info if stats is required which may take longer time to process --- .../java/org/zanata/client/commands/PushPullCommand.java | 8 ++++++-- .../java/org/zanata/client/commands/pull/PullCommand.java | 4 ++++ .../org/zanata/client/commands/pull/RawPullCommand.java | 4 ++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java index dd3c9608..a20c76e2 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java @@ -314,13 +314,17 @@ protected Optional>> prepareStatsIf Optional>> optionalStats = Optional.absent(); O opts = getOpts(); - if (pullTarget && opts instanceof PullOptions && - ((PullOptions) opts).getMinDocPercent() > 0) { + if (needToGetStatistics(pullTarget)) { optionalStats = Optional.of(getDocsTranslatedPercent(locales)); } return optionalStats; } + protected boolean needToGetStatistics(boolean pullTarget) { + return pullTarget && getOpts() instanceof PullOptions && + ((PullOptions) getOpts()).getMinDocPercent() > 0; + } + protected boolean shouldPullThisLocale( Optional>> optionalStats, String localDocName, LocaleId serverLocale) { diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java index ff871163..ffb218a0 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java @@ -199,6 +199,10 @@ && getOpts().getPullType() == PushPullType.Source) { pullSrc = false; } + if (needToGetStatistics(pullTarget)) { + log.info("Setting minimum document completion percentage may potentially increase the processing time."); + } + if (pullSrc) { log.warn("Pull Type set to '" + pullType diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java index 08d5c273..6df41791 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/RawPullCommand.java @@ -136,6 +136,10 @@ public void run() throws IOException { boolean pullTarget = pullType == PushPullType.Both || pullType == PushPullType.Trans; + if (needToGetStatistics(pullTarget)) { + log.info("Setting minimum document completion percentage may potentially increase the processing time."); + } + if (pullSrc) { log.warn("Pull Type set to '" + pullType From 2f0ab59fad05fe251cc14cb96ae12e7ba1e5d5a1 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Tue, 26 May 2015 14:38:00 +1000 Subject: [PATCH 09/10] fix fingbugs error --- .../main/java/org/zanata/client/commands/PushPullCommand.java | 1 - 1 file changed, 1 deletion(-) diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java index a20c76e2..9a425fd4 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/PushPullCommand.java @@ -313,7 +313,6 @@ protected Optional>> prepareStatsIf Optional>> optionalStats = Optional.absent(); - O opts = getOpts(); if (needToGetStatistics(pullTarget)) { optionalStats = Optional.of(getDocsTranslatedPercent(locales)); } From a0cd16978d6590b6509ec8fab5f8adff2fb8c521 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Wed, 27 May 2015 11:20:28 +1000 Subject: [PATCH 10/10] rhbz1215274 - add info to indicate the stats is message based --- .../java/org/zanata/client/commands/pull/PullCommand.java | 4 ++-- .../java/org/zanata/client/commands/pull/PullOptionsImpl.java | 2 +- .../src/main/java/org/zanata/maven/AbstractPullMojo.java | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java index ffb218a0..39215e6f 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullCommand.java @@ -126,7 +126,7 @@ public static void logOptions(Logger logger, PullOptions opts) { logger.info("Pulling target documents (translations) only"); logger.info("Target-language base directory (translations): {}", opts.getTransDir()); - logger.info("Minimum accepted translation percentage: {}%", + logger.info("Minimum accepted translation percentage (message based): {}%", opts.getMinDocPercent()); } else { logger.info("Pulling source and target (translation) documents"); @@ -134,7 +134,7 @@ public static void logOptions(Logger logger, PullOptions opts) { opts.getSrcDir()); logger.info("Target-language base directory (translations): {}", opts.getTransDir()); - logger.info("Minimum accepted translation percentage: {}%", + logger.info("Minimum accepted translation percentage (message based): {}%", opts.getMinDocPercent()); } } diff --git a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java index f01fd285..e1703a1c 100644 --- a/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java +++ b/zanata-client-commands/src/main/java/org/zanata/client/commands/pull/PullOptionsImpl.java @@ -173,7 +173,7 @@ public int getMinDocPercent() { } @Option(name = "--min-doc-percent", metaVar = "PERCENT", - usage = "Accepts integer from 0 to 100. Only pull translation documents which are at least PERCENT % completed.\n" + + usage = "Accepts integer from 0 to 100. Only pull translation documents which are at least PERCENT % (message based) completed.\n" + "Please note specifying this option may cause longer time to pull for a large project") public void setMinDocPercent(int minDocPercent) { Preconditions diff --git a/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java b/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java index 820e904b..5602b2ed 100644 --- a/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java +++ b/zanata-maven-plugin/src/main/java/org/zanata/maven/AbstractPullMojo.java @@ -110,8 +110,8 @@ public abstract class AbstractPullMojo extends /** * Accepts integer from 0 to 100. Only pull translation documents which are - * at least PERCENT % completed. Please note specifying this option may - * cause longer time to pull for a large project. + * at least PERCENT % (message based) completed. Please note specifying this + * option may cause longer time to pull for a large project. * * @parameter expression="${zanata.minDocPercent}" default-value="0" */