[CODE REVIEW]
hi, is there a better way than to INSERT once then SELECT again... any way to do it in 1 db call ?
#SQL better implementation
rocky cave
func generateIdentifier(length int) string {
rand.New(rand.NewSource(time.Now().UnixNano()))
b := make([]byte, length)
for i := range b {
b[i] = alphanumericChars[rand.Intn(len(alphanumericChars))]
}
return string(b)
}
func CreateFile(
name string,
path string,
createdAt time.Time,
expiresAt time.Time,
email string,
) (structures.File, error) {
db, err := sql.Open("sqlite", "./data/arcfile.db")
if err != nil {
panic(err)
}
defer db.Close()
var identifier string
for {
identifier = generateIdentifier(6)
var count int
err = db.QueryRow("SELECT COUNT(*) FROM files WHERE identifier = ?", identifier).Scan(&count)
if err != nil {
log.Fatalln("error getting identifier count:", err)
return structures.File{}, err
}
if count == 0 {
break // unique
}
}
query := "INSERT INTO files (identifier, name, path, created_at, expires_at, email) VALUES (?, ?, ?, ?, ?, ?)"
result, err := db.Exec(query, identifier, name, path, createdAt, expiresAt, email)
if err != nil {
log.Fatalln("error inserting file:", err)
return structures.File{}, err
}
lastInsertID, err := result.LastInsertId()
if err != nil {
log.Fatalln("error getting last insert ID:", err)
return structures.File{}, err
}
var file structures.File
err = db.QueryRow("SELECT id, identifier, name, path, created_at, expires_at, email FROM files WHERE id = ?", lastInsertID).Scan(
&file.ID,
&file.Identifier,
&file.Filename,
&file.Path,
&file.CreatedAt,
&file.ExpiresAt,
&file.Email,
)
if err != nil {
log.Fatalln("error fetching inserted file:", err)
return structures.File{}, err
}
return file, nil
}
use returning in your insert?
ah this is new , thanks ! ill look into it
rocky cave
use returning in your insert?
Everyday, i learn something new, thanks very much it worked
deft hollyBOT
If your question has been solved, please close the thread with </solved:1022173889255706726>
tough solstice
```go
func generateIdentifier(length int) string {
rand.New(rand.NewSource(t...
Also, instead of checking for identifier collisions, just use a unique/primary index. If you try the insert, you can check the error that comes back, and if it's a duplicate key error, try again with a different identifier. This way, best case you're sending one query to the database, instead of two in your case. You also avoid the race condition doing it this way. With yours, there's an astronomically tiny chance that two separate requests could create a file with the same identifier.
Which database driver are you using for sqlite?
zinc condor
```go
func generateIdentifier(length int) string {
rand.New(rand.NewSource(t...
There is nothing auto filling (except the ID, but you get it from the insert result.) Why don't just construct the structure use the user's input?
rocky cave
Which database driver are you using for sqlite?
So i took all of the suggestioons in and here's what I am at now
package database
import (
"database/sql"
"log"
"math/rand"
"time"
"github.com/nxrmqlly/arcfile-backend/structures"
_ "modernc.org/sqlite"
)
const alphanumericChars = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"
// generates a random identifier for the file
func generateIdentifier(length int) string {
rand.New(rand.NewSource(time.Now().UnixNano()))
b := make([]byte, length)
for i := range b {
b[i] = alphanumericChars[rand.Intn(len(alphanumericChars))]
}
return string(b)
}
// creates a new file entry in database
func CreateFile(
name string,
uuid string,
createdAt time.Time,
expiresAt time.Time,
email string,
) (structures.File, error) {
db, err := sql.Open("sqlite", "./data/arcfile.db")
if err != nil {
panic(err)
}
defer db.Close()
var identifier string
for {
identifier = generateIdentifier(6)
var count int
err = db.QueryRow("SELECT COUNT(*) FROM files WHERE identifier = ?", identifier).Scan(&count)
if err != nil {
log.Fatalln("error getting identifier count:", err)
return structures.File{}, err
}
if count == 0 {
break // unique
}
}
query := `INSERT INTO files
(identifier, filename, uuid, created_at, expires_at, email)
VALUES (?, ?, ?, ?, ?, ?)
RETURNING id, identifier, filename, uuid, created_at, expires_at, email;`
var file structures.File
err = db.QueryRow(
query,
identifier,
name,
uuid,
createdAt,
expiresAt,
email,
).Scan(
&file.ID,
&file.Identifier,
&file.Filename,
&file.UUID,
&file.CreatedAt,
&file.ExpiresAt,
&file.Email,
)
if err != nil {
log.Println("error creating file record:", err)
return structures.File{}, err
}
return file, nil
}
I will have to restructure my DB to use the Identifier as the Primary key, that'll simplify things a lot
tough solstice
Also, instead of checking for identifier collisions, just use a unique/primary i...
Don't forget this one. This could have a pretty significant performance impact when under load.
rocky cave
Don't forget this one. This could have a pretty significant performance impact w...
yup implemented
I have another question:
How do i use the DB exported here
//storage/init.go
package storage
import (
"database/sql"
"os"
)
var DB *sql.DB
func InitDatabase() {
//ensure dir exisits
if err := os.MkdirAll("./data", os.ModePerm); err != nil {
panic(err)
}
DB, err := sql.Open("sqlite", "./data/arcfile.db")
if err != nil {
panic(err)
}
defer DB.Close()
createTable := `
CREATE TABLE IF NOT EXISTS files (
identifier TEXT PRIMARY KEY,
filename TEXT NOT NULL,
uuid TEXT NOT NULL,
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
expires_at DATETIME NOT NULL,
email TEXT NOT NULL
);`
_, err = DB.Exec(createTable)
if err != nil {
panic(err)
}
}
in another file like
//storage/get.go
package storage
import (
"fmt"
"github.com/nxrmqlly/arcfile-backend/structures"
_ "modernc.org/sqlite"
)
type FileNotFoundError struct {
identifier string
}
func (e *FileNotFoundError) Error() string {
return fmt.Sprintf("file not found with identifier: %s", e.identifier)
}
func GetFile(identifier string) (structures.File, error) {
query := "SELECT * FROM files WHERE identifier = ?"
rows, err := DB.Query(query, identifier)
if err != nil {
return structures.File{}, &FileNotFoundError{identifier}
}
var File = structures.File{}
rows.Next()
if err := rows.Scan(
&File.Identifier,
&File.Filename,
&File.UUID,
&File.CreatedAt,
&File.ExpiresAt,
&File.Email,
); err != nil {
return structures.File{}, err
}
return File, nil
}
the current implentaion panics
error here: fixed
tough solstice
I have another question:
How do i use the DB exported here
```go
//storage/init....
Don't do this. It's bad practice. Instead of having a global variable DB, you should pass your *sql.DB around to anything that needs it.
rocky cave
what is the best methoud abou this
tough solstice
You can use a method of dependency injection to achieve that. My goto for this would be a struct that contains methods that need database access. For example
func InitDatabase() (*sql.DB, error) {
// ensure dir exisits
if err := os.MkdirAll("./data", os.ModePerm); err != nil {
return nil, fmt.Errorf("create data dir: %w", err)
}
db, err := sql.Open("sqlite", "./data/arcfile.db")
if err != nil {
return nil, fmt.Errorf("open db: %w", err)
}
defer db.Close()
createTable := `
CREATE TABLE IF NOT EXISTS files (
identifier TEXT PRIMARY KEY,
filename TEXT NOT NULL,
uuid TEXT NOT NULL,
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
expires_at DATETIME NOT NULL,
email TEXT NOT NULL
);`
_, err = db.Exec(createTable)
if err != nil {
return nil, fmt.Errorf("create table: %w", err)
}
return db, nil
}
type Repository struct {
db *sql.DB
}
func NewRepository(db *sql.DB) *Repository {
return &Repository{db: db}
}
func (r *Repository) GetFile(ctx context.Context, identifier string) (structures.File, error) {
query := "SELECT * FROM files WHERE identifier = ?"
var file structures.File
if err := r.db.QueryRowContext(ctx, query, identifier).Scan(
&file.Identifier,
&file.Filename,
&file.UUID,
&file.CreatedAt,
&file.ExpiresAt,
&file.Email,
); err != nil {
return structures.File{}, err
}
return file, nil
}
func main() {
db, err := InitDatabase()
if err != nil {
log.Fatalf("init db: %v", err)
}
repo := NewRepository(db)
file, err := repo.GetFile(context.Background(), "123")
if err != nil {
log.Fatalf("get file: %v", err)
}
}
You should be using context.Context for database access, so that any cancellation is propagated, and you don't unnecessarily waste database resources when a user cancels their query.
context.Background() is fine here because quitting the program will kill the connection.
If you're working on the context of a http.Handler, you can get the request's context from (*http.Request).Context()
Minimal example of that
func main() {
db, err := InitDatabase()
if err != nil {
log.Fatalf("init db: %v", err)
}
repo := NewRepository(db)
getFileHandler := func(w http.ResponseWriter, r *http.Request) {
file, err := repo.GetFile(r.Context(), r.PathValue("id"))
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
fmt.Fprintf(w, "file: %+v", file)
}
http.HandleFunc("/file/{id}", getFileHandler)
if err := http.ListenAndServe(":8080", nil); err != nil {
log.Fatalf("listen and serve: %v", err)
}
}
this is cool, ill look into this im using the gin package
tough solstice
Yeah, the concepts transfer pretty well. Gin has a way of getting the request context from the gin context, I believe
rocky cave
current file structure
tough solstice
But yeah, generally you want to avoid using globals at all costs. They make it harder for you to trace the dependencies through your application. You rely entirely on your IDE telling you what uses a given global variable. With dependency injection, it's clear what parts of your application require other parts of your application. It's also easier to test because you can accept interfaces as dependencies, and mock those out in your unit tests.
For example
type dbConn interface {
QueryRowContext(ctx context.Context, query string, args ...interface{}) *sql.Row
}
type Repository struct {
db dbConn
}
func NewRepository(db dbConn) *Repository {
return &Repository{db: db}
}
Now it's even clearer which methods of the *sql.DB your Repository depends on. You can then use something like sqlmock
rocky cave
For example
```go
type dbConn interface {
QueryRowContext(ctx context.Contex...
i didnt understand this part
tough solstice
Do you know what interfaces are?
tough solstice
So, an interface is like a contract. Here, the dbConn interface states that in order for a type to be classed as a dbConn, it must have the QueryRowContext method, exactly as it's stated there, with those exact arguments and return types.
tough solstice
theyre the classes equivalent in other languages right
No, other languages have interfaces
rocky cave
oh this is new
tough solstice
Structs are more analogous to classes in other languages
rocky cave
So, an interface is like a contract. Here, the `dbConn` interface states that in...
so, interfaces are declarations of methods in a struct
tough solstice
oh this is new
So, the *sql.DB type has the QueryRowContext method, with those exact arguments and types, so it can be used in place of something expecting a dbConn type.
tough solstice
so, interfaces are declarations of methods in a struct
Interfaces are commonly just a set of methods
In this case it's just a single method
So long as a type has all of those methods, it implements that interface.
Take a look at this, to get a better understanding of interfaces
package main
import "fmt"
type Animal interface {
Speak() string
}
func AnimalSpeak(a Animal) string {
return a.Speak()
}
type Dog struct {
Breed string
}
func (d Dog) Speak() string {
switch d.Breed {
case "poodle":
return "arf!"
default:
return "woof!"
}
}
type Cat struct{}
func (c Cat) Speak() string {
return "Meow!"
}
func main() {
animals := []Animal{
Dog{Breed: "poodle"},
Dog{Breed: "bulldog"},
Cat{},
}
for _, animal := range animals {
fmt.Println(AnimalSpeak(animal))
}
}
Both the Cat and Dog types implement the Animal interface, because they both have the method Speak() string
rocky cave
Minimal example of that
```go
func main() {
db, err := InitDatabase()
if...
how do i use this repo var in other files
tough solstice
how do i use this `repo` var in other files
You need to pass it between your types/functions using dependency injection. Instead of "using it in files", the types and function defined in those files should accept a *sql.DB as a dependency/argument
You will likely end up with your main() function just wiring everything together
rocky cave
well, if i do InitDatabase in main() how do i pass repo.db to storage.GetFile?
tough solstice
GetFile should be a method on the Repository type. They both need to be in the same package for you to be able to do that
Repository should have methods which access the database
You could move Repository and NewRepository to the storage package
tough solstice
Perfect!
So now, any time you want to use the CreateFile function anywhere, you need to pass around the *Repository type
Then you just call repo.CreateFile(ctx, ...)
rocky cave
but then it means i need my repository (repo) to be available to my handlers/ (package handlers)
because thats where its called
tough solstice
Yes, so you can do the same exact thing, using dependency injection
type Handlers struct {
repo *storage.Repository
}
func NewHandlers(repo *storage.Repository) *Handlers {
return &Handlers{repo: repo}
}
func (h *Handlers) GetFile(w http.ResponseWriter, r *http.Request)
Or alternatively, if Repository is the only dependency of your handlers, or you have only one or two extra dependencies, you can use another form of dependency injection which takes advantage of closures
func GetFile(repo *storage.Repository) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
...
}
}
rocky cave
```go
type Handlers struct {
repo *storage.Repository
}
func NewHandlers(re...
ah this is neat.
tough solstice
Likewise, the same concepts apply using Gin, just your method function signature will look a bit different
So you might have
func (h *Handlers) Upload(c *gin.Context) {
...
if err := h.repo.CreateFile(&file); err != nil { ... }
}
so like this (main file)
tough solstice
Or
func Upload(repo *storage.Repository) gin.HandlerFunc {
return func(c *gin.Context) { ... }
}
What is handlers?
rocky cave
What is `handlers`?
package
tough solstice
Yeah, kinda
// Note that we're doing something called "shadowing" here. We're creating a variable called "handlers", which shadows the package "handlers". So, from this point on, any time you reference "handlers", you're referencing the variable we just created, instead of the package with the same name "handlers"
handlers := handlers.New(repo)
router.Post("/api/upload", handlers.Upload)
I've removed some repeated words to make it read a bit nicer.
rocky cave
exactly what i was trying to say, thanks let me try it out
tough solstice
handlers.UploadHandler is unnecessarily long. You don't need the word "Handler" in there twice
Likewise handlers.NewHandlers could just be handlers.New
rocky cave
Likewise `handlers.NewHandlers` could just be `handlers.New`
Cool, did tat!
so when i use QueryContext/QueryRowContext, do i pass in a *gin.Context or a gin.Context.Reuest.Context() of type context.Context
do i pass in the entirety of c or just c.Result.Context()
tough solstice
If memory serves, I think *gin.Context implements the context.Context interface, so you should be able to just pass c
rocky cave
If memory serves, I think `*gin.Context` implements the `context.Context` inter...
so better practice is c
tough solstice
Let me double check
tough solstice
so better practice is `c`
Yes, use *gin.Context
// Err returns nil when c.Request has no Context.
func (c *Context) Err() error {
if !c.hasRequestContext() {
return nil
}
return c.Request.Context().Err()
}
Look like it uses c.Request.Context() if it has one
Either will work, honestly. c is just more concise than c.Request.Context()
rocky cave
Either will work, honestly. `c` is just more concise than `c.Request.Context()`
im somehow again getting a nil pointer dereference
2025/01/28 19:14:41 [Recovery] 2025/01/28 - 19:14:41 panic recovered:
GET /api/file/8QHW4L HTTP/1.1
Host: localhost:8080
Accept: */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
User-Agent: python-requests/2.32.3
runtime error: invalid memory address or nil pointer dereference
C:/Program Files/Go/src/runtime/panic.go:262 (0xfdd3f7)
panicmem: panic(memoryError)
C:/Program Files/Go/src/runtime/signal_windows.go:401 (0xfdd3c7)
sigpanic: panicmem()
C:/Users/Ritam Das/Documents/Projects/apis/arcfile-backend/handlers/info.go:31 (0x150d5d0)
(*Handlers).FileInfo: "message": err.Error(),
C:/Users/Ritam Das/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:185 (0x13144ce)
(*Context).Next: c.handlers[c.index](c)
C:/Users/Ritam Das/go/pkg/mod/github.com/gin-gonic/[email protected]/recovery.go:102 (0x13144bb)
CustomRecoveryWithWriter.func1: c.Next()
C:/Users/Ritam Das/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:185 (0x1313604)
(*Context).Next: c.handlers[c.index](c)
C:/Users/Ritam Das/go/pkg/mod/github.com/gin-gonic/[email protected]/logger.go:249 (0x13135eb)
LoggerWithConfig.func1: c.Next()
C:/Users/Ritam Das/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:185 (0x1312a31)
(*Context).Next: c.handlers[c.index](c)
C:/Users/Ritam Das/go/pkg/mod/github.com/gin-gonic/[email protected]/gin.go:633 (0x13124a0)
(*Engine).handleHTTPRequest: c.Next()
C:/Users/Ritam Das/go/pkg/mod/github.com/gin-gonic/[email protected]/gin.go:589 (0x1312131)
(*Engine).ServeHTTP: engine.handleHTTPRequest(c)
C:/Program Files/Go/src/net/http/server.go:3210 (0x120dler.ServeHTTP(rw, req)
C:/Program Files/Go/src/net/http/server.go:2092 (0x11f7ecf)
(*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
C:/Program Files/Go/src/runtime/asm_amd64.s:1700 (0x1003880)
goexit: BYTE $0x90 // NOP
// GET /api/file/:identifier
func (h *Handlers) FileInfo(c *gin.Context) {
identifier := c.Param("identifier")
log.Println("identifier:", identifier)
file, err := h.repo.GetFile(c, identifier)
var ae *storage.FileNotFoundError
if errors.As(err, &ae) {
c.JSON(http.StatusNotFound, gin.H{
"code": http.StatusNotFound,
"message": err.Error(),
})
return
} else {
// some other internal error
c.JSON(http.StatusInternalServerError, gin.H{
"code": http.StatusInternalServerError,
"message": err.Error(),
})
}
c.JSON(http.StatusOK, gin.H{
"code": http.StatusOK,
"message": "file found",
"data": gin.H{
"identifier": file.Identifier,
"filename": file.Filename,
"uuid": file.UUID,
"created_at": file.CreatedAt,
"expires_at": file.ExpiresAt,
"email": file.Email,
},
})
}