Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add PostgreSQL as storage backend #89

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

radiohertz
Copy link
Contributor

@radiohertz radiohertz commented Nov 5, 2024

closes #88

Most of it is ported from #5.

  • Used lib/pq as pgx requires go 1.21.
  • How should I handle migrations? Any pointers? I didn't see any existing code from the mysql impl.

cc @jessepeterson

@radiohertz radiohertz changed the title feat: add PostgreSQL as storage backend Draft: feat: add PostgreSQL as storage backend Nov 5, 2024
@radiohertz radiohertz force-pushed the dev-pg-backend branch 8 times, most recently from 16a2dd3 to a0052a0 Compare November 5, 2024 07:19
storage/psql/psql.go Outdated Show resolved Hide resolved
@radiohertz radiohertz changed the title Draft: feat: add PostgreSQL as storage backend feat: add PostgreSQL as storage backend Nov 5, 2024
@jessepeterson
Copy link
Member

@radiohertz hello. Re: migrations.

We've not used any schema migration tooling at all. In the other Nano* project MySQL backends we provide manually generated numbered SQL schema files. For example see NanoMDM's MySQL backend storage folder with the set of schema.0000X.sql files.

@jessepeterson
Copy link
Member

@radiohertz Quick review thoughts. Wanna get these done and we can review again?

  1. Include a generate.go for sqlc.
  2. NanoMDM uses a backend name of pgsql which fits nicely with mysql. You've used psql here. I think I'd prefer that (not to mention is not to be confused with the psql CLI tool).
  3. I think the pq driver natively supports parsing the time.Time types. So we can convert to using sqlc's native time handling (mysql driver is an optional argument I didn't want to require). I.e.:
diff --git a/storage/psql/psql.go b/storage/psql/psql.go
index 62ac2bd..7e07175 100644
--- a/storage/psql/psql.go
+++ b/storage/psql/psql.go
@@ -72,8 +72,6 @@ func New(opts ...Option) (*PSQLStorage, error) {
 
 }
 
-const timestampFormat = "2006-01-02T15:04:05Z"
-
 // RetrieveAuthTokens reads the DEP OAuth tokens for name (DEP name).
 func (s *PSQLStorage) RetrieveAuthTokens(ctx context.Context, name string) (*client.OAuth1Tokens, error) {
 	tokenRow, err := s.q.GetAuthTokens(ctx, name)
@@ -86,17 +84,12 @@ func (s *PSQLStorage) RetrieveAuthTokens(ctx context.Context, name string) (*cli
 	if !tokenRow.ConsumerKey.Valid { // all auth token fields are set together
 		return nil, fmt.Errorf("consumer key not valid: %w", storage.ErrNotFound)
 	}
-	fmt.Println(tokenRow.AccessTokenExpiry.String)
-	accessTokenExpiryTime, err := time.Parse(timestampFormat, tokenRow.AccessTokenExpiry.String)
-	if err != nil {
-		return nil, err
-	}
 	return &client.OAuth1Tokens{
 		ConsumerKey:       tokenRow.ConsumerKey.String,
 		ConsumerSecret:    tokenRow.ConsumerSecret.String,
 		AccessToken:       tokenRow.AccessToken.String,
 		AccessSecret:      tokenRow.AccessSecret.String,
-		AccessTokenExpiry: accessTokenExpiryTime,
+		AccessTokenExpiry: tokenRow.AccessTokenExpiry.Time,
 	}, nil
 }
 
@@ -108,7 +101,7 @@ func (s *PSQLStorage) StoreAuthTokens(ctx context.Context, name string, tokens *
 		ConsumerSecret:    sql.NullString{String: tokens.ConsumerSecret, Valid: true},
 		AccessToken:       sql.NullString{String: tokens.AccessToken, Valid: true},
 		AccessSecret:      sql.NullString{String: tokens.AccessSecret, Valid: true},
-		AccessTokenExpiry: sql.NullString{String: tokens.AccessTokenExpiry.Format(timestampFormat), Valid: true},
+		AccessTokenExpiry: sql.NullTime{Time: tokens.AccessTokenExpiry, Valid: true},
 	})
 }
 
@@ -158,7 +151,7 @@ func (s *PSQLStorage) RetrieveAssignerProfile(ctx context.Context, name string)
 		profileUUID = assignerRow.AssignerProfileUuid.String
 	}
 	if assignerRow.AssignerProfileUuidAt.Valid {
-		modTime, err = time.Parse(timestampFormat, assignerRow.AssignerProfileUuidAt.String)
+		modTime = assignerRow.AssignerProfileUuidAt.Time
 	}
 	return
 }
