Decouple web from App implementation (#364)

* Decouple app from web

* Simplify

* Fix test

* Encapsulate SSL logic to dbconn

* Fix test

* Fix email type

pkg/server/.env.dev

@@ -5,6 +5,7 @@ DBPort=5432
 DBName=dnote
 DBUser=postgres
 DBPassword=
+DBSkipSSL=true
 
 SmtpUsername=mock-SmtpUsername
 SmtpPassword=mock-SmtpPassword

pkg/server/.env.test

@@ -5,6 +5,7 @@ DBPort=5432
 DBName=dnote_test
 DBUser=postgres
 DBPassword=
+DBSkipSSL=true
 
 SmtpUsername=mock-SmtpUsername
 SmtpPassword=mock-SmtpPassword

pkg/server/app/notes.go

@@ -161,13 +161,13 @@ func (a *App) DeleteNote(tx *gorm.DB, user database.User, note database.Note) (d
 }
 
 // GetNote retrieves a note for the given user
-func (a *App) GetNote(uuid string, user database.User) (database.Note, bool, error) {
+func GetNote(db *gorm.DB, uuid string, user database.User) (database.Note, bool, error) {
 	zeroNote := database.Note{}
 	if !helpers.ValidateUUID(uuid) {
 		return zeroNote, false, nil
 	}
 
-	conn := a.DB.Where("notes.uuid = ? AND deleted = ?", uuid, false)
+	conn := db.Where("notes.uuid = ? AND deleted = ?", uuid, false)
 	conn = database.PreloadNote(conn)
 
 	var note database.Note

pkg/server/app/notes_test.go

@@ -341,7 +341,7 @@ func TestGetNote(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			a := NewTest(nil)
-			note, ok, err := a.GetNote(tc.note.UUID, tc.user)
+			note, ok, err := GetNote(a.DB, tc.note.UUID, tc.user)
 			if err != nil {
 				t.Fatal(errors.Wrap(err, "executing"))
 			}
@@ -376,7 +376,7 @@ func TestGetNote_nonexistent(t *testing.T) {
 
 	a := NewTest(nil)
 	nonexistentUUID := "4fd19336-671e-4ff3-8f22-662b80e22edd"
-	note, ok, err := a.GetNote(nonexistentUUID, user)
+	note, ok, err := GetNote(a.DB, nonexistentUUID, user)
 	if err != nil {
 		t.Fatal(errors.Wrap(err, "executing"))
 	}

pkg/server/dbconn/dbconn.go

@@ -20,6 +20,7 @@ package dbconn
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/jinzhu/gorm"
 	"github.com/pkg/errors"
@@ -27,7 +28,6 @@ import (
 
 // Config holds the connection configuration
 type Config struct {
-	SkipSSL  bool
 	Host     string
 	Port     string
 	Name     string
@@ -64,13 +64,27 @@ func validateConfig(c Config) error {
 	return nil
 }
 
+// checkSSLMode checks if SSL is required for the database connection
+func checkSSLMode() bool {
+	// TODO: deprecate DB_NOSSL in favor of DBSkipSSL
+	if os.Getenv("DB_NOSSL") != "" {
+		return true
+	}
+
+	if os.Getenv("DBSkipSSL") == "true" {
+		return true
+	}
+
+	return os.Getenv("GO_ENV") != "PRODUCTION"
+}
+
 func getPGConnectionString(c Config) (string, error) {
 	if err := validateConfig(c); err != nil {
 		return "", errors.Wrap(err, "invalid database config")
 	}
 
 	var sslmode string
-	if c.SkipSSL {
+	if checkSSLMode() {
 		sslmode = "disable"
 	} else {
 		sslmode = "require"

pkg/server/dbconn/dbconn_test.go

@@ -40,7 +40,6 @@ func TestValidateConfig(t *testing.T) {
 		},
 		{
 			input: Config{
-				SkipSSL:  true,
 				Host:     "mockHost",
 				Port:     "mockPort",
 				Name:     "mockName",

pkg/server/handlers/notes.go

@@ -26,6 +26,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/dnote/dnote/pkg/server/app"
 	"github.com/dnote/dnote/pkg/server/database"
 	"github.com/dnote/dnote/pkg/server/helpers"
 	"github.com/dnote/dnote/pkg/server/presenters"
@@ -108,7 +109,7 @@ func (a *API) getNote(w http.ResponseWriter, r *http.Request) {
 	vars := mux.Vars(r)
 	noteUUID := vars["noteUUID"]
 
-	note, ok, err := a.App.GetNote(noteUUID, user)
+	note, ok, err := app.GetNote(a.App.DB, noteUUID, user)
 	if !ok {
 		RespondNotFound(w)
 		return

pkg/server/job/repetition/repetition.go

@@ -209,6 +209,9 @@ func process(p Params, now time.Time, rule database.RepetitionRule) error {
 	tx := p.DB.Begin()
 
 	if !checkCooldown(now, rule) {
+		log.WithFields(log.Fields{
+			"uuid": rule.UUID,
+		}).Info("Skipping repetition processing due to cooldown")
 		return nil
 	}
 

pkg/server/mailer/backend.go

@@ -48,22 +48,13 @@ type dialerParams struct {
 	Password string
 }
 
-func validateSMTPConfig() bool {
-	port := os.Getenv("SmtpPort")
-	host := os.Getenv("SmtpHost")
-	username := os.Getenv("SmtpUsername")
-	password := os.Getenv("SmtpPassword")
-
-	return port != "" && host != "" && username != "" && password != ""
-}
-
 func getSMTPParams() (*dialerParams, error) {
 	portEnv := os.Getenv("SmtpPort")
 	hostEnv := os.Getenv("SmtpHost")
 	usernameEnv := os.Getenv("SmtpUsername")
 	passwordEnv := os.Getenv("SmtpPassword")
 
-	if portEnv != "" && hostEnv != "" && usernameEnv != "" && passwordEnv != "" {
+	if portEnv == "" || hostEnv == "" || usernameEnv == "" || passwordEnv == "" {
 		return nil, ErrSMTPNotConfigured
 	}
 
@@ -106,7 +97,7 @@ func (b *SimpleBackendImplementation) Queue(subject, from string, to []string, c
 
 	d := gomail.NewPlainDialer(p.Host, p.Port, p.Username, p.Password)
 	if err := d.DialAndSend(m); err != nil {
-		return err
+		return errors.Wrap(err, "dialing and sending email")
 	}
 
 	return nil

pkg/server/mailer/mailer.go

@@ -44,9 +44,9 @@ var (
 
 var (
 	// EmailKindHTML is the type of html email
-	EmailKindHTML = "html"
+	EmailKindHTML = "text/html"
 	// EmailKindHTML is the type of text email
-	EmailKindText = "text"
+	EmailKindText = "text/plain"
 )
 
 // template is the common interface shared between Template from

pkg/server/main.go

@@ -56,11 +56,11 @@ func mustFind(box *packr.Box, path string) []byte {
 	return b
 }
 
-func initContext(a *app.App) web.Context {
+func initWebContext(db *gorm.DB) web.Context {
 	staticBox := packr.New("static", "../../web/public/static")
 
 	return web.Context{
-		App:              a,
+		DB:               db,
 		IndexHTML:        mustFind(rootBox, "index.html"),
 		RobotsTxt:        mustFind(rootBox, "robots.txt"),
 		ServiceWorkerJs:  mustFind(rootBox, "service-worker.js"),
@@ -75,7 +75,7 @@ func initServer(a app.App) (*http.ServeMux, error) {
 		return nil, errors.Wrap(err, "initializing router")
 	}
 
-	webCtx := initContext(&a)
+	webCtx := initWebContext(a.DB)
 	webHandlers, err := web.Init(webCtx)
 	if err != nil {
 		return nil, errors.Wrap(err, "initializing web handlers")
@@ -92,15 +92,7 @@ func initServer(a app.App) (*http.ServeMux, error) {
 }
 
 func initDB() *gorm.DB {
-	var skipSSL bool
-	if os.Getenv("GO_ENV") != "PRODUCTION" || os.Getenv("DB_NOSSL") != "" || os.Getenv("DBSkipSSL") == "true" {
-		skipSSL = true
-	} else {
-		skipSSL = false
-	}
-
 	db := dbconn.Open(dbconn.Config{
-		SkipSSL:  skipSSL,
 		Host:     os.Getenv("DBHost"),
 		Port:     os.Getenv("DBPort"),
 		Name:     os.Getenv("DBName"),

pkg/server/testutils/main.go

@@ -51,7 +51,6 @@ var DB *gorm.DB
 // the environment variable configuration and initalizes a new schema
 func InitTestDB() {
 	db := dbconn.Open(dbconn.Config{
-		SkipSSL:  true,
 		Host:     os.Getenv("DBHost"),
 		Port:     os.Getenv("DBPort"),
 		Name:     os.Getenv("DBName"),

pkg/server/tmpl/app.go

@@ -24,7 +24,7 @@ import (
 	"net/http"
 	"regexp"
 
-	"github.com/dnote/dnote/pkg/server/app"
+	"github.com/jinzhu/gorm"
 	"github.com/pkg/errors"
 )
 
@@ -37,15 +37,15 @@ var templateNoteMetaTags = "note_metatags"
 
 // AppShell represents the application in HTML
 type AppShell struct {
-	App *app.App
-	T   *template.Template
+	DB *gorm.DB
+	T  *template.Template
 }
 
 // ErrNotFound is an error indicating that a resource was not found
 var ErrNotFound = errors.New("not found")
 
 // NewAppShell parses the templates for the application
-func NewAppShell(a *app.App, content []byte) (AppShell, error) {
+func NewAppShell(db *gorm.DB, content []byte) (AppShell, error) {
 	t, err := template.New(templateIndex).Parse(string(content))
 	if err != nil {
 		return AppShell{}, errors.Wrap(err, "parsing the index template")
@@ -56,7 +56,7 @@ func NewAppShell(a *app.App, content []byte) (AppShell, error) {
 		return AppShell{}, errors.Wrap(err, "parsing the note meta tags template")
 	}
 
-	return AppShell{App: a, T: t}, nil
+	return AppShell{DB: db, T: t}, nil
 }
 
 // Execute executes the index template

pkg/server/tmpl/app_test.go

@@ -24,17 +24,14 @@ import (
 	"testing"
 
 	"github.com/dnote/dnote/pkg/assert"
-	"github.com/dnote/dnote/pkg/server/app"
 	"github.com/dnote/dnote/pkg/server/database"
 	"github.com/dnote/dnote/pkg/server/testutils"
 	"github.com/pkg/errors"
 )
 
 func TestAppShellExecute(t *testing.T) {
-	testApp := app.NewTest(nil)
-
 	t.Run("home", func(t *testing.T) {
-		a, err := NewAppShell(&testApp, []byte("<head><title>{{ .Title }}</title>{{ .MetaTags }}</head>"))
+		a, err := NewAppShell(testutils.DB, []byte("<head><title>{{ .Title }}</title>{{ .MetaTags }}</head>"))
 		if err != nil {
 			t.Fatal(errors.Wrap(err, "preparing app shell"))
 		}
@@ -69,7 +66,7 @@ func TestAppShellExecute(t *testing.T) {
 		}
 		testutils.MustExec(t, testutils.DB.Save(&n1), "preparing note")
 
-		a, err := NewAppShell(&testApp, []byte("{{ .MetaTags }}"))
+		a, err := NewAppShell(testutils.DB, []byte("{{ .MetaTags }}"))
 		if err != nil {
 			t.Fatal(errors.Wrap(err, "preparing app shell"))
 		}

pkg/server/tmpl/data.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/dnote/dnote/pkg/server/app"
 	"github.com/dnote/dnote/pkg/server/database"
 	"github.com/dnote/dnote/pkg/server/handlers"
 	"github.com/pkg/errors"
@@ -51,12 +52,12 @@ type notePage struct {
 }
 
 func (a AppShell) newNotePage(r *http.Request, noteUUID string) (notePage, error) {
-	user, _, err := handlers.AuthWithSession(a.App.DB, r, nil)
+	user, _, err := handlers.AuthWithSession(a.DB, r, nil)
 	if err != nil {
 		return notePage{}, errors.Wrap(err, "authenticating with session")
 	}
 
-	note, ok, err := a.App.GetNote(noteUUID, user)
+	note, ok, err := app.GetNote(a.DB, noteUUID, user)
 
 	if !ok {
 		return notePage{}, ErrNotFound

pkg/server/tmpl/data_test.go

@@ -24,8 +24,8 @@ import (
 	"time"
 
 	"github.com/dnote/dnote/pkg/assert"
-	"github.com/dnote/dnote/pkg/server/app"
 	"github.com/dnote/dnote/pkg/server/database"
+	"github.com/dnote/dnote/pkg/server/testutils"
 	"github.com/pkg/errors"
 )
 
@@ -39,8 +39,7 @@ func TestDefaultPageGetData(t *testing.T) {
 }
 
 func TestNotePageGetData(t *testing.T) {
-	testApp := app.NewTest(nil)
-	a, err := NewAppShell(&testApp, nil)
+	a, err := NewAppShell(testutils.DB, nil)
 	if err != nil {
 		t.Fatal(errors.Wrap(err, "preparing app shell"))
 	}

pkg/server/web/handlers.go

@@ -22,13 +22,15 @@ package web
 import (
 	"net/http"
 
-	"github.com/dnote/dnote/pkg/server/app"
 	"github.com/dnote/dnote/pkg/server/handlers"
 	"github.com/dnote/dnote/pkg/server/tmpl"
+	"github.com/jinzhu/gorm"
 	"github.com/pkg/errors"
 )
 
 var (
+	// ErrEmptyDatabase is an error for missing db in the context
+	ErrEmptyDatabase = errors.New("No DB was provided")
 	// ErrEmptyIndexHTML is an error for missing index.html content in the context
 	ErrEmptyIndexHTML = errors.New("No index.html content was provided")
 	// ErrEmptyRobotsTxt is an error for missing robots.txt content in the context
@@ -41,7 +43,7 @@ var (
 
 // Context contains contents of web assets
 type Context struct {
-	App              *app.App
+	DB               *gorm.DB
 	IndexHTML        []byte
 	RobotsTxt        []byte
 	ServiceWorkerJs  []byte
@@ -57,8 +59,8 @@ type Handlers struct {
 }
 
 func validateContext(c Context) error {
-	if err := c.App.Validate(); err != nil {
-		return errors.Wrap(err, "validating app")
+	if c.DB == nil {
+		return ErrEmptyDatabase
 	}
 	if c.IndexHTML == nil {
 		return ErrEmptyIndexHTML
@@ -92,7 +94,7 @@ func Init(c Context) (Handlers, error) {
 
 // getRootHandler returns an HTTP handler that serves the app shell
 func getRootHandler(c Context) http.HandlerFunc {
-	appShell, err := tmpl.NewAppShell(c.App, c.IndexHTML)
+	appShell, err := tmpl.NewAppShell(c.DB, c.IndexHTML)
 	if err != nil {
 		panic(errors.Wrap(err, "initializing app shell"))
 	}

pkg/server/web/handlers_test.go

@@ -24,7 +24,7 @@ import (
 	"testing"
 
 	"github.com/dnote/dnote/pkg/assert"
-	"github.com/dnote/dnote/pkg/server/app"
+	"github.com/dnote/dnote/pkg/server/testutils"
 	"github.com/pkg/errors"
 )
 
@@ -34,17 +34,13 @@ func TestInit(t *testing.T) {
 	mockServiceWorkerJs := []byte("function() {}")
 	mockStaticFileSystem := http.Dir(".")
 
-	testApp := app.NewTest(nil)
-	testAppNoDB := app.NewTest(nil)
-	testAppNoDB.DB = nil
-
 	testCases := []struct {
 		ctx         Context
 		expectedErr error
 	}{
 		{
 			ctx: Context{
-				App:              &testApp,
+				DB:               testutils.DB,
 				IndexHTML:        mockIndexHTML,
 				RobotsTxt:        mockRobotsTxt,
 				ServiceWorkerJs:  mockServiceWorkerJs,
@@ -54,17 +50,17 @@ func TestInit(t *testing.T) {
 		},
 		{
 			ctx: Context{
-				App:              &testAppNoDB,
+				DB:               nil,
 				IndexHTML:        mockIndexHTML,
 				RobotsTxt:        mockRobotsTxt,
 				ServiceWorkerJs:  mockServiceWorkerJs,
 				StaticFileSystem: mockStaticFileSystem,
 			},
-			expectedErr: app.ErrEmptyDB,
+			expectedErr: ErrEmptyDatabase,
 		},
 		{
 			ctx: Context{
-				App:              &testApp,
+				DB:               testutils.DB,
 				IndexHTML:        nil,
 				RobotsTxt:        mockRobotsTxt,
 				ServiceWorkerJs:  mockServiceWorkerJs,
@@ -74,7 +70,7 @@ func TestInit(t *testing.T) {
 		},
 		{
 			ctx: Context{
-				App:              &testApp,
+				DB:               testutils.DB,
 				IndexHTML:        mockIndexHTML,
 				RobotsTxt:        nil,
 				ServiceWorkerJs:  mockServiceWorkerJs,
@@ -84,7 +80,7 @@ func TestInit(t *testing.T) {
 		},
 		{
 			ctx: Context{
-				App:              &testApp,
+				DB:               testutils.DB,
 				IndexHTML:        mockIndexHTML,
 				RobotsTxt:        mockRobotsTxt,
 				ServiceWorkerJs:  nil,
@@ -94,7 +90,7 @@ func TestInit(t *testing.T) {
 		},
 		{
 			ctx: Context{
-				App:              &testApp,
+				DB:               testutils.DB,
 				IndexHTML:        mockIndexHTML,
 				RobotsTxt:        mockRobotsTxt,
 				ServiceWorkerJs:  mockServiceWorkerJs,