feat: improve fetch (#160)

* feat: improve fetch

* chore: fetch should use hash

Author: matt-FFFFFF

alzlib.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"slices"
-	"strconv"
 	"strings"
 	"sync"
 
@@ -385,12 +384,12 @@ func (az *AlzLib) Init(ctx context.Context, libs ...LibraryReference) error {
 	}
 
 	// Process the libraries
-	for i, ref := range libs {
+	for _, ref := range libs {
 		if ref == nil {
 			return errors.New("Alzlib.Init: library is nil")
 		}
 		if ref.FS() == nil {
-			if _, err := ref.Fetch(ctx, strconv.Itoa(i)); err != nil {
+			if _, err := ref.Fetch(ctx, hash(ref)); err != nil {
 				return fmt.Errorf("Alzlib.Init: error fetching library %s: %w", ref, err)
 			}
 		}

cmd/alzlibtool/command/check/library.go

@@ -32,7 +32,7 @@ var libraryCmd = cobra.Command{
 		}
 		az.AddPolicyClient(cf)
 		thisRef := alzlib.NewCustomLibraryReference(args[0])
-		libs, err := alzlib.FetchLibraryWithDependencies(cmd.Context(), 0, thisRef, make(alzlib.LibraryReferences, 0, 5))
+		libs, err := thisRef.FetchWithDependencies(cmd.Context())
 		if err != nil {
 			cmd.PrintErrf("%s could not fetch all libraries with dependencies: %v\n", cmd.ErrPrefix(), err)
 		}

cmd/alzlibtool/command/document/library.go

@@ -15,7 +15,7 @@ var documentLibraryBaseCmd = cobra.Command{
 	Args:  cobra.ExactArgs(1),
 	Run: func(cmd *cobra.Command, args []string) {
 		thislib := alzlib.NewCustomLibraryReference(args[0])
-		alllibs, err := alzlib.FetchLibraryWithDependencies(cmd.Context(), 0, thislib, make(alzlib.LibraryReferences, 0, 5))
+		alllibs, err := thislib.FetchWithDependencies(cmd.Context())
 		if err != nil {
 			cmd.PrintErrf("%s could not fetch all libraries with dependencies: %v\n", cmd.ErrPrefix(), err)
 		}

initialize.go

@@ -5,13 +5,12 @@ package alzlib
 
 import (
 	"context"
+	"crypto/sha256"
 	"fmt"
 	"io/fs"
 	"net/url"
 	"os"
 	"path/filepath"
-	"slices"
-	"strconv"
 
 	"github.com/Azure/alzlib/internal/processor"
 	"github.com/hashicorp/go-getter/v2"
@@ -24,46 +23,45 @@ const (
 	alzLibraryGitUrlEnv    = "ALZLIB_LIBRARY_GIT_URL"                       // alzLibraryGitUrlEnv is the environment variable to override the default git URL.
 )
 
-// FetchLibraryWithDependencies takes a library reference, fetches it, and then fetches all of its dependencies.
+// fetchLibraryWithDependencies takes a library reference, fetches it, and then fetches all of its dependencies.
 // The destination directory is an integer that will be appended to the `.alzlib` directory in the current working directory.
 // This can be override by setting the `ALZLIB_DIR` environment variable.
-//
-// Example usage:
-//
-// ```go
-// az := alzlib.NewAlzLib(nil)
-// // ... ensure that clients are created and initialized
-// // e.g. az.AddPolicyClient(myClientFactory)
-// thisLib := NewCustomLibraryReference("path/to/library")
-// libs, err := FetchLibraryWithDependencies(ctx, ".alzlib", 0, thisLib, make(LibraryReferences, 0, 5))
-// // ... handle error
-//
-// err = az.Init(ctx, libs...)
-// // ... handle error
-// ```
-//
 // The `LibraryReferences` slice can be used to initialize the AlzLib instance.
-func FetchLibraryWithDependencies(ctx context.Context, i int, lib LibraryReference, libs LibraryReferences) (LibraryReferences, error) {
-	f, err := lib.Fetch(ctx, strconv.Itoa(i))
+func fetchLibraryWithDependencies(ctx context.Context, processed map[string]bool, lib LibraryReference, result *LibraryReferences) error {
+	if processed[lib.String()] {
+		return nil
+	}
+	f, err := lib.Fetch(ctx, hash(lib))
 	if err != nil {
-		return nil, fmt.Errorf("FetchLibraryWithDependencies: error fetching library %s: %w", lib.String(), err)
+		return fmt.Errorf("FetchLibraryWithDependencies: error fetching library %s: %w", lib.String(), err)
 	}
 	pscl := processor.NewProcessorClient(f)
 	libmeta, err := pscl.Metadata()
 	if err != nil {
-		return nil, fmt.Errorf("FetchLibraryWithDependencies: error getting metadata for library %s: %w", lib.String(), err)
+		return fmt.Errorf("FetchLibraryWithDependencies: error getting metadata for library %s: %w", lib.String(), err)
 	}
 	meta := NewMetadata(libmeta, lib)
 	// for each dependency, recurse using this function
 	for _, dep := range meta.Dependencies() {
-		i++
-		libs, err = FetchLibraryWithDependencies(ctx, i, dep, libs)
+		err = fetchLibraryWithDependencies(ctx, processed, dep, result)
 		if err != nil {
-			return nil, fmt.Errorf("FetchLibraryWithDependencies: error fetching dependencies for library %s: %w", lib.String(), err)
+			return fmt.Errorf("FetchLibraryWithDependencies: error fetching dependencies for library %s: %w", lib.String(), err)
 		}
 	}
 	// add the current library reference to the list
-	return addLibraryReferenceToSlice(libs, lib), nil
+	*result = append(*result, lib)
+	processed[lib.String()] = true
+	return nil
+}
+
+// hash returns the SHA224 hash of a fmt.Stringer, as a string.
+func hash(s fmt.Stringer) string {
+	return hashStr(s.String())
+}
+
+// hash returns the SHA224 hash of a string, as a string.
+func hashStr(s string) string {
+	return fmt.Sprintf("%x", sha256.Sum224([]byte(s)))
 }
 
 // FetchAzureLandingZonesLibraryByTag is a convenience function to fetch the Azure Landing Zones library by member path and tag (ref).
@@ -115,14 +113,3 @@ func FetchLibraryByGetterString(ctx context.Context, getterString, dstDir string
 	}
 	return os.DirFS(dst), nil
 }
-
-// addLibraryReferenceToSlice adds a library reference to a slice if it does not already exist.
-func addLibraryReferenceToSlice(libs LibraryReferences, lib LibraryReference) LibraryReferences {
-	if exists := slices.ContainsFunc(libs, func(l LibraryReference) bool {
-		return l.String() == lib.String()
-	}); exists {
-		return libs
-	}
-
-	return append(libs, lib)
-}

initialize_test.go

@@ -28,7 +28,7 @@ func TestFetchLibraryWithDependencies(t *testing.T) {
 	require.NoError(t, os.RemoveAll(".alzlib"))
 	defer os.RemoveAll(".alzlib") // nolint: errcheck
 
-	libs, err := FetchLibraryWithDependencies(ctx, 0, NewCustomLibraryReference("./testdata/dependent-libs/lib1"), make(LibraryReferences, 0, 2))
+	libs, err := NewCustomLibraryReference("./testdata/dependent-libs/lib1").FetchWithDependencies(ctx)
 	assert.NoError(t, err)
 	assert.Len(t, libs, 2)
 }
@@ -38,7 +38,7 @@ func TestFetchLibraryWithDependencies_MissingCustomDependency(t *testing.T) {
 	require.NoError(t, os.RemoveAll(".alzlib"))
 	defer os.RemoveAll(".alzlib") // nolint: errcheck
 
-	_, err := FetchLibraryWithDependencies(ctx, 0, NewCustomLibraryReference("./testdata/dependent-libs/missing-dep-custom"), make(LibraryReferences, 0, 2))
+	_, err := NewCustomLibraryReference("./testdata/dependent-libs/missing-dep-custom").FetchWithDependencies(ctx)
 	assert.ErrorContains(t, err, "could not fetch library member")
 }
 
@@ -47,6 +47,6 @@ func TestFetchLibraryWithDependencies_MissingLibraryDependency(t *testing.T) {
 	require.NoError(t, os.RemoveAll(".alzlib"))
 	defer os.RemoveAll(".alzlib") // nolint: errcheck
 
-	_, err := FetchLibraryWithDependencies(ctx, 0, NewCustomLibraryReference("./testdata/dependent-libs/missing-dep-library"), make(LibraryReferences, 0, 2))
+	_, err := NewCustomLibraryReference("./testdata/dependent-libs/missing-dep-library").FetchWithDependencies(ctx)
 	assert.ErrorContains(t, err, "could not fetch library member")
 }

integrationtest/examples_test.go

@@ -32,7 +32,7 @@ func Example_deploymentNewHierarchy() {
 	az.AddPolicyClient(cf)
 	//dirFs, err := alzlib.FetchAzureLandingZonesLibraryMember(ctx, alzLibraryMember, alzLibraryTag, "alz")
 	lib := alzlib.NewCustomLibraryReference("testdata/alzlib-2024-07-01")
-	libs, err := alzlib.FetchLibraryWithDependencies(ctx, 0, lib, make(alzlib.LibraryReferences, 0, 5))
+	libs, err := lib.FetchWithDependencies(ctx)
 	if err != nil {
 		fmt.Println(err)
 		return

internal/doc/doc.go

@@ -36,8 +36,20 @@ func AlzlibReadmeMd(ctx context.Context, w io.Writer, libs ...alzlib.LibraryRefe
 }
 
 func alzlibReadmeMdTitle(md *markdown.Markdown, metad *alzlib.Metadata) *markdown.Markdown {
-	return md.H1f("%s (%s)", metad.Name(), metad.DisplayName()).LF().
-		PlainText(metad.Description()).LF()
+	name := metad.Name()
+	if name == "" {
+		name = "No name in metadata"
+	}
+	displayName := metad.DisplayName()
+	if displayName == "" {
+		displayName = "No display name in metadata"
+	}
+	description := metad.Description()
+	if description == "" {
+		description = "No description in metadata"
+	}
+	return md.H1f("%s (%s)", name, displayName).LF().
+		PlainText(description).LF()
 }
 
 func alzlibReadmeMdDependencies(md *markdown.Markdown, deps alzlib.LibraryReferences) *markdown.Markdown {

internal/processor/processor.go

@@ -93,10 +93,10 @@ func (client *ProcessorClient) Metadata() (*LibMetadata, error) {
 	var pe *fs.PathError
 	if errors.As(err, &pe) {
 		return &LibMetadata{
-			Name:         "No metadata file found",
-			DisplayName:  "No metadata file found",
-			Description:  "No metadata file found",
-			Path:         "No metadata file found",
+			Name:         "",
+			DisplayName:  "",
+			Description:  "",
+			Path:         "",
 			Dependencies: make([]LibMetadataDependency, 0),
 		}, nil
 	}
@@ -114,18 +114,6 @@ func (client *ProcessorClient) Metadata() (*LibMetadata, error) {
 	if err != nil {
 		return nil, fmt.Errorf("ProcessorClient.Metadata: error unmarshaling metadata: %w", err)
 	}
-	if metadata.Description == "" {
-		metadata.Description = "No description provided"
-	}
-	if metadata.DisplayName == "" {
-		metadata.DisplayName = "No display name provided"
-	}
-	if metadata.Path == "" {
-		metadata.Path = "No path provided"
-	}
-	if metadata.Name == "" {
-		metadata.Name = "No name provided"
-	}
 	for _, dep := range metadata.Dependencies {
 		switch {
 		case dep.Path != "" && dep.Ref != "" && dep.CustomUrl == "":

metadata.go

@@ -27,19 +27,34 @@ type Metadata struct {
 type LibraryReferences []LibraryReference
 
 // FSs returns the filesystems of the library references, can be used with Alzlib.Init().
-func (lr LibraryReferences) FSs() []fs.FS {
-	fss := make([]fs.FS, len(lr))
-	for i, l := range lr {
+func (m LibraryReferences) FSs() []fs.FS {
+	fss := make([]fs.FS, len(m))
+	for i, l := range m {
 		fss[i] = l.FS()
 	}
 	return fss
 }
 
+// FetchWithDependencies recursively fetches all the library references and their dependencies.
+// The destination directory a hash value that will be appended to the `.alzlib` directory in the current working directory unless overridden by the `ALZLIB_DIR` environment variable.
+func (m LibraryReferences) FetchWithDependencies(ctx context.Context) (LibraryReferences, error) {
+	processed := make(map[string]bool)
+	result := make(LibraryReferences, 0, 5)
+	for _, lib := range m {
+		err := fetchLibraryWithDependencies(ctx, processed, lib, &result)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return result, nil
+}
+
 // LibraryReference is an interface that represents a dependency of a library member.
 // It can be fetched form either a custom go-getter URL or from the ALZ Library.
 type LibraryReference interface {
 	fmt.Stringer
 	Fetch(ctx context.Context, desinationDirectory string) (fs.FS, error) // Fetch fetches the library member to the `.alzlib/destinationDirectory`. Override the base dir using `ALZLIB_DIR` env var.
+	FetchWithDependencies(ctx context.Context) (LibraryReferences, error) // FetchWithDependencies fetches the library member and its dependencies.
 	FS() fs.FS                                                            // FS returns the filesystem of the library member, can be used in Alzlib.Init()
 }
 
@@ -92,6 +107,14 @@ func (m *AlzLibraryReference) Ref() string {
 	return m.ref
 }
 
+// FetchWithDependencies fetches the library member and its dependencies.
+// If you have more than one LibraryReference in a LibraryReferences slice, use LibraryReferences.FetchWithDependencies() instead.
+func (m *AlzLibraryReference) FetchWithDependencies(ctx context.Context) (LibraryReferences, error) {
+	processed := make(map[string]bool)
+	result := make(LibraryReferences, 0, 5)
+	return result, fetchLibraryWithDependencies(ctx, processed, m, &result)
+}
+
 // CustomLibraryReference is a struct that represents a dependency of a library member that is fetched from a custom go-getter URL.
 type CustomLibraryReference struct {
 	url        string
@@ -128,6 +151,14 @@ func (m *CustomLibraryReference) String() string {
 	return m.url
 }
 
+// FetchWithDependencies fetches the library member and its dependencies.
+// If you have more than one LibraryReference in a LibraryReferences slice, use LibraryReferences.FetchWithDependencies() instead.
+func (m *CustomLibraryReference) FetchWithDependencies(ctx context.Context) (LibraryReferences, error) {
+	processed := make(map[string]bool)
+	result := make(LibraryReferences, 0, 5)
+	return result, fetchLibraryWithDependencies(ctx, processed, m, &result)
+}
+
 func NewMetadata(in *processor.LibMetadata, ref LibraryReference) *Metadata {
 	dependencies := make([]LibraryReference, len(in.Dependencies))
 	for i, dep := range in.Dependencies {

metadata_test.go

@@ -0,0 +1,57 @@
+package alzlib
+
+import (
+	"context"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestFetchLibrariesWithDependencies tests fetching libraries with dependencies and they they are fetched in the right order.
+func TestFetchLibrariesWithDependencies(t *testing.T) {
+	ctx := context.Background()
+	require.NoError(t, os.RemoveAll(".alzlib"))
+	defer os.RemoveAll(".alzlib") // nolint: errcheck
+	expcted := []string{
+		"testdata/dependent-libs/lib2",
+		"testdata/dependent-libs/lib1",
+		"testdata/dependent-libs/libB",
+		"testdata/dependent-libs/libA",
+	}
+	lib1 := NewCustomLibraryReference("testdata/dependent-libs/lib1")
+	libA := NewCustomLibraryReference("testdata/dependent-libs/libA")
+	libs := LibraryReferences{lib1, libA}
+	libs, err := libs.FetchWithDependencies(ctx)
+	assert.NoError(t, err)
+	require.Len(t, libs, 4)
+	result := make([]string, 4)
+	for i, lib := range libs {
+		result[i] = lib.String()
+	}
+	assert.ElementsMatch(t, expcted, result)
+}
+
+// TestFetchLibrariesWithCommonDependency checks that a libraries having a common dependency is fetched only once.
+func TestFetchLibrariesWithCommonDependency(t *testing.T) {
+	ctx := context.Background()
+	require.NoError(t, os.RemoveAll(".alzlib"))
+	defer os.RemoveAll(".alzlib") // nolint: errcheck
+	expcted := []string{
+		"testdata/dependent-libs/lib2",
+		"testdata/dependent-libs/lib1",
+		"testdata/dependent-libs/lib3",
+	}
+	lib1 := NewCustomLibraryReference("testdata/dependent-libs/lib1")
+	libA := NewCustomLibraryReference("testdata/dependent-libs/lib3")
+	libs := LibraryReferences{lib1, libA}
+	libs, err := libs.FetchWithDependencies(ctx)
+	assert.NoError(t, err)
+	require.Len(t, libs, 3)
+	result := make([]string, 3)
+	for i, lib := range libs {
+		result[i] = lib.String()
+	}
+	assert.ElementsMatch(t, expcted, result)
+}

testdata/dependent-libs/lib3/alz_library_metadata.json

@@ -0,0 +1,11 @@
+{
+  "$schema": "https://raw.githubusercontent.com/Azure/Azure-Landing-Zones-Library/main/schemas/library_metadata.json",
+  "name": "lib3",
+  "display_name": "library 3",
+  "description": "Like lib1, lib 3 is dependent on lib 2",
+  "dependencies": [
+    {
+      "custom_url": "testdata/dependent-libs/lib2"
+    }
+  ]
+}

testdata/dependent-libs/libA/alz_library_metadata.json

@@ -0,0 +1,11 @@
+{
+  "$schema": "https://raw.githubusercontent.com/Azure/Azure-Landing-Zones-Library/main/schemas/library_metadata.json",
+  "name": "libA",
+  "display_name": "library A",
+  "description": "libA is dependent on libB",
+  "dependencies": [
+    {
+      "custom_url": "testdata/dependent-libs/libB"
+    }
+  ]
+}

testdata/dependent-libs/libB/alz_library_metadata.json

@@ -0,0 +1,6 @@
+{
+  "$schema": "https://raw.githubusercontent.com/Azure/Azure-Landing-Zones-Library/main/schemas/library_metadata.json",
+  "name": "libB",
+  "display_name": "library B",
+  "description": "library B description"
+}

to/doc.go

@@ -1,6 +1,3 @@
-//go:build go1.18
-// +build go1.18
-
 // Copyright 2017 Microsoft Corporation. All rights reserved.
 // Use of this source code is governed by an MIT
 // license that can be found in the LICENSE file.