Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-7214] FilePath.validateAntFileMask too slow for /confi…
…gure.(cherry picked from commit a885cc2)

Conflicts:
	changelog.html
	core/src/test/java/hudson/FilePathTest.java
  • Loading branch information
jglick authored and vjuranek committed Oct 23, 2012
1 parent 760859d commit 53d25ff
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 11 deletions.
3 changes: 3 additions & 0 deletions changelog.html
Expand Up @@ -55,6 +55,9 @@
<!-- Record your changes in the trunk here. -->
<div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
<ul class=image>
<li class=bug>
<code>FilePath.validateAntFileMask</code> too slow for <code>/configure</code>.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-7214">issue 7214</a>)
<li class=>
</ul>
</div><!--=TRUNK-END=-->
Expand Down
52 changes: 44 additions & 8 deletions core/src/main/java/hudson/FilePath.java
Expand Up @@ -830,7 +830,7 @@ private <T> T act(final FileCallable<T> callable, ClassLoader cl) throws IOExcep
try {
return channel.call(new FileCallableWrapper<T>(callable,cl));
} catch (TunneledInterruptedException e) {
throw (InterruptedException)new InterruptedException().initCause(e);
throw (InterruptedException)new InterruptedException(e.getMessage()).initCause(e);
} catch (AbortException e) {
throw e; // pass through so that the caller can catch it as AbortException
} catch (IOException e) {
Expand Down Expand Up @@ -1903,8 +1903,26 @@ public Boolean call() throws IOException {
* @see #validateFileMask(FilePath, String)
*/
public String validateAntFileMask(final String fileMasks) throws IOException, InterruptedException {
return validateAntFileMask(fileMasks, Integer.MAX_VALUE);
}

/**
* Like {@link #validateAntFileMask(String)} but performing only a bounded number of operations.
* <p>Whereas the unbounded overload is appropriate for calling from cancelable, long-running tasks such as build steps,
* this overload should be used when an answer is needed quickly, such as for {@link #validateFileMask(String)}
* or anything else returning {@link FormValidation}.
* <p>If a positive match is found, {@code null} is returned immediately.
* A message is returned in case the file pattern can definitely be determined to not match anything in the directory within the alloted time.
* If the time runs out without finding a match but without ruling out the possibility that there might be one, {@link InterruptedException} is thrown,
* in which case the calling code should give the user the benefit of the doubt and use {@link hudson.util.FormValidation.Kind#OK} (with or without a message).
* @param bound a maximum number of negative operations (deliberately left vague) to perform before giving up on a precise answer; 10_000 is a reasonable pick
* @throws InterruptedException not only in case of a channel failure, but also if too many operations were performed without finding any matches
* @since 1.484
*/
public String validateAntFileMask(final String fileMasks, final int bound) throws IOException, InterruptedException {
return act(new FileCallable<String>() {
public String invoke(File dir, VirtualChannel channel) throws IOException {
private static final long serialVersionUID = 1;
public String invoke(File dir, VirtualChannel channel) throws IOException, InterruptedException {
if(fileMasks.startsWith("~"))
return Messages.FilePath_TildaDoesntWork();

Expand Down Expand Up @@ -2001,10 +2019,28 @@ public String invoke(File dir, VirtualChannel channel) throws IOException {
return null; // no error
}

private boolean hasMatch(File dir, String pattern) {
FileSet fs = Util.createFileSet(dir,pattern);
DirectoryScanner ds = fs.getDirectoryScanner(new Project());

private boolean hasMatch(File dir, String pattern) throws InterruptedException {
class Cancel extends RuntimeException {}
DirectoryScanner ds = bound == Integer.MAX_VALUE ? new DirectoryScanner() : new DirectoryScanner() {
int ticks;
@Override public synchronized boolean isCaseSensitive() {
if (!filesIncluded.isEmpty() || !dirsIncluded.isEmpty() || ticks++ > bound) {
throw new Cancel();
}
return super.isCaseSensitive();
}
};
ds.setBasedir(dir);
ds.setIncludes(new String[] {pattern});
try {
ds.scan();
} catch (Cancel c) {
if (ds.getIncludedFilesCount()!=0 || ds.getIncludedDirsCount()!=0) {
return true;
} else {
throw new InterruptedException("no matches found within " + bound);
}
}
return ds.getIncludedFilesCount()!=0 || ds.getIncludedDirsCount()!=0;
}

Expand Down Expand Up @@ -2053,11 +2089,11 @@ public FormValidation validateFileMask(String value, boolean errorIfNotExist) th
if(!exists()) // no workspace. can't check
return FormValidation.ok();

String msg = validateAntFileMask(value);
String msg = validateAntFileMask(value, 10000);
if(errorIfNotExist) return FormValidation.error(msg);
else return FormValidation.warning(msg);
} catch (InterruptedException e) {
return FormValidation.ok();
return FormValidation.ok(Messages.FilePath_did_not_manage_to_validate_may_be_too_sl(value));
}
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/util/FormFieldValidator.java
Expand Up @@ -378,11 +378,11 @@ protected void check() throws IOException, ServletException {
return;
}

String msg = ws.validateAntFileMask(value);
String msg = ws.validateAntFileMask(value, 10000);
if(errorIfNotExist) error(msg);
else warning(msg);
} catch (InterruptedException e) {
ok(); // coundn't check
ok(Messages.FormFieldValidator_did_not_manage_to_validate_may_be_too_sl(value));
}
}

Expand Down
1 change: 1 addition & 0 deletions core/src/main/resources/hudson/Messages.properties
Expand Up @@ -20,6 +20,7 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

FilePath.did_not_manage_to_validate_may_be_too_sl=Did not manage to validate {0} (may be too slow)
FilePath.validateAntFileMask.whitespaceSeprator=\
Whitespace can no longer be used as the separator. Please Use '','' as the separator instead.
FilePath.validateAntFileMask.doesntMatchAndSuggest=\
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/resources/hudson/util/Messages.properties
Expand Up @@ -24,6 +24,7 @@ ClockDifference.InSync=In sync
ClockDifference.Ahead=\ ahead
ClockDifference.Behind=\ behind
ClockDifference.Failed=Failed to check
FormFieldValidator.did_not_manage_to_validate_may_be_too_sl=Did not manage to validate {0} (may be too slow)
FormValidation.ValidateRequired=Required
FormValidation.Error.Details=(show details)
HttpResponses.Saved=Saved
HttpResponses.Saved=Saved
51 changes: 51 additions & 0 deletions core/src/test/java/hudson/FilePathTest.java
Expand Up @@ -424,4 +424,55 @@ public void testMultiSegmentRelativePaths() throws Exception {
assertEquals("/opt/jenkins/workspace/foo/bar/manchu", new FilePath(nixPath, "foo/bar\\manchu").getRemote());
assertEquals("/opt/jenkins/workspace/foo/bar/manchu", new FilePath(nixPath, "foo/bar/manchu").getRemote());
}

public void testValidateAntFileMask() throws Exception {
File tmp = Util.createTempDir();
try {
FilePath d = new FilePath(french, tmp.getPath());
d.child("d1/d2/d3").mkdirs();
d.child("d1/d2/d3/f.txt").touch(0);
d.child("d1/d2/d3/f.html").touch(0);
d.child("d1/d2/f.txt").touch(0);
assertValidateAntFileMask(null, d, "**/*.txt");
assertValidateAntFileMask(null, d, "d1/d2/d3/f.txt");
assertValidateAntFileMask(null, d, "**/*.html");
assertValidateAntFileMask(Messages.FilePath_validateAntFileMask_portionMatchButPreviousNotMatchAndSuggest("**/*.js", "**", "**/*.js"), d, "**/*.js");
assertValidateAntFileMask(Messages.FilePath_validateAntFileMask_doesntMatchAnything("index.htm"), d, "index.htm");
assertValidateAntFileMask(Messages.FilePath_validateAntFileMask_doesntMatchAndSuggest("f.html", "d1/d2/d3/f.html"), d, "f.html");
// XXX lots more to test, e.g. multiple patterns separated by commas; ought to have full code coverage for this method
} finally {
Util.deleteRecursive(tmp);
}
}

private static void assertValidateAntFileMask(String expected, FilePath d, String fileMasks) throws Exception {
assertEquals(expected, d.validateAntFileMask(fileMasks));
}

@Bug(7214)
public void testValidateAntFileMaskBounded() throws Exception {
File tmp = Util.createTempDir();
try {
FilePath d = new FilePath(french, tmp.getPath());
FilePath d2 = d.child("d1/d2");
d2.mkdirs();
for (int i = 0; i < 100; i++) {
FilePath d3 = d2.child("d" + i);
d3.mkdirs();
d3.child("f.txt").touch(0);
}
assertEquals(null, d.validateAntFileMask("d1/d2/**/f.txt"));
assertEquals(null, d.validateAntFileMask("d1/d2/**/f.txt", 10));
assertEquals(Messages.FilePath_validateAntFileMask_portionMatchButPreviousNotMatchAndSuggest("**/*.js", "**", "**/*.js"), d.validateAntFileMask("**/*.js", 1000));
try {
d.validateAntFileMask("**/*.js", 10);
fail();
} catch (InterruptedException x) {
// good
}
} finally {
Util.deleteRecursive(tmp);
}
}

}

0 comments on commit 53d25ff

Please sign in to comment.