From a72c6f795fa44bc8be140eec01684c3d2a5cb96f Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Sun, 11 Oct 2020 13:22:17 +0530
Subject: [PATCH 01/10] started work on lock expiry

---
 internal/lockservice/simpleLockService.go | 35 +++++++++++++++++------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/internal/lockservice/simpleLockService.go b/internal/lockservice/simpleLockService.go
index bb61b5d..517c613 100644
--- a/internal/lockservice/simpleLockService.go
+++ b/internal/lockservice/simpleLockService.go
@@ -2,14 +2,17 @@ package lockservice
 
 import (
 	"sync"
+	"time"
 
 	"github.com/rs/zerolog"
 )
 
 // SafeLockMap is the lockserver's data structure
 type SafeLockMap struct {
-	LockMap map[string]string
-	Mutex   sync.Mutex
+	LockMap       map[string]string
+	TimestampMap  map[string]time.Time
+	LeaseDuration time.Duration
+	Mutex         sync.Mutex
 }
 
 // SimpleConfig implements Config.
@@ -115,36 +118,50 @@ func NewSimpleLockService(log zerolog.Logger) *SimpleLockService {
 	}
 }
 
+// getCurrentDuration returns the duration between the current time
+// and the time at which a lock was required
+func getCurrentDuration(timestamp time.Time) time.Duration {
+	return time.Now().Sub(timestamp)
+}
+
 // Acquire function lets a client acquire a lock on an object.
 func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 	ls.lockMap.Mutex.Lock()
-	if _, ok := ls.lockMap.LockMap[sd.ID()]; ok {
+
+	// If the lock is not present in the LockMap or
+	// the lock has expired, then allow the acquisition
+	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || getCurrentDuration(ls.lockMap.TimestampMap[sd.ID()]) > ls.lockMap.LeaseDuration {
+		ls.lockMap.LockMap[sd.ID()] = sd.Owner()
+		ls.lockMap.TimestampMap[sd.ID()] = time.Now()
 		ls.lockMap.Mutex.Unlock()
 		ls.
 			log.
 			Debug().
 			Str("descriptor", sd.ID()).
-			Msg("can't acquire, already been acquired")
-		return ErrFileacquired
+			Str("owner", sd.Owner()).
+			Str("timestamp", ls.lockMap.TimestampMap[sd.ID()].String()).
+			Msg("locked")
+		return nil
 	}
-	ls.lockMap.LockMap[sd.ID()] = sd.Owner()
 	ls.lockMap.Mutex.Unlock()
 	ls.
 		log.
 		Debug().
 		Str("descriptor", sd.ID()).
-		Str("owner", sd.Owner()).
-		Msg("locked")
-	return nil
+		Msg("can't acquire, already been acquired")
+	return ErrFileacquired
+
 }
 
 // Release lets a client to release a lock on an object.
