Skip to content

Commit

Permalink
CLDR-16946 Fix triple up arrow producing error in VXML (unicode-org#3501
Browse files Browse the repository at this point in the history
)

-Replace makeVettedFile with return make(loc.getBaseName(), false)

-Remove dead code including VoteLoadingContext.VXML_GENERATION, makeVettedSource, makeVettedFile

-Baseline has long been used instead of last-release; obsolete comments indicated VXML required VoteResolver for last-release

-Comments
  • Loading branch information
btangmu authored Feb 13, 2024
1 parent 78e3f23 commit 7c82449
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ VoteResolver<String> setValueFromResolver(
*
* Do not skip vote resolution if VoteLoadingContext.SINGLE_VOTE, even for empty xpd. Otherwise an Abstain can
* result in "no votes", "skip vote resolution", failure to get the right winning value, possibly inherited.
*
* Do not skip vote resolution if VoteLoadingContext.VXML_GENERATION, even for empty xpd. We may need to call
* getWinningValue for vote resolution for a larger set of paths to get baseline, etc.
*/

/**
Expand Down Expand Up @@ -98,7 +95,6 @@ private String getFullPathWithResolver(String path, VoteResolver<String> resolve
/*
* Catch Status.missing, or it will trigger an exception in draftStatusFromWinningStatus
* since there is no "missing" in DraftStatus.
* This may happen especially for VoteLoadingContext.VXML_GENERATION.
*
* Status.missing can also occur for VoteLoadingContext.SINGLE_VOTE, when a user abstains
* after submitting a new value. Then, delegate.removeValueAtDPath and/or delegate.putValueAtPath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ private File writeManualOutputFile(File vetDataDir, CLDRLocale loc, Kind kind) {
long st = System.currentTimeMillis();
CLDRFile cldrFile;
if (kind == Kind.vxml) {
cldrFile = sm.getSTFactory().makeVettedFile(loc);
cldrFile = sm.getSTFactory().make(loc.getBaseName(), false);
} else if (kind == Kind.pxml) {
cldrFile = sm.getSTFactory().makeProposedFile(loc);
} else {
Expand Down
88 changes: 10 additions & 78 deletions tools/cldr-apps/src/main/java/org/unicode/cldr/web/STFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,6 @@ enum VoteLoadingContext {
*/
ORDINARY_LOAD_VOTES,

/**
* The special context when loadVoteValues is called by makeVettedSource for generating VXML
*/
VXML_GENERATION,

/**
* The context when a voting (or abstaining) event occurs and setValueFromResolver is called
* by voteForValue (not used for loadVoteValues)
Expand Down Expand Up @@ -359,7 +354,7 @@ public Date getLastModDate() {
new BallotBoxXMLSource<User>(
diskDataEntry.diskData.cloneAsThawed(), this);
registerXmlSource(dataBackedSource);
loadVoteValues(dataBackedSource, VoteLoadingContext.ORDINARY_LOAD_VOTES);
loadVoteValues();
nextStamp();
XMLSource resolvedXmlsource = makeResolvingSource();
rFile =
Expand Down Expand Up @@ -428,18 +423,10 @@ private boolean isValidSurveyToolVote(UserRegistry.User user, String xpath) {
}

/**
* Load internal data (votes, etc.) for this PerLocaleData, and push it into the given
* DataBackedSource.
*
* @param targetXmlSource the DataBackedSource which might or might not equal
* this.xmlsource; for makeVettedSource, it is a different (uncached) DataBackedSource.
* @param voteLoadingContext VoteLoadingContext.ORDINARY_LOAD_VOTES or
* VoteLoadingContext.VXML_GENERATION (not VoteLoadingContext.SINGLE_VOTE)
* <p>Called by PerLocaleData.makeSource (with VoteLoadingContext.ORDINARY_LOAD_VOTES)
* and by PerLocaleData.makeVettedSource (with VoteLoadingContext.VXML_GENERATION).
* Load internal data (votes, etc.) for this PerLocaleData, and push it into
* dataBackedSource
*/
private void loadVoteValues(
BallotBoxXMLSource<User> targetXmlSource, VoteLoadingContext voteLoadingContext) {
private void loadVoteValues() {
VoteResolver<String> resolver = null; // save recalculating this.
ElapsedTimer et =
(SurveyLog.DEBUG) ? new ElapsedTimer("Loading PLD for " + locale) : null;
Expand Down Expand Up @@ -554,35 +541,17 @@ private void loadVoteValues(
: null;
/*
* Now that we've loaded all the votes, resolve the votes for each path.
*
* For VoteLoadingContext.VXML_GENERATION we use all paths in diskData (trunk) in
* addition to allPXDPaths(); otherwise, vxml produced by OutputFileManager is missing some paths.
* allPXDPaths() may return an empty array if there are no votes in current votes table.
* (However, we assume that last-release value soon won't be used anymore for vote resolution.
* If we did need paths from last-release, or any paths missing from trunk and current votes table,
* we could loop through sm.getSTFactory().getPathsForFile(locale); however, that would generally
* include more paths than are wanted for vxml.)
* Reference: https://unicode-org.atlassian.net/browse/CLDR-11909
*
* TODO: revisit whether this difference for VoteLoadingContext.VXML_GENERATION is still necessary; when added
* cases where last-release value made a difference to vote resolution; now that "baseline" = trunk not
* last-release it's possible that vote resolution isn't needed for items without current votes.
*/
Set<String> xpathSet;
if (voteLoadingContext == VoteLoadingContext.VXML_GENERATION) {
xpathSet = new HashSet<>(allPXDPaths());
for (String xp : diskDataEntry.diskData) {
xpathSet.add(xp);
}
} else { // voteLoadingContext == VoteLoadingContext.ORDINARY_LOAD_VOTES
xpathSet = allPXDPaths();
}
Set<String> xpathSet = allPXDPaths();
int j = 0;
for (String xp : xpathSet) {
try {
resolver =
targetXmlSource.setValueFromResolver(
xp, resolver, voteLoadingContext, peekXpathData(xp));
dataBackedSource.setValueFromResolver(
xp,
resolver,
VoteLoadingContext.ORDINARY_LOAD_VOTES,
peekXpathData(xp));
} catch (Exception e) {
e.printStackTrace();
SurveyLog.logException(logger, e, "In setValueFromResolver, xp = " + xp);
Expand Down Expand Up @@ -779,23 +748,6 @@ public String getVoteValue(User user, String distinguishingXpath) {
}
}

/**
* Make a vetted source for this PerLocaleData, suitable for producing vxml with
* vote-resolution done on more paths.
*
* <p>This function is similar to makeSource, but with VoteLoadingContext.VXML_GENERATION.
*
* @return the DataBackedSource (NOT the same as PerLocaleData.xmlsource)
*/
private synchronized XMLSource makeVettedSource() {
BallotBoxXMLSource<User> vxmlSource =
new BallotBoxXMLSource<User>(diskDataEntry.diskData.cloneAsThawed(), this);
if (!readonly) {
loadVoteValues(vxmlSource, VoteLoadingContext.VXML_GENERATION);
}
return vxmlSource;
}

@Override
public void unvoteFor(User user, String distinguishingXpath)
throws BallotBox.InvalidXPathException, VoteNotAcceptedException {
Expand Down Expand Up @@ -1456,26 +1408,6 @@ public CLDRFile make(CLDRLocale loc, boolean resolved) {
return make(loc.getBaseName(), resolved);
}

/**
* Make a "vetted" CLDRFile with more paths resolved, for generating VXML (vetted XML).
*
* <p>See loadVoteValues for what exactly "more paths" means.
*
* <p>This kind of CLDRFile should not be confused with ordinary (not-fully-vetted) files, or
* re-used for anything other than vxml. Avoid mixing data for the two kinds of CLDRFile in
* caches (such as rLocales).
*
* @param loc the CLDRLocale
* @return the vetted CLDRFile with more paths resolved
*/
public CLDRFile makeVettedFile(CLDRLocale loc) {
PerLocaleData pld = get(loc.getBaseName());
XMLSource xmlSource = pld.makeVettedSource();
CLDRFile cldrFile = new CLDRFile(xmlSource);
cldrFile.setSupplementalDirectory(getSupplementalDirectory());
return cldrFile;
}

/**
* Prepare statement. Args: locale Result: xpath,submitter,value
*
Expand Down

0 comments on commit 7c82449

Please sign in to comment.