ACL: Fix transitions check & a '1' in RefName
In determining which access pattern has precedence, the 1st check is which pattern is closer via Levenshtein distance. The 2nd criteria is if they differ on finite vs inifite. The 3rd criteria is based on number of transitions in the pattern, where it is supposed to be the pattern with the greatest number of transitions having precedence. The check of using the number of transitions had the check reversed. This caused the pattern with the fewest number of transitions to have precedence. When determining the Levenshtein distance of a glob style pattern the trailing '*' would be changed to a '1'. The issue with doing this is that it could cause a different access pattern to take precedence if a RefName contained a '1' character as that would cause the Levenshtein distance to become decreased by one. Change-Id: Ibdcd483c303d24565ef43001b3831f6463c2ed8f Signed-off-by: John L. Villalovos <john.l.villalovos@intel.com>
This commit is contained in:

committed by
David Pursehouse

parent
a4e1af79bd
commit
b7da2a97d4
@@ -28,7 +28,7 @@ import java.util.Comparator;
|
||||
* name and the regex string shortest example. A shorter distance is a more
|
||||
* specific match.
|
||||
* <li>2 - Finites first, infinities after.
|
||||
* <li>3 - Number of transitions.
|
||||
* <li>3 - Number of transitions. More transitions is more specific.
|
||||
* <li>4 - Length of the expression text.
|
||||
* </ul>
|
||||
*
|
||||
@@ -72,7 +72,7 @@ public final class MostSpecificComparator implements
|
||||
}
|
||||
}
|
||||
if (cmp == 0) {
|
||||
cmp = transitions(pattern1) - transitions(pattern2);
|
||||
cmp = transitions(pattern2) - transitions(pattern1);
|
||||
}
|
||||
if (cmp == 0) {
|
||||
cmp = pattern2.length() - pattern1.length();
|
||||
@@ -86,7 +86,7 @@ public final class MostSpecificComparator implements
|
||||
example = RefControl.shortestExample(pattern);
|
||||
|
||||
} else if (pattern.endsWith("/*")) {
|
||||
example = pattern.substring(0, pattern.length() - 1) + '1';
|
||||
example = pattern;
|
||||
|
||||
} else if (pattern.equals(refName)) {
|
||||
return 0;
|
||||
|
@@ -0,0 +1,80 @@
|
||||
// Copyright (C) 2014 The Android Open Source Project
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package com.google.gerrit.server.util;
|
||||
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
public class MostSpecificComparatorTest {
|
||||
|
||||
private MostSpecificComparator cmp;
|
||||
|
||||
@Test
|
||||
public void shorterDistanceWins() {
|
||||
cmp = new MostSpecificComparator("refs/heads/master");
|
||||
moreSpecificFirst("refs/heads/master", "refs/heads/master2");
|
||||
moreSpecificFirst("refs/heads/master", "refs/heads/maste");
|
||||
moreSpecificFirst("refs/heads/master", "refs/heads/*");
|
||||
moreSpecificFirst("refs/heads/master", "^refs/heads/.*");
|
||||
moreSpecificFirst("refs/heads/master", "^refs/heads/master.*");
|
||||
}
|
||||
|
||||
/**
|
||||
* Assuming two patterns have the same Levenshtein distance,
|
||||
* the pattern which represents a finite language wins over a pattern
|
||||
* which represents an infinite language.
|
||||
*/
|
||||
@Test
|
||||
public void finiteWinsOverInfinite() {
|
||||
cmp = new MostSpecificComparator("refs/heads/master");
|
||||
moreSpecificFirst("^refs/heads/......", "refs/heads/*");
|
||||
moreSpecificFirst("^refs/heads/maste.", "^refs/heads/maste.*");
|
||||
}
|
||||
|
||||
/**
|
||||
* Assuming two patterns have the same Levenshtein distance
|
||||
* and are both either finite or infinite the one with the higher
|
||||
* number of state transitions (in an equivalent automaton) wins
|
||||
*/
|
||||
@Test
|
||||
public void higherNumberOfTransitionsWins() {
|
||||
cmp = new MostSpecificComparator("refs/heads/x");
|
||||
moreSpecificFirst("^refs/heads/[a-z].*", "refs/heads/*");
|
||||
// Previously there was a bug where having a '1' in a refname would cause a
|
||||
// glob pattern's Levenshtein distance to decrease by 1. These two
|
||||
// patterns should be a Levenshtein distance of 12 from the both of the
|
||||
// refnames, where previously the 'branch1' refname would be a distance of
|
||||
// 11 from 'refs/heads/abc/*'
|
||||
cmp = new MostSpecificComparator("refs/heads/abc/spam/branch2");
|
||||
moreSpecificFirst("^refs/heads/.*spam.*", "refs/heads/abc/*");
|
||||
cmp = new MostSpecificComparator("refs/heads/abc/spam/branch1");
|
||||
moreSpecificFirst("^refs/heads/.*spam.*", "refs/heads/abc/*");
|
||||
}
|
||||
|
||||
/**
|
||||
* Assuming the same Levenshtein distance, (in)finity and the number
|
||||
* of transitions, the longer pattern wins
|
||||
*/
|
||||
@Test
|
||||
public void longerPatternWins() {
|
||||
cmp = new MostSpecificComparator("refs/heads/x");
|
||||
moreSpecificFirst("^refs/heads/[a-z].*", "^refs/heads/..*");
|
||||
}
|
||||
|
||||
private void moreSpecificFirst(String first, String second) {
|
||||
assertTrue(cmp.compare(first, second) < 0);
|
||||
}
|
||||
}
|
Reference in New Issue
Block a user