From 9a23bd14f0cf093a7770ba2349bb212413b0ae19 Mon Sep 17 00:00:00 2001
From: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Date: Wed, 10 Jan 2024 13:43:13 -0800
Subject: [PATCH] internal/session: tidy up query

---
 internal/session/query.go              | 33 ++++----------------------
 internal/session/repository_session.go | 26 +++++---------------
 internal/session/repository_test.go    |  2 +-
 3 files changed, 11 insertions(+), 50 deletions(-)

diff --git a/internal/session/query.go b/internal/session/query.go
index c16ecd0bb3..b2ceacf77d 100644
--- a/internal/session/query.go
+++ b/internal/session/query.go
@@ -124,27 +124,6 @@ from
 	session_connection_limit, session_connection_count;
 `
 
-	sessionList = `
-with
-session_ids as (
-	select public_id
-	from session as s
-	-- where clause is constructed
-	%s
-	-- order by clause is constructed
-	%s
-	-- limit is constructed
-	%s
-)
-select *
-from session_list
-where
-	session_list.public_id in (select * from session_ids)
--- order by clause again since order from cte is not guaranteed to be preserved
-%s
-;
-`
-
 	terminateSessionIfPossible = `
     -- is terminate_session_id in a canceling state
     with session_version as (
@@ -427,8 +406,7 @@ where session_id = ?
 with session_ids as (
     select public_id
       from session
-           -- search condition for applying permissions is constructed
-     where %s
+     where %s -- search condition for applying permissions is constructed
   order by create_time desc, public_id desc
      limit %d
 )
@@ -442,8 +420,7 @@ with session_ids as (
     select public_id
       from session
      where (create_time, public_id) < (@last_item_create_time, @last_item_id)
-           -- search condition for applying permissions is constructed
-       and %s
+       and %s -- search condition for applying permissions is constructed
   order by create_time desc, public_id desc
      limit %d
 )
@@ -457,8 +434,7 @@ with session_ids as (
     select public_id
       from session
      where update_time > @updated_after_time
-           -- search condition for applying permissions is constructed
-       and %s
+       and %s -- search condition for applying permissions is constructed
   order by update_time desc, public_id desc
      limit %d
 )
@@ -473,8 +449,7 @@ with session_ids as (
       from session
      where update_time > @updated_after_time
        and (update_time, public_id) < (@last_item_update_time, @last_item_id)
-           -- search condition for applying permissions is constructed
-       and %s
+       and %s -- search condition for applying permissions is constructed
   order by update_time desc, public_id desc
      limit %d
 )
diff --git a/internal/session/repository_session.go b/internal/session/repository_session.go
index 7f9babaa1f..948f191ad4 100644
--- a/internal/session/repository_session.go
+++ b/internal/session/repository_session.go
@@ -290,16 +290,9 @@ func (r *Repository) listSessions(ctx context.Context, opt ...Option) ([]*Sessio
 
 	opts := getOpts(opt...)
 
-	var permissionWhereClause string
-	if len(where) > 0 {
-		permissionWhereClause = "(" + strings.Join(where, " or ") + ")"
-		if !opts.withTerminated {
-			permissionWhereClause += " and termination_reason is null"
-		}
-	} else {
-		if !opts.withTerminated {
-			permissionWhereClause = " where termination_reason is null"
-		}
+	permissionWhereClause := "(" + strings.Join(where, " or ") + ")"
+	if !opts.withTerminated {
+		permissionWhereClause += " and termination_reason is null"
 	}
 
 	limit := r.defaultLimit
@@ -339,16 +332,9 @@ func (r *Repository) listSessionsRefresh(ctx context.Context, updatedAfter time.
 
 	opts := getOpts(opt...)
 
-	var permissionWhereClause string
-	if len(where) > 0 {
-		permissionWhereClause = "(" + strings.Join(where, " or ") + ")"
-		if !opts.withTerminated {
-			permissionWhereClause += " and termination_reason is null"
-		}
-	} else {
-		if !opts.withTerminated {
-			permissionWhereClause = " where termination_reason is null"
-		}
+	permissionWhereClause := "(" + strings.Join(where, " or ") + ")"
+	if !opts.withTerminated {
+		permissionWhereClause += " and termination_reason is null"
 	}
 
 	limit := r.defaultLimit
diff --git a/internal/session/repository_test.go b/internal/session/repository_test.go
index 95396218c4..7967680580 100644
--- a/internal/session/repository_test.go
+++ b/internal/session/repository_test.go
@@ -132,7 +132,7 @@ func TestRepository_convertToSessions(t *testing.T) {
 	sess, err = repo.CreateSession(ctx, sessionWrapper, sess, []string{"0.0.0.0"})
 	require.NoError(t, err)
 
-	query := fmt.Sprintf(sessionList, "", "", "", "")
+	query := fmt.Sprintf(listSessionsTemplate, "termination_reason is null", 1000)
 	rows, err := rw.Query(ctx, query, nil)
 	require.NoError(t, err)
 	t.Cleanup(func() {