Cross Platform lock + windows encryption by abhidnya13 · Pull Request #7 · AzureAD/microsoft-authentication-extensions-for-go

abhidnya13 · 2021-09-13T13:41:57Z

No description provided.

abhidnya13 · 2021-10-14T17:52:45Z

cross_platform_lock.go

+	"github.com/AzureAD/microsoft-authentication-extensions-for-go/flock"
+)
+
+type CrossPlatLock struct {


type Lock will be better
Lock.New() for initializing Lock
Directory structure - extensions main directory and then lock directory

element-of-surprise · 2021-10-14T17:54:50Z

cross_platform_lock.go

+)
+
+type CrossPlatLock struct {
+	retryNumber            int


Rename "retries"

element-of-surprise · 2021-10-14T17:55:15Z

cross_platform_lock.go

+
+type CrossPlatLock struct {
+	retryNumber            int
+	retryDelayMilliseconds time.Duration


rename retryDelay

element-of-surprise · 2021-10-14T17:55:33Z

cross_platform_lock.go

+
+	lockFile *os.File
+
+	fileLock *flock.Flock


element-of-surprise · 2021-10-14T17:56:18Z

cross_platform_lock.go

+
+	fileLock *flock.Flock
+
+	lockfileName string


element-of-surprise · 2021-10-14T17:56:59Z

cross_platform_lock.go

+	fileLock *flock.Flock
+
+	lockfileName string
+	locked       bool


element-of-surprise · 2021-10-14T18:03:02Z

cross_platform_lock.go

+
+	lockfileName string
+	locked       bool
+}


Add internal sync.Mutex called mu.

element-of-surprise · 2021-10-14T18:03:57Z

cross_platform_lock.go

+}
+
+func (c CrossPlatLock) Lock() error {
+	for tryCount := 0; tryCount < c.retryNumber; tryCount++ {


Add these lines:

c.mu.Lock()
defer c.mu.Unlock()

Change (c CrossPlatLock) to use a pointer

element-of-surprise · 2021-10-14T18:04:07Z

cross_platform_lock.go

+	locked       bool
+}
+
+func NewLock(lockFileName string, retryNumber int, retryDelayMilliseconds time.Duration) (CrossPlatLock, error) {


Have this return a pointer

Let's get rid of lockFileName, use a default defined in a constructor.

Let's use this signature:

type Option func(l *Lock)

func WithRetries(n int) Option{
return func(l *Lock) {
l.retries = n
}
}

func New(options ...Option) (*Lock, error) {
l := &Lock{
...
}
for _, o := range options {
o(l)
}
...
return l, nil
}

element-of-surprise · 2021-10-14T18:05:17Z

cross_platform_lock.go

+	return errors.New("Failed to acquire lock")
+}
+
+func (c CrossPlatLock) UnLock() error {


Add defer c.mu.Unlock()

element-of-surprise · 2021-10-14T18:17:13Z

cross_platform_lock.go

+			return nil
+		}
+	}
+	return errors.New("Failed to acquire lock")


error text must start with lower case letter

abhidnya13 · 2021-10-19T17:58:18Z

extensions/lock/lock_test.go

+
+func validateResult(cacheFile string, t *testing.T) int {
+	count := 0
+	var prevProc string = ""


var
(
count int
prevProc, tag, proc string
)
)

abhidnya13 · 2021-10-19T17:58:49Z

extensions/lock/lock_test.go

+	if err != nil {
+		log.Println(err)
+	}
+	dat := string(data)


All in one line and call it s instead of ele

element-of-surprise · 2021-10-20T18:24:09Z

extensions/lock/lock.go

+}
+
+func New(lockFileName string, options ...Option) (*Lock, error) {
+	l := &Lock{}


You need to add default value for retries and retryDelay.

element-of-surprise · 2021-10-20T18:24:43Z

extensions/lock/lock.go

+		l.retries = n
+	}
+}
+func WithRetryDelay(t time.Duration) Option {


Public requires a comment. I'd put:

// WithRetryDelay changes the default delay from [delay] to t.

element-of-surprise · 2021-10-20T18:25:45Z

extensions/lock/lock.go

+
+type Option func(l *Lock)
+
+func WithRetries(n int) Option {


Public requires a comment. I'd put:

// WithRetries changes the default number of retries from [retries] to n. Negative values panic()

Also, add panic on negative values.

element-of-surprise · 2021-10-20T18:28:26Z

extensions/lock/lock.go

+	}
+}
+
+func New(lockFileName string, options ...Option) (*Lock, error) {


lockFileName can be just "p".

// New creates a Lock for a lock file at "p". If "p" doesn't exist, it will be created when using. Lock().
func New(p string options ...Option) (*Lock, error)

element-of-surprise · 2021-10-20T18:28:52Z

extensions/lock/lock.go

+func (l *Lock) Lock() error {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	for tryCount := 0; tryCount < l.retries; tryCount++ {


for i := 0; i < l.retries; i++ {
...

element-of-surprise · 2021-10-20T18:49:14Z

extensions/lock/lock_test.go

+		prevProc = ""
+	}
+	return count, nil
+}


Add two tests:

One tests all the conditions of Lock().
Another tests all the conditions of Unlock()

flock.Unlock() has error

Lockfile can't be removed

You can do this by providing an interface:

type flocker interface{
TryLock() error
Unlock() error
Path() string
}

type fakeFlock struct {
err bool
}

func (f fakeFlock) TryLock() error {
if f.err {
return errors.New("error")
}
return nil
}

func (f fakeFlock) Unlock() error {
if f.err {
return errors.New("error")
}
return nil
}

element-of-surprise · 2021-10-20T18:50:25Z

extensions/lock/lock_test.go

+const cacheFile = "cache.txt"
+
+func TestLocking(t *testing.T) {
+


Remove blank line

element-of-surprise · 2021-10-20T18:53:00Z

extensions/lock/lock_test.go

+		if strings.TrimSpace(s) == "" {
+			continue
+		}
+		count += 1


element-of-surprise · 2021-10-20T18:54:03Z

go.mod

+require (
+	github.com/AzureAD/microsoft-authentication-library-for-go v0.3.0
+	github.com/billgraziano/dpapi v0.4.0
+	github.com/gofrs/flock v0.8.1


This flock line shouldn't. be here. So either a path change is needed or you need to run go tidy

element-of-surprise · 2021-10-20T18:55:20Z

internal/cache_accessor.go

@@ -0,0 +1,50 @@
+package internal


Move these files under:

internal/accessor
accessor.go
file/
file.go
windows/
window.go

accessor.go:

Defines type Cache interface{}
file.go:

Defines Accessor struct{}

Defines New() constructor
windows.go

Defines Accessor struct{}

Defines New() constructor

All "Accessor" types implement accessor.Cache

…soft-authentication-extensions-for-go into windows_encryption

chlowell · 2023-06-08T16:39:29Z

Superseded by #13 and #15

abhidnya13 and others added 7 commits August 30, 2021 12:07

Initial commit

180b593

Changes 1

187858e

More changes

93a1abd

Adding test cleanup

43149eb

Error handling changes

cbd3859

Adding modified time

b231fb4

Cleanup

54f1c22

abhidnya13 commented Oct 14, 2021

View reviewed changes