adding comment about priority ordering and updating test for ListTargets with delegates to hit default no roles passed case

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

client/client.go

@@ -405,6 +405,11 @@ func (r *NotaryRepository) RemoveTarget(targetName string, roles ...string) erro
 
 // ListTargets lists all targets for the current repository. The list of
 // roles should be passed in order from highest to lowest priority.
+// IMPORTANT: if you pass a set of roles such as [ "targets/a", "targets/x"
+// "targets/a/b" ], even though "targets/a/b" is part of the "targets/a" subtree
+// its entries will be strictly shadowed by those in other parts of the "targets/a"
+// subtree and also the "targets/x" subtree, as we will defer parsing it until
+// we explicitly reach it in our iteration of the provided list of roles.
 func (r *NotaryRepository) ListTargets(roles ...string) ([]*Target, error) {
 	c, err := r.bootstrapClient()
 	if err != nil {
@@ -471,6 +476,7 @@ func (r *NotaryRepository) listSubtree(targets map[string]*Target, role string,
 // If roles are passed, they should be passed in descending priority and
 // the target entry found in the subtree of the highest priority role
 // will be returned
+// See the IMPORTANT section on ListTargets above. Those roles also apply here.
 func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*Target, error) {
 	c, err := r.bootstrapClient()
 	if err != nil {

client/client_test.go

@@ -943,7 +943,7 @@ func testListTargetWithDelegates(t *testing.T, rootType string) {
 	assert.NoError(t, err, "error creating repository: %s", err)
 
 	latestTarget := addTarget(t, repo, "latest", "../fixtures/intermediate-ca.crt")
-	addTarget(t, repo, "current", "../fixtures/intermediate-ca.crt")
+	currentTarget := addTarget(t, repo, "current", "../fixtures/intermediate-ca.crt")
 
 	// setup delegated targets/level1 role
 	k, err := repo.CryptoService.Create("targets/level1", rootType)
@@ -985,7 +985,23 @@ func testListTargetWithDelegates(t *testing.T, rootType string) {
 
 	fakeServerData(t, repo, mux, keys)
 
-	targets, err := repo.ListTargets("targets/level1", data.CanonicalTargetsRole)
+	// test default listing
+	targets, err := repo.ListTargets()
+	assert.NoError(t, err)
+
+	// Should be two targets
+	assert.Len(t, targets, 4, "unexpected number of targets returned by ListTargets")
+
+	sort.Stable(targetSorter(targets))
+
+	// current should be first.
+	assert.Equal(t, currentTarget, targets[0], "current target does not match")
+	assert.Equal(t, latestTarget, targets[1], "latest target does not match")
+	assert.Equal(t, level2Target, targets[2], "level2 target does not match")
+	assert.Equal(t, otherTarget, targets[3], "other target does not match")
+
+	// test listing with priority specified
+	targets, err = repo.ListTargets("targets/level1", data.CanonicalTargetsRole)
 	assert.NoError(t, err)
 
 	// Should be two targets