+// TODO: Prevent a lock from being released if it has expired.
 func (ls *SimpleLockService) Release(sd Descriptors) error {
 	ls.lockMap.Mutex.Lock()
 	// Only the entity that posseses the lock for this object
 	// is allowed to release the lock
 	if ls.lockMap.LockMap[sd.ID()] == sd.Owner() {
 		delete(ls.lockMap.LockMap, sd.ID())
+		delete(ls.lockMap.TimestampMap, sd.ID())
 		ls.
 			log.
 			Debug().

From c7613663fccc112001a694342acb4885c0302b77 Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Wed, 11 Nov 2020 10:47:18 +0530
Subject: [PATCH 02/10] started wokring on lock expiry

---
 .vscode/settings.json                     |  2 ++
 internal/lockservice/simpleLockService.go | 25 +++++++++++++++++++----
 2 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 .vscode/settings.json

diff --git a/.vscode/settings.json b/.vscode/settings.json
new file mode 100644
index 0000000..7a73a41
--- /dev/null
+++ b/.vscode/settings.json
@@ -0,0 +1,2 @@
+{
+}
\ No newline at end of file
diff --git a/internal/lockservice/simpleLockService.go b/internal/lockservice/simpleLockService.go
index 517c613..a31a834 100644
--- a/internal/lockservice/simpleLockService.go
+++ b/internal/lockservice/simpleLockService.go
@@ -9,12 +9,21 @@ import (
 
 // SafeLockMap is the lockserver's data structure
 type SafeLockMap struct {
-	LockMap       map[string]string
+	LockMap       map[string]*LockMapEntry
 	TimestampMap  map[string]time.Time
 	LeaseDuration time.Duration
 	Mutex         sync.Mutex
 }
 
+// LockMapEntry defines the structure for objects placed
+// in the LockMap. It consists of the owner of the lock
+// that is acquired and the timestamp at which the
+// acquisition took place.
+type LockMapEntry struct {
+	owner     string
+	timestamp time.Time
+}
+
 // SimpleConfig implements Config.
 type SimpleConfig struct {
 	IPAddr   string
@@ -84,6 +93,14 @@ func (sd *LockDescriptor) Owner() string {
 	return sd.UserID
 }
 
+// NewLockMapEntry returns an instance of a LockMapEntry
+func NewLockMapEntry(owner string, timestamp time.Time) *LockMapEntry {
+	return &LockMapEntry{
+		owner:     owner,
+		timestamp: timestamp,
+	}
+}
+
 // NewSimpleConfig returns an instance of the SimpleConfig.
 func NewSimpleConfig(IPAddr, PortAddr string) *SimpleConfig {
 	return &SimpleConfig{
@@ -110,7 +127,7 @@ func NewObjectDescriptor(ObjectID string) *ObjectDescriptor {
 // NewSimpleLockService creates and returns a new lock service ready to use.
 func NewSimpleLockService(log zerolog.Logger) *SimpleLockService {
 	safeLockMap := &SafeLockMap{
-		LockMap: make(map[string]string),
+		LockMap: make(map[string]*LockMapEntry),
 	}
 	return &SimpleLockService{
 		log:     log,
@@ -131,7 +148,7 @@ func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 	// If the lock is not present in the LockMap or
 	// the lock has expired, then allow the acquisition
 	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || getCurrentDuration(ls.lockMap.TimestampMap[sd.ID()]) > ls.lockMap.LeaseDuration {
-		ls.lockMap.LockMap[sd.ID()] = sd.Owner()
+		ls.lockMap.LockMap[sd.ID()] = NewLockMapEntry(sd.Owner(), time.Now())
 		ls.lockMap.TimestampMap[sd.ID()] = time.Now()
 		ls.lockMap.Mutex.Unlock()
 		ls.
@@ -159,7 +176,7 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 	ls.lockMap.Mutex.Lock()
 	// Only the entity that posseses the lock for this object
 	// is allowed to release the lock
-	if ls.lockMap.LockMap[sd.ID()] == sd.Owner() {
+	if ls.lockMap.LockMap[sd.ID()].owner == sd.Owner() {
 		delete(ls.lockMap.LockMap, sd.ID())
 		delete(ls.lockMap.TimestampMap, sd.ID())
 		ls.

From e2deede1821db9e0d8d9470682302873d5add894 Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Tue, 24 Nov 2020 10:23:09 +0530
Subject: [PATCH 03/10] cleaned up code

---
 internal/lockclient/cache/error.go        |  2 +-
 internal/lockservice/simpleLockService.go | 13 +++++--------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/internal/lockclient/cache/error.go b/internal/lockclient/cache/error.go
index b5a0fa3..fbf20d3 100644
--- a/internal/lockclient/cache/error.go
+++ b/internal/lockclient/cache/error.go
@@ -9,6 +9,6 @@ func (e Error) Error() string { return string(e) }
 // Rule of thumb, all errors start with a small letter and end with no full stop.
 const (
 	ErrElementDoesntExist   = Error("element doesn't exist in the cache")
-	ErrElementAlreadyExists = Error("element already exists in the cache")
+	ErrElementAlreadyExists = Error("")
 	ErrCacheDoesntExist     = Error("cache doesn't exist")
 )
diff --git a/internal/lockservice/simpleLockService.go b/internal/lockservice/simpleLockService.go
index a31a834..27c4e41 100644
--- a/internal/lockservice/simpleLockService.go
+++ b/internal/lockservice/simpleLockService.go
@@ -10,7 +10,6 @@ import (
 // SafeLockMap is the lockserver's data structure
 type SafeLockMap struct {
 	LockMap       map[string]*LockMapEntry
-	TimestampMap  map[string]time.Time
 	LeaseDuration time.Duration
 	Mutex         sync.Mutex
 }
@@ -147,16 +146,15 @@ func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 
 	// If the lock is not present in the LockMap or
 	// the lock has expired, then allow the acquisition
-	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || getCurrentDuration(ls.lockMap.TimestampMap[sd.ID()]) > ls.lockMap.LeaseDuration {
+	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || getCurrentDuration(ls.lockMap.LockMap[sd.ID()].timestamp) > ls.lockMap.LeaseDuration {
 		ls.lockMap.LockMap[sd.ID()] = NewLockMapEntry(sd.Owner(), time.Now())
-		ls.lockMap.TimestampMap[sd.ID()] = time.Now()
 		ls.lockMap.Mutex.Unlock()
 		ls.
 			log.
 			Debug().
 			Str("descriptor", sd.ID()).
-			Str("owner", sd.Owner()).
-			Str("timestamp", ls.lockMap.TimestampMap[sd.ID()].String()).
+			Str("owner", ls.lockMap.LockMap[sd.ID()].owner).
+			Time("timestamp", ls.lockMap.LockMap[sd.ID()].timestamp).
 			Msg("locked")
 		return nil
 	}
@@ -178,7 +176,6 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 	// is allowed to release the lock
 	if ls.lockMap.LockMap[sd.ID()].owner == sd.Owner() {
 		delete(ls.lockMap.LockMap, sd.ID())
-		delete(ls.lockMap.TimestampMap, sd.ID())
 		ls.
 			log.
 			Debug().
@@ -214,14 +211,14 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 func (ls *SimpleLockService) CheckAcquired(sd Descriptors) (string, bool) {
 	ls.lockMap.Mutex.Lock()
 	id := sd.ID()
-	if owner, ok := ls.lockMap.LockMap[id]; ok {
+	if entry, ok := ls.lockMap.LockMap[id]; ok {
 		ls.lockMap.Mutex.Unlock()
 		ls.
 			log.
 			Debug().
 			Str("descriptor", id).
 			Msg("checkacquire success")
-		return owner, true
+		return entry.owner, true
 	}
 	ls.
 		log.

From 5d7733a8031936844e860bdf4b621aca00535592 Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Tue, 24 Nov 2020 11:33:34 +0530
Subject: [PATCH 04/10] acquire for expiry works, need to fix release to
 support this functionality

---
 internal/lockclient/simple_client_test.go |  8 +++--
 internal/lockservice/simpleLockService.go | 42 ++++++++++++++++-------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/internal/lockclient/simple_client_test.go b/internal/lockclient/simple_client_test.go
index ef5e36d..3e13dc0 100644
--- a/internal/lockclient/simple_client_test.go
+++ b/internal/lockclient/simple_client_test.go
@@ -17,7 +17,8 @@ func TestLockService(t *testing.T) {
 
 	log := zerolog.New(os.Stdout).With().Logger().Level(zerolog.GlobalLevel())
 	scfg := lockservice.NewSimpleConfig("http://127.0.0.1", "1234")
-	ls := lockservice.NewSimpleLockService(log)
+	duration := 2 * time.Second // 2 second expiry
+	ls := lockservice.NewSimpleLockService(log, duration)
 
 	quit := make(chan bool, 1)
 	go func() {
@@ -173,7 +174,7 @@ func BenchmarkLocKeyWithoutCache(b *testing.B) {
 
 	log := zerolog.New(os.Stdout).With().Logger().Level(zerolog.GlobalLevel())
 	scfg := lockservice.NewSimpleConfig("http://127.0.0.1", "1234")
-	ls := lockservice.NewSimpleLockService(log)
+	ls := lockservice.NewSimpleLockService(log, 5)
 
 	quit := make(chan bool, 1)
 	go func() {
@@ -211,7 +212,8 @@ func BenchmarkLocKeyWithCache(b *testing.B) {
 
 	log := zerolog.New(os.Stdout).With().Logger().Level(zerolog.GlobalLevel())
 	scfg := lockservice.NewSimpleConfig("http://127.0.0.1", "1234")
-	ls := lockservice.NewSimpleLockService(log)
+	duration := 2 * time.Second // 2 second expiry
+	ls := lockservice.NewSimpleLockService(log, duration)
 
 	quit := make(chan bool, 1)
 	go func() {
diff --git a/internal/lockservice/simpleLockService.go b/internal/lockservice/simpleLockService.go
index b9048f2..52fad36 100644
--- a/internal/lockservice/simpleLockService.go
+++ b/internal/lockservice/simpleLockService.go
@@ -1,6 +1,7 @@
 package lockservice
 
 import (
+	"fmt"
 	"sync"
 	"time"
 
@@ -130,20 +131,35 @@ func NewObjectDescriptor(ObjectID string) *ObjectDescriptor {
 }
 
 // NewSimpleLockService creates and returns a new lock service ready to use.
-func NewSimpleLockService(log zerolog.Logger) *SimpleLockService {
+func NewSimpleLockService(log zerolog.Logger, leaseDuration time.Duration) *SimpleLockService {
 	safeLockMap := &SafeLockMap{
 		LockMap: make(map[string]*LockMapEntry),
 	}
+	safeLockMap.LeaseDuration = leaseDuration
 	return &SimpleLockService{
 		log:     log,
 		lockMap: safeLockMap,
 	}
 }
+func fmtDuration(d time.Duration) string {
+	d = d.Round(time.Minute)
+	h := d / time.Hour
+	d -= h * time.Hour
+	ms := d / time.Microsecond
+	return fmt.Sprintf("%02d:%02d", h, ms)
+}
 
 // getCurrentDuration returns the duration between the current time
 // and the time at which a lock was required
-func getCurrentDuration(timestamp time.Time) time.Duration {
-	return time.Now().Sub(timestamp)
+func compareDuration(timestamp time.Time, lease time.Duration) bool {
+	// fmt.Printf("current: %s timestamp: %s duration: %s %s\n", time.Now().String(), timestamp.String(), fmtDuration(time.Now().Sub(timestamp)))
+	intDuration := int64(time.Now().Sub(timestamp))
+	intLease := int64(lease)
+	fmt.Printf("%d %d \n", intDuration, intLease)
+	if time.Now().Sub(timestamp) > lease {
+		return true
+	}
+	return false
 }
 
 // Acquire function lets a client acquire a lock on an object.
@@ -152,7 +168,7 @@ func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 
 	// If the lock is not present in the LockMap or
 	// the lock has expired, then allow the acquisition
-	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || getCurrentDuration(ls.lockMap.LockMap[sd.ID()].timestamp) > ls.lockMap.LeaseDuration {
+	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || (ok && (compareDuration(ls.lockMap.LockMap[sd.ID()].timestamp, ls.lockMap.LeaseDuration))) {
 		ls.lockMap.LockMap[sd.ID()] = NewLockMapEntry(sd.Owner(), time.Now())
 		ls.lockMap.Mutex.Unlock()
 		ls.
@@ -180,25 +196,25 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 	ls.lockMap.Mutex.Lock()
 	// Only the entity that posseses the lock for this object
 	// is allowed to release the lock
-	if ls.lockMap.LockMap[sd.ID()].owner == sd.Owner() {
-		delete(ls.lockMap.LockMap, sd.ID())
+	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok {
 		ls.
 			log.
 			Debug().
 			Str("descriptor", sd.ID()).
-			Str("owner", sd.Owner()).
-			Msg("released")
+			Msg("can't release, hasn't been acquired")
 		ls.lockMap.Mutex.Unlock()
-		return nil
-	} else if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok {
+		return ErrCantReleaseFile
+
+	} else if ls.lockMap.LockMap[sd.ID()].owner == sd.Owner() {
+		delete(ls.lockMap.LockMap, sd.ID())
 		ls.
 			log.
 			Debug().
 			Str("descriptor", sd.ID()).
-			Msg("can't release, hasn't been acquired")
+			Str("owner", sd.Owner()).
+			Msg("released")
 		ls.lockMap.Mutex.Unlock()
-		return ErrCantReleaseFile
-
+		return nil
 	} else {
 		ls.
 			log.

From 6759e052e54f1931bec1e117221e201e2303a3fd Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Mon, 21 Dec 2020 11:15:11 +0530
Subject: [PATCH 05/10] added test for lock expiry test

---
 cmd/main.go                               |  3 ++-
 internal/lockclient/simple_client.go      |  4 +++-
 internal/lockclient/simple_client_test.go | 26 ++++++++++++++++++++---
 internal/lockservice/simpleLockService.go | 12 ++++++++++-
 4 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/cmd/main.go b/cmd/main.go
index 865607d..314edee 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -2,6 +2,7 @@ package main
 
 import (
 	"os"
+	"time"
 
 	"github.com/SystemBuilders/LocKey/internal/lockservice"
 	"github.com/SystemBuilders/LocKey/internal/lockservice/node"
@@ -12,7 +13,7 @@ func main() {
 	zerolog.New(os.Stdout).With()
 
 	log := zerolog.New(os.Stdout).With().Logger().Level(zerolog.GlobalLevel())
-	ls := lockservice.NewSimpleLockService(log)
+	ls := lockservice.NewSimpleLockService(log, 5*time.Second)
 
 	scfg := lockservice.NewSimpleConfig("127.0.0.1", "1234")
 	node.Start(ls, *scfg)
diff --git a/internal/lockclient/simple_client.go b/internal/lockclient/simple_client.go
index 411e997..d57545a 100644
--- a/internal/lockclient/simple_client.go
+++ b/internal/lockclient/simple_client.go
@@ -435,7 +435,9 @@ func (sc *SimpleClient) startSession(processID id.ID) {
 		sc.sessionTimers[processID] = timerChan
 		sc.mu.Unlock()
 		// Sessions last for 200ms.
-		time.Sleep(200 * time.Millisecond)
+		// changed to 10s just to test lock expiry
+		// TODO: Make the desired length of session expiry user configurable
+		time.Sleep(10000 * time.Millisecond)
 
 		sc.mu.Lock()
 		sc.sessionTimers[processID] <- struct{}{}
diff --git a/internal/lockclient/simple_client_test.go b/internal/lockclient/simple_client_test.go
index 3e13dc0..69cc812 100644
--- a/internal/lockclient/simple_client_test.go
+++ b/internal/lockclient/simple_client_test.go
@@ -155,14 +155,34 @@ func TestLockService(t *testing.T) {
 			t.Errorf("acquire: got %q want %q", got, want)
 		}
 
-		// Wait for the session to expire
-		time.Sleep(500 * time.Millisecond)
+		// Wait for the lock's lease to expire
+		time.Sleep(3 * time.Second)
+
 		got = sc.Release(d, session)
-		want = ErrSessionNonExistent
+		want = lockservice.ErrCantReleaseFile
 		if got != want {
 			t.Errorf("release: got %q want %q", got, want)
 		}
 	})
+	t.Run("try acquiring after lock expiry; should succeed", func(t *testing.T) {
+		sc := NewSimpleClient(scfg, log, nil)
+		session := sc.Connect()
+		d := lockservice.NewObjectDescriptor("test2")
+
+		got := sc.Acquire(d, session)
+		var want error
+		if got != want {
+			t.Errorf("acquire: got %q want %q", got, want)
+		}
+
+		// Wait for the lock's lease to expire
+		time.Sleep(3 * time.Second)
+
+		got = sc.Acquire(d, session)
+		if got != want {
+			t.Errorf("acquire: got %q want %q", got, want)
+		}
+	})
 
 	quit <- true
 	return
diff --git a/internal/lockservice/simpleLockService.go b/internal/lockservice/simpleLockService.go
index 52fad36..a90ea6d 100644
--- a/internal/lockservice/simpleLockService.go
+++ b/internal/lockservice/simpleLockService.go
@@ -191,7 +191,7 @@ func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 }
 
 // Release lets a client to release a lock on an object.
-// TODO: Prevent a lock from being released if it has expired.
+// TODO: Prevent a lock from being released if it has expired. !!!!!!
 func (ls *SimpleLockService) Release(sd Descriptors) error {
 	ls.lockMap.Mutex.Lock()
 	// Only the entity that posseses the lock for this object
@@ -205,6 +205,16 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 		ls.lockMap.Mutex.Unlock()
 		return ErrCantReleaseFile
 
+	} else if compareDuration(ls.lockMap.LockMap[sd.ID()].timestamp, ls.lockMap.LeaseDuration) {
+		// lease expired
+		ls.
+			log.
+			Debug().
+			Str("descriptor", sd.ID()).
+			Msg("can't release, lease of lock has expired")
+		ls.lockMap.Mutex.Unlock()
+		return ErrCantReleaseFile
+
 	} else if ls.lockMap.LockMap[sd.ID()].owner == sd.Owner() {
 		delete(ls.lockMap.LockMap, sd.ID())
 		ls.

From 0b321c2d1652e6b2f11f9f52b786f9d510a15b50 Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Mon, 28 Dec 2020 21:18:23 +0530
Subject: [PATCH 06/10] Update internal/lockservice/simpleLockService.go

Co-authored-by: Sumukha Pk <sumukhapk46@gmail.com>
---
 internal/lockservice/simpleLockService.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/lockservice/simpleLockService.go b/internal/lockservice/simpleLockService.go
index a90ea6d..c559ceb 100644
--- a/internal/lockservice/simpleLockService.go
+++ b/internal/lockservice/simpleLockService.go
@@ -150,7 +150,7 @@ func fmtDuration(d time.Duration) string {
 }
 
 // getCurrentDuration returns the duration between the current time
-// and the time at which a lock was required
+// and the time at which a lock was required.
 func compareDuration(timestamp time.Time, lease time.Duration) bool {
 	// fmt.Printf("current: %s timestamp: %s duration: %s %s\n", time.Now().String(), timestamp.String(), fmtDuration(time.Now().Sub(timestamp)))
 	intDuration := int64(time.Now().Sub(timestamp))

From e35d304a843adfdd95990a7b00bc9a51a8c6dcd5 Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Wed, 30 Dec 2020 17:26:22 +0530
Subject: [PATCH 07/10] made changes to expiry based on review

---
 .gitignore                                |  2 +-
 .vscode/settings.json                     |  2 --
 internal/lockclient/cache/error.go        |  2 +-
 internal/lockservice/simpleLockService.go | 12 +++++-------
 4 files changed, 7 insertions(+), 11 deletions(-)
 delete mode 100644 .vscode/settings.json

diff --git a/.gitignore b/.gitignore
index 5242542..693daf8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,2 @@
 main
-.vscode
\ No newline at end of file
+.vscode/
\ No newline at end of file
diff --git a/.vscode/settings.json b/.vscode/settings.json
deleted file mode 100644
index 7a73a41..0000000
--- a/.vscode/settings.json
+++ /dev/null
@@ -1,2 +0,0 @@
-{
-}
\ No newline at end of file
diff --git a/internal/lockclient/cache/error.go b/internal/lockclient/cache/error.go
index fbf20d3..b5a0fa3 100644
--- a/internal/lockclient/cache/error.go
+++ b/internal/lockclient/cache/error.go
@@ -9,6 +9,6 @@ func (e Error) Error() string { return string(e) }
 // Rule of thumb, all errors start with a small letter and end with no full stop.
 const (
 	ErrElementDoesntExist   = Error("element doesn't exist in the cache")
-	ErrElementAlreadyExists = Error("")
+	ErrElementAlreadyExists = Error("element already exists in the cache")
 	ErrCacheDoesntExist     = Error("cache doesn't exist")
 )
diff --git a/internal/lockservice/simpleLockService.go b/internal/lockservice/simpleLockService.go
index a90ea6d..d562ef0 100644
--- a/internal/lockservice/simpleLockService.go
+++ b/internal/lockservice/simpleLockService.go
@@ -149,10 +149,9 @@ func fmtDuration(d time.Duration) string {
 	return fmt.Sprintf("%02d:%02d", h, ms)
 }
 
-// getCurrentDuration returns the duration between the current time
-// and the time at which a lock was required
-func compareDuration(timestamp time.Time, lease time.Duration) bool {
-	// fmt.Printf("current: %s timestamp: %s duration: %s %s\n", time.Now().String(), timestamp.String(), fmtDuration(time.Now().Sub(timestamp)))
+// hasLeaseExpired returns true if the lease for a lock has expired and
+// returns false if the lease is still valid
+func hasLeaseExpired(timestamp time.Time, lease time.Duration) bool {
 	intDuration := int64(time.Now().Sub(timestamp))
 	intLease := int64(lease)
 	fmt.Printf("%d %d \n", intDuration, intLease)
@@ -168,7 +167,7 @@ func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 
 	// If the lock is not present in the LockMap or
 	// the lock has expired, then allow the acquisition
-	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || (ok && (compareDuration(ls.lockMap.LockMap[sd.ID()].timestamp, ls.lockMap.LeaseDuration))) {
+	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || hasLeaseExpired(ls.lockMap.LockMap[sd.ID()].timestamp, ls.lockMap.LeaseDuration) {
 		ls.lockMap.LockMap[sd.ID()] = NewLockMapEntry(sd.Owner(), time.Now())
 		ls.lockMap.Mutex.Unlock()
 		ls.
@@ -191,7 +190,6 @@ func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 }
 
 // Release lets a client to release a lock on an object.
-// TODO: Prevent a lock from being released if it has expired. !!!!!!
 func (ls *SimpleLockService) Release(sd Descriptors) error {
 	ls.lockMap.Mutex.Lock()
 	// Only the entity that posseses the lock for this object
@@ -205,7 +203,7 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 		ls.lockMap.Mutex.Unlock()
 		return ErrCantReleaseFile
 
-	} else if compareDuration(ls.lockMap.LockMap[sd.ID()].timestamp, ls.lockMap.LeaseDuration) {
+	} else if hasLeaseExpired(ls.lockMap.LockMap[sd.ID()].timestamp, ls.lockMap.LeaseDuration) {
 		// lease expired
 		ls.
 			log.

From cd7a20ae794510dc8d89de58bb5bc7c6acc315f1 Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Wed, 30 Dec 2020 20:09:02 +0530
Subject: [PATCH 08/10] made the duration of the session a configurable
 parameter

---
 internal/lockclient/simple_client.go      | 11 ++++++-----
 internal/lockclient/simple_client_test.go | 17 ++++++++++-------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/internal/lockclient/simple_client.go b/internal/lockclient/simple_client.go
index d57545a..dd51c26 100644
--- a/internal/lockclient/simple_client.go
+++ b/internal/lockclient/simple_client.go
@@ -38,11 +38,13 @@ type SimpleClient struct {
 	// whether the process owning the lock has an active session
 	// or not, this guarantee has to be ensured by the client.
 	sessionAcquisitions map[id.ID][]lockservice.Descriptors
+
+	sessionDuration time.Duration
 }
 
 // NewSimpleClient returns a new SimpleClient of the given parameters.
 // This client works with or without the existance of a cache.
-func NewSimpleClient(config *lockservice.SimpleConfig, log zerolog.Logger, cache *cache.LRUCache) *SimpleClient {
+func NewSimpleClient(config *lockservice.SimpleConfig, log zerolog.Logger, cache *cache.LRUCache, sessionDuration time.Duration) *SimpleClient {
 	clientID := id.Create()
 	sessions := make(map[id.ID]session.Session)
 	sessionTimers := make(map[id.ID]chan struct{})
@@ -55,6 +57,7 @@ func NewSimpleClient(config *lockservice.SimpleConfig, log zerolog.Logger, cache
 		sessions:            sessions,
 		sessionTimers:       sessionTimers,
 		sessionAcquisitions: sessionAcquisitions,
+		sessionDuration:     sessionDuration,
 	}
 }
 
@@ -434,10 +437,8 @@ func (sc *SimpleClient) startSession(processID id.ID) {
 		sc.mu.Lock()
 		sc.sessionTimers[processID] = timerChan
 		sc.mu.Unlock()
-		// Sessions last for 200ms.
-		// changed to 10s just to test lock expiry
-		// TODO: Make the desired length of session expiry user configurable
-		time.Sleep(10000 * time.Millisecond)
+		// Sessions last for user configured duration
+		time.Sleep(sc.sessionDuration)
 
 		sc.mu.Lock()
 		sc.sessionTimers[processID] <- struct{}{}
diff --git a/internal/lockclient/simple_client_test.go b/internal/lockclient/simple_client_test.go
index 69cc812..e60e3f8 100644
--- a/internal/lockclient/simple_client_test.go
+++ b/internal/lockclient/simple_client_test.go
@@ -18,6 +18,7 @@ func TestLockService(t *testing.T) {
 	log := zerolog.New(os.Stdout).With().Logger().Level(zerolog.GlobalLevel())
 	scfg := lockservice.NewSimpleConfig("http://127.0.0.1", "1234")
 	duration := 2 * time.Second // 2 second expiry
+	sessionDuration := 5 * time.Second
 	ls := lockservice.NewSimpleLockService(log, duration)
 
 	quit := make(chan bool, 1)
@@ -43,7 +44,7 @@ func TestLockService(t *testing.T) {
 	t.Run("acquire test release test", func(t *testing.T) {
 		size := 5
 		cache := cache.NewLRUCache(size)
-		sc := NewSimpleClient(scfg, log, cache)
+		sc := NewSimpleClient(scfg, log, cache, sessionDuration)
 
 		session := sc.Connect()
 
@@ -77,7 +78,7 @@ func TestLockService(t *testing.T) {
 	t.Run("acquire test, acquire test, release test", func(t *testing.T) {
 		size := 5
 		cache := cache.NewLRUCache(size)
-		sc := NewSimpleClient(scfg, log, cache)
+		sc := NewSimpleClient(scfg, log, cache, sessionDuration)
 
 		session := sc.Connect()
 		d := lockservice.NewObjectDescriptor("test")
@@ -105,7 +106,7 @@ func TestLockService(t *testing.T) {
 	t.Run("acquire test, trying to release test as another entity should fail", func(t *testing.T) {
 		size := 2
 		cache := cache.NewLRUCache(size)
-		sc := NewSimpleClient(scfg, log, cache)
+		sc := NewSimpleClient(scfg, log, cache, sessionDuration)
 
 		session := sc.Connect()
 		d := lockservice.NewObjectDescriptor("test")
@@ -145,7 +146,7 @@ func TestLockService(t *testing.T) {
 	})
 
 	t.Run("acquire test and release after session expiry", func(t *testing.T) {
-		sc := NewSimpleClient(scfg, log, nil)
+		sc := NewSimpleClient(scfg, log, nil, sessionDuration)
 		session := sc.Connect()
 		d := lockservice.NewObjectDescriptor("test3")
 
@@ -165,7 +166,7 @@ func TestLockService(t *testing.T) {
 		}
 	})
 	t.Run("try acquiring after lock expiry; should succeed", func(t *testing.T) {
-		sc := NewSimpleClient(scfg, log, nil)
+		sc := NewSimpleClient(scfg, log, nil, sessionDuration)
 		session := sc.Connect()
 		d := lockservice.NewObjectDescriptor("test2")
 
@@ -209,7 +210,8 @@ func BenchmarkLocKeyWithoutCache(b *testing.B) {
 	}()
 	time.Sleep(100 * time.Millisecond)
 
-	sc := NewSimpleClient(scfg, log, nil)
+	sessionDuration := 5 * time.Second
+	sc := NewSimpleClient(scfg, log, nil, sessionDuration)
 	session := sc.Connect()
 	d := lockservice.NewObjectDescriptor("test")
 	for n := 0; n < b.N; n++ {
@@ -250,7 +252,8 @@ func BenchmarkLocKeyWithCache(b *testing.B) {
 
 	size := 5
 	cache := cache.NewLRUCache(size)
-	sc := NewSimpleClient(scfg, log, cache)
+	sessionDuration := 5 * time.Second
+	sc := NewSimpleClient(scfg, log, cache, sessionDuration)
 	session := sc.Connect()
 	d := lockservice.NewObjectDescriptor("test")
 	for n := 0; n < b.N; n++ {

From 4d6134da58b3f17153520d867d11271d1756e31a Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Thu, 31 Dec 2020 11:58:59 +0530
Subject: [PATCH 09/10] added documentation

---
 internal/lockservice/simpleLockService.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/internal/lockservice/simpleLockService.go b/internal/lockservice/simpleLockService.go
index d562ef0..de9e790 100644
--- a/internal/lockservice/simpleLockService.go
+++ b/internal/lockservice/simpleLockService.go
@@ -162,6 +162,10 @@ func hasLeaseExpired(timestamp time.Time, lease time.Duration) bool {
 }
 
 // Acquire function lets a client acquire a lock on an object.
+// This lock is valid for a fixed duration that is set in the SafeLockMap.LeaseDuration
+// field. Beyond this duration, the lock has expired and the entity that owned the lock
+// for this period can no longer release it. The lock is open for acquisition after it
+// has expired.
 func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 	ls.lockMap.Mutex.Lock()
 
@@ -180,6 +184,7 @@ func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 		return nil
 	}
 	ls.lockMap.Mutex.Unlock()
+	// Since the lock is already acquired, return an error
 	ls.
 		log.
 		Debug().
@@ -195,6 +200,7 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 	// Only the entity that posseses the lock for this object
 	// is allowed to release the lock
 	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok {
+		// trying to release an unacquired lock
 		ls.
 			log.
 			Debug().
@@ -214,6 +220,7 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 		return ErrCantReleaseFile
 
 	} else if ls.lockMap.LockMap[sd.ID()].owner == sd.Owner() {
+		// conditions satisfied, lock is released
 		delete(ls.lockMap.LockMap, sd.ID())
 		ls.
 			log.
@@ -224,6 +231,7 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 		ls.lockMap.Mutex.Unlock()
 		return nil
 	} else {
+		// trying to release a lock that you don't own
 		ls.
 			log.
 			Debug().

From d0da4030dd69d3aa9b37b0d37aaa5df5edf0622c Mon Sep 17 00:00:00 2001
From: Suraj Singh <suraj1998@gmail.com>
Date: Sun, 3 Jan 2021 23:46:05 +0530
Subject: [PATCH 10/10] made changes in line with review

---
 internal/lockclient/simple_client.go      |  5 +++--
 internal/lockservice/simpleLockService.go | 13 +++++++------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/internal/lockclient/simple_client.go b/internal/lockclient/simple_client.go
index dd51c26..7ad57e3 100644
--- a/internal/lockclient/simple_client.go
+++ b/internal/lockclient/simple_client.go
@@ -44,7 +44,8 @@ type SimpleClient struct {
 
 // NewSimpleClient returns a new SimpleClient of the given parameters.
 // This client works with or without the existance of a cache.
-func NewSimpleClient(config *lockservice.SimpleConfig, log zerolog.Logger, cache *cache.LRUCache, sessionDuration time.Duration) *SimpleClient {
+func NewSimpleClient(config *lockservice.SimpleConfig, log zerolog.Logger, cache *cache.LRUCache,
+	sessionDuration time.Duration) *SimpleClient {
 	clientID := id.Create()
 	sessions := make(map[id.ID]session.Session)
 	sessionTimers := make(map[id.ID]chan struct{})
@@ -437,7 +438,7 @@ func (sc *SimpleClient) startSession(processID id.ID) {
 		sc.mu.Lock()
 		sc.sessionTimers[processID] = timerChan
 		sc.mu.Unlock()
-		// Sessions last for user configured duration
+		// Sessions last for user configured duration.
 		time.Sleep(sc.sessionDuration)
 
 		sc.mu.Lock()
diff --git a/internal/lockservice/simpleLockService.go b/internal/lockservice/simpleLockService.go
index de9e790..6d06e19 100644
--- a/internal/lockservice/simpleLockService.go
+++ b/internal/lockservice/simpleLockService.go
@@ -150,11 +150,8 @@ func fmtDuration(d time.Duration) string {
 }
 
 // hasLeaseExpired returns true if the lease for a lock has expired and
-// returns false if the lease is still valid
+// false if the lease is still valid
 func hasLeaseExpired(timestamp time.Time, lease time.Duration) bool {
-	intDuration := int64(time.Now().Sub(timestamp))
-	intLease := int64(lease)
-	fmt.Printf("%d %d \n", intDuration, intLease)
 	if time.Now().Sub(timestamp) > lease {
 		return true
 	}
@@ -169,9 +166,11 @@ func hasLeaseExpired(timestamp time.Time, lease time.Duration) bool {
 func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 	ls.lockMap.Mutex.Lock()
 
+	timestamp := ls.lockMap.LockMap[sd.ID()].timestamp
+	duration := ls.lockMap.LeaseDuration
 	// If the lock is not present in the LockMap or
 	// the lock has expired, then allow the acquisition
-	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || hasLeaseExpired(ls.lockMap.LockMap[sd.ID()].timestamp, ls.lockMap.LeaseDuration) {
+	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok || hasLeaseExpired(timestamp, duration) {
 		ls.lockMap.LockMap[sd.ID()] = NewLockMapEntry(sd.Owner(), time.Now())
 		ls.lockMap.Mutex.Unlock()
 		ls.
@@ -197,6 +196,8 @@ func (ls *SimpleLockService) Acquire(sd Descriptors) error {
 // Release lets a client to release a lock on an object.
 func (ls *SimpleLockService) Release(sd Descriptors) error {
 	ls.lockMap.Mutex.Lock()
+	timestamp := ls.lockMap.LockMap[sd.ID()].timestamp
+	duration := ls.lockMap.LeaseDuration
 	// Only the entity that posseses the lock for this object
 	// is allowed to release the lock
 	if _, ok := ls.lockMap.LockMap[sd.ID()]; !ok {
@@ -209,7 +210,7 @@ func (ls *SimpleLockService) Release(sd Descriptors) error {
 		ls.lockMap.Mutex.Unlock()
 		return ErrCantReleaseFile
 
-	} else if hasLeaseExpired(ls.lockMap.LockMap[sd.ID()].timestamp, ls.lockMap.LeaseDuration) {
+	} else if hasLeaseExpired(timestamp, duration) {
 		// lease expired
 		ls.
 			log.