diff --git a/storage/psql/sqlc.yaml b/storage/psql/sqlc.yaml
index 4dbc2de..334bf67 100644
--- a/storage/psql/sqlc.yaml
+++ b/storage/psql/sqlc.yaml
@@ -24,9 +24,3 @@ sql:
             go_type:
               type: "byte"
               slice: true
-          - column: "dep_names.access_token_expiry"
-            go_type:
-              type: "sql.NullString"
-          - column: "dep_names.assigner_profile_uuid_at"
-            go_type:
-              type: "sql.NullString"
diff --git a/storage/psql/sqlc/db.go b/storage/psql/sqlc/db.go
index 2248616..c5852e0 100644
--- a/storage/psql/sqlc/db.go
+++ b/storage/psql/sqlc/db.go
@@ -1,6 +1,6 @@
 // Code generated by sqlc. DO NOT EDIT.
 // versions:
-//   sqlc v1.27.0
+//   sqlc v1.26.0
 
 package sqlc
 
diff --git a/storage/psql/sqlc/models.go b/storage/psql/sqlc/models.go
index a5669e4..b634a2d 100644
--- a/storage/psql/sqlc/models.go
+++ b/storage/psql/sqlc/models.go
@@ -1,6 +1,6 @@
 // Code generated by sqlc. DO NOT EDIT.
 // versions:
-//   sqlc v1.27.0
+//   sqlc v1.26.0
 
 package sqlc
 
@@ -14,7 +14,7 @@ type DepName struct {
 	ConsumerSecret         sql.NullString
 	AccessToken            sql.NullString
 	AccessSecret           sql.NullString
-	AccessTokenExpiry      sql.NullString
+	AccessTokenExpiry      sql.NullTime
 	ConfigBaseUrl          sql.NullString
 	TokenpkiCertPem        []byte
 	TokenpkiKeyPem         []byte
@@ -22,7 +22,7 @@ type DepName struct {
 	TokenpkiStagingKeyPem  []byte
 	SyncerCursor           sql.NullString
 	AssignerProfileUuid    sql.NullString
-	AssignerProfileUuidAt  sql.NullString
+	AssignerProfileUuidAt  sql.NullTime
 	CreatedAt              sql.NullTime
 	UpdatedAt              sql.NullTime
 }
diff --git a/storage/psql/sqlc/query.sql.go b/storage/psql/sqlc/query.sql.go
index 20d369a..0632f5d 100644
--- a/storage/psql/sqlc/query.sql.go
+++ b/storage/psql/sqlc/query.sql.go
@@ -1,6 +1,6 @@
 // Code generated by sqlc. DO NOT EDIT.
 // versions:
-//   sqlc v1.27.0
+//   sqlc v1.26.0
 // source: query.sql
 
 package sqlc
@@ -22,7 +22,7 @@ WHERE
 
 type GetAssignerProfileRow struct {
 	AssignerProfileUuid   sql.NullString
-	AssignerProfileUuidAt sql.NullString
+	AssignerProfileUuidAt sql.NullTime
 }
 
 func (q *Queries) GetAssignerProfile(ctx context.Context, name string) (GetAssignerProfileRow, error) {
@@ -50,7 +50,7 @@ type GetAuthTokensRow struct {
 	ConsumerSecret    sql.NullString
 	AccessToken       sql.NullString
 	AccessSecret      sql.NullString
-	AccessTokenExpiry sql.NullString
+	AccessTokenExpiry sql.NullTime
 }
 
 func (q *Queries) GetAuthTokens(ctx context.Context, name string) (GetAuthTokensRow, error) {
@@ -174,7 +174,7 @@ type StoreAuthTokensParams struct {
 	ConsumerSecret    sql.NullString
 	AccessToken       sql.NullString
 	AccessSecret      sql.NullString
-	AccessTokenExpiry sql.NullString
+	AccessTokenExpiry sql.NullTime
 }
 
 func (q *Queries) StoreAuthTokens(ctx context.Context, arg StoreAuthTokensParams) error {

Copy link
Member

@jessepeterson jessepeterson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See last PR comment. :)

@radiohertz radiohertz force-pushed the dev-pg-backend branch 2 times, most recently from 8091d01 to 9ac2c03 Compare November 13, 2024 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Add PostgreSQL as storage backend
2 participants