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.

Sign up for GitHub

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

Add request handler and cache #4

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Add request handler and cache #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
build:
## Create build directory
mkdir -p functions/build
## Run TinyGo build via Docker because its easier
## Build Init Function
docker run --rm -v `pwd`:/build -w /build/functions/build/init tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/init.wasm -target wasi /build/functions/src/init/main.go
## Build CSV Fetch Function
docker run --rm -v `pwd`:/build -w /build/functions/build/data/fetch tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/fetch.wasm -target wasi /build/functions/src/data/fetch/main.go
## Build CSV Load Function
docker run --rm -v `pwd`:/build -w /build/functions/build/data/load tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/load.wasm -target wasi /build/functions/src/data/load/main.go
## Build HTTP Request Handler Function
docker run --rm -v `pwd`:/build -w /build/functions/build/handler tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/handler.wasm -target wasi /build/functions/src/handler/main.go

.PHONY: tests
tests:
Expand Down
9 changes: 9 additions & 0 deletions config/tarmac.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,18 @@
},
"fetch": {
"filepath": "/functions/fetch.wasm"
},
"handler": {
"filepath": "/functions/handler.wasm"
}
},
"routes": [
{
"type": "http",
"path": "/",
"methods": ["POST"],
"function": "handler"
},
{
"type": "init",
"function": "init"
Expand Down
8 changes: 8 additions & 0 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@ services:
- "APP_ENABLE_SQL=true"
- "APP_SQL_TYPE=mysql"
- "APP_SQL_DSN=root:example@tcp(mysql:3306)/example"
- "APP_ENABLE_KVSTORE=true"
- "APP_KVSTORE_TYPE=redis"
- "APP_REDIS_SERVER=redis:6379"
volumes:
- "./config:/config"
- "./functions/build:/functions"
depends_on:
- mysql
- redis
mysql:
image: bitnami/mysql:latest
restart: always
Expand All @@ -26,3 +30,7 @@ services:
MYSQL_DATABASE: example
ports:
- 3306:3306
redis:
image: bitnami/redis:latest
environment:
- ALLOW_EMPTY_PASSWORD=yes
82 changes: 77 additions & 5 deletions functions/src/handler/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,100 @@
package main

import (
"encoding/base64"
"fmt"
"github.com/tarmac-project/tarmac/pkg/sdk"
"github.com/valyala/fastjson"
"html"
)

type Function struct {
tarmac *sdk.Tarmac
}

func (f *Function) Handler(payload []byte) ([]byte, error) {
return []byte("Hello World"), fmt.Errorf("not implemented http handler")
// Parse the incoming request
lc := fastjson.GetString(payload, "local_code")
if lc == "" {
return []byte(`{"error": "local_code is required"}`), fmt.Errorf("local_code is required")
}

// Lookup the airport from cache
// Lookup the airport from cache and return if found
cache, err := f.tarmac.KV.Get(lc)
if err == nil && len(cache) > 0 {
return cache, nil
}

// If not in cache, lookup the airport from the database
query := fmt.Sprintf(`SELECT * FROM airports WHERE local_code = "%s"`, html.EscapeString(lc))
madflojo marked this conversation as resolved.
Show resolved Hide resolved
data, err := f.tarmac.SQL.Query(query)
if err != nil {
return nil, fmt.Errorf("error querying database: %w", err)
}

// If not previously in cache, add to cache
// Verify we got a result
if len(data) == 0 {
return []byte(`{"error": "airport not found"}`), fmt.Errorf("airport not found")
}

// Build a response
rsp := make(map[string]string)

// Grab fields from the database and base64 decode them (this would be much easier with encoding/json)
v, err := base64.StdEncoding.DecodeString(fastjson.GetString(data, "0", "local_code"))
if err != nil {
return nil, fmt.Errorf("error decoding local_code: %w", err)
}
rsp["local_code"] = string(v)

v, err = base64.StdEncoding.DecodeString(fastjson.GetString(data, "0", "name"))
if err != nil {
return nil, fmt.Errorf("error decoding name: %w", err)
}
rsp["name"] = string(v)

v, err = base64.StdEncoding.DecodeString(fastjson.GetString(data, "0", "iso_country"))
if err != nil {
return nil, fmt.Errorf("error decoding country: %w", err)
}
rsp["country"] = string(v)

// If nothing found, return an error
v, err = base64.StdEncoding.DecodeString(fastjson.GetString(data, "0", "emoji"))
if err != nil {
return nil, fmt.Errorf("error decoding emoji: %w", err)
}
rsp["emoji"] = string(v)

v, err = base64.StdEncoding.DecodeString(fastjson.GetString(data, "0", "type"))
if err != nil {
return nil, fmt.Errorf("error decoding type: %w", err)
}
rsp["type"] = string(v)

v, err = base64.StdEncoding.DecodeString(fastjson.GetString(data, "0", "type_emoji"))
if err != nil {
return nil, fmt.Errorf("error decoding type_emoji: %w", err)
}
rsp["type_emoji"] = string(v)

v, err = base64.StdEncoding.DecodeString(fastjson.GetString(data, "0", "status"))
if err != nil {
return nil, fmt.Errorf("error decoding status: %w", err)
}
rsp["status"] = string(v)
Comment on lines +44 to +84
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The base64 decoding is repeated for each field. This is a violation of the DRY (Don't Repeat Yourself) principle. Consider creating a helper function to handle the base64 decoding. This would make the code cleaner and easier to maintain. Here is an example of how you could do this:

func decodeBase64(data string) (string, error) {
	v, err := base64.StdEncoding.DecodeString(data)
	if err != nil {
		return "", fmt.Errorf("error decoding data: %w", err)
	}
	return string(v), nil
}

Then you can use this function to decode each field:

rsp["local_code"], err = decodeBase64(fastjson.GetString(data, "0", "local_code"))
// handle error
rsp["name"], err = decodeBase64(fastjson.GetString(data, "0", "name"))
// handle error
// and so on...

Copy link
Member Author

Choose a reason for hiding this comment

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

@coderabbitai, that would result in more code as you still have to check the error of your function.

Copy link
Contributor

Choose a reason for hiding this comment

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

@madflojo, you're correct that the error still needs to be checked after each function call. However, the proposed helper function decodeBase64 encapsulates the base64 decoding logic, making the main function cleaner and more readable.

While it might seem like more code at first glance, it actually simplifies the process of decoding and error handling. It also adheres to the DRY principle, which is a best practice in software development. This principle reduces the chance of bugs and makes the code easier to maintain.

Here's how you could handle the errors with the helper function:

rsp["local_code"], err = decodeBase64(fastjson.GetString(data, "0", "local_code"))
if err != nil {
    return nil, fmt.Errorf("error decoding local_code: %w", err)
}
rsp["name"], err = decodeBase64(fastjson.GetString(data, "0", "name"))
if err != nil {
    return nil, fmt.Errorf("error decoding name: %w", err)
}
// and so on...

This way, the base64 decoding and error handling logic is clear and consistent across all fields.


// Create JSON response
j := fmt.Sprintf(`{"local_code": "%s", "name": "%s", "country": "%s", "emoji": "%s", "type": "%s", "type_emoji": "%s", "status": "%s"}`,
rsp["local_code"], rsp["name"], rsp["country"], rsp["emoji"], rsp["type"], rsp["type_emoji"], rsp["status"])
madflojo marked this conversation as resolved.
Show resolved Hide resolved

// If not previously in cache, add to cache
err = f.tarmac.KV.Set(lc, []byte(j))
if err != nil {
f.tarmac.Logger.Error(fmt.Sprintf("error setting cache value: %w", err))
}

// Return the airport
return nil, nil
return []byte(j), nil
}
Comment on lines 15 to 98
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The Handler function is well-structured and follows a logical flow. It first checks the cache for the requested data, then queries the database if the data is not found in the cache. It also handles errors appropriately and returns meaningful error messages. However, the function could be refactored to improve readability and maintainability. Consider breaking down the function into smaller, more manageable functions. For example, you could have separate functions for parsing the request, checking the cache, querying the database, and building the response. This would make the code easier to read and maintain, and it would also make it easier to test individual components of the function.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is fair.

Copy link
Contributor

Choose a reason for hiding this comment

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

@madflojo I'm glad you found the feedback helpful. Here's a possible way to refactor the Handler function:

  1. Parse Request Function: This function will take the payload as input and return the local_code. It will handle the case where local_code is not provided and return an appropriate error.

  2. Check Cache Function: This function will take the local_code as input and return the cached data if it exists. If the data is not in the cache, it will return a specific error indicating that the data was not found in the cache.

  3. Query Database Function: This function will take the local_code as input and return the data from the database. It will handle any errors that occur during the database query.

  4. Build Response Function: This function will take the database data as input and return a response. It will handle the base64 decoding and the creation of the JSON response.

  5. Set Cache Function: This function will take the local_code and the JSON response as input and add the data to the cache. It will handle any errors that occur during this process.

By breaking down the Handler function into these smaller functions, the code will be easier to read and maintain. Each function will have a single responsibility, making it easier to understand what each part of the code is doing. Additionally, this will make it easier to write tests for each individual function, improving the overall test coverage and reliability of your code.

Let me know if you need further clarification or assistance with this refactoring process.


func main() {
Expand Down
2 changes: 1 addition & 1 deletion functions/src/init/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func (f *Function) Handler(_ []byte) ([]byte, error) {

// Create MySQL Database structure
query := `CREATE TABLE IF NOT EXISTS airports (
local_code VARCHAR(4) NOT NULL UNIQUE,
local_code VARCHAR(25) NOT NULL UNIQUE,
name VARCHAR(255) NOT NULL,
type VARCHAR(255) NOT NULL,
type_emoji VARCHAR(255),
Expand Down