Fix internal errors when 'destination:' refers to non-existing destination
When querying with the destination: predicate using a destination name that does not exist, it is supposed to return an error: "Unknown named destination" However it was not, because of two problems: - When the revision is null, VersionedAccountDestinations's call to getPathInfos results in NPE due to dereferencing the revision. - When no destinations exist, DestinationList.getDestinations returns an empty list, but the ChangeQueryBuilder is only testing for null Fix both of the bugs and add a test that querying "destination:" with a non-existing destination results in the correct response. Adding tests for existing destinations is not within the scope of this change. Change-Id: I7787a90a508f61ef19586f327c6be427a0870e69
This commit is contained in:
		@@ -53,6 +53,9 @@ public class VersionedAccountDestinations extends VersionedMetaData {
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
  @Override
 | 
					  @Override
 | 
				
			||||||
  protected void onLoad() throws IOException, ConfigInvalidException {
 | 
					  protected void onLoad() throws IOException, ConfigInvalidException {
 | 
				
			||||||
 | 
					    if (revision == null) {
 | 
				
			||||||
 | 
					      return;
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
    String prefix = DestinationList.DIR_NAME + "/";
 | 
					    String prefix = DestinationList.DIR_NAME + "/";
 | 
				
			||||||
    for (PathInfo p : getPathInfos(true)) {
 | 
					    for (PathInfo p : getPathInfos(true)) {
 | 
				
			||||||
      if (p.fileMode == FileMode.REGULAR_FILE) {
 | 
					      if (p.fileMode == FileMode.REGULAR_FILE) {
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -1047,7 +1047,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
 | 
				
			|||||||
      VersionedAccountDestinations d = VersionedAccountDestinations.forUser(self());
 | 
					      VersionedAccountDestinations d = VersionedAccountDestinations.forUser(self());
 | 
				
			||||||
      d.load(git);
 | 
					      d.load(git);
 | 
				
			||||||
      Set<Branch.NameKey> destinations = d.getDestinationList().getDestinations(name);
 | 
					      Set<Branch.NameKey> destinations = d.getDestinationList().getDestinations(name);
 | 
				
			||||||
      if (destinations != null) {
 | 
					      if (destinations != null && !destinations.isEmpty()) {
 | 
				
			||||||
        return new DestinationPredicate(destinations, name);
 | 
					        return new DestinationPredicate(destinations, name);
 | 
				
			||||||
      }
 | 
					      }
 | 
				
			||||||
    } catch (RepositoryNotFoundException e) {
 | 
					    } catch (RepositoryNotFoundException e) {
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -2105,6 +2105,16 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
 | 
				
			|||||||
    assertQuery("-assignee:" + user.getUserName(), change2);
 | 
					    assertQuery("-assignee:" + user.getUserName(), change2);
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  @Test
 | 
				
			||||||
 | 
					  public void userDestination() throws Exception {
 | 
				
			||||||
 | 
					    TestRepository<Repo> repo = createProject("repo");
 | 
				
			||||||
 | 
					    insert(repo, newChange(repo));
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    assertThatQueryException("destination:foo")
 | 
				
			||||||
 | 
					        .hasMessageThat()
 | 
				
			||||||
 | 
					        .isEqualTo("Unknown named destination: foo");
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
 | 
					  protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
 | 
				
			||||||
    return newChange(repo, null, null, null, null);
 | 
					    return newChange(repo, null, null, null, null);
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user