More backends refactoring #71

flashmob · 2017-02-10T04:55:53Z

Edit: deleted comment as no longer relevant! See new comment

…er" and header parser

- Hasher decorator - MySQL decorator also fixes a bug with header parsing Todo: redis decorator, cleanup / remove legacy code, finalise interfaces

- Compressor decorator - Header decorator - Redis decorator Other: Moved events to its own package so that they can be used by backend without getting into circular dep. Guerrilla embeds events directly instead of wrapping the function Todo: , cleanup / remove legacy code, finalise interfaces, change processor initialize to use events. Use a channel for backend errors.

…er and shutdowner

- processor lines can configured via config and built during initialization - backend gateway manages creating of processor lines and starting / stopping workers that use these lines - TODO: lots of cleanup, change to allow multiple rcpt to, more comments, test backend shutdown/restart on config reload

flashmob · 2017-02-14T02:00:04Z

This is a huge... YYYYYUGE... backend refactor!

The main idea is to break backend processing down to individual, reusable components. In a nutshell, you'll now be able to customize your backend by configuration, like so:

"process_line": "HeadersParser|Debugger|Hasher|Header|Compressor|Redis|MySql"

Tokens are separated by a pipe '|'. Each token is a component called a "Processor". When an email is processed, it goes through each component in a line, starting at the left and ending on the right.

How's this implemented?
It's using a decorator pattern found here https://github.com/smaxwellstewart/golang-decorator-pattern
The video was mind-blowing https://www.youtube.com/watch?v=xyDkyFjzFVc

Still need to finalize some things, clean up and determine which interfaces / values need to be public.

- add comments at the start of all processors

- Remove initializers / shutdowners once backend is shutdown - redis processor: propagate close error - mysql processor: propaget close error - add more comments

- Added in type support to Backend's extractConfig func - saveStatus contains queued instead of hash, better semantically - BackendGateway default config if processor_line not present, or save_workers_size not present - added queued id to envelope, a temporary default id is generated too

…reation - Made some symbols public, ie. Service.ExtractConfig, BaseConfig - Envelope: added a NewRreader() function that returns a reader for the processed email - Moved resetting the envelope from client.go to envelope itself. - Added a Reseed() function that seeds the envelope with data at the start of a connection

- rename Service to Svc - rename BackendResult to Result (the package already has the word backend) - rename saveStatus to notifyMsg to better reflect what it does -Rename NewBackendResult to NewResult - add ValidateRcpt method to Backend interface - use private vars where possible - Added the Errors type - Svc.shutdown returns Errors - Svc.initialize returns Errors - renamed savePayload to workerMsg for a more general name - added a new validateRcptChan channel - savedNotify renamed to notifyMe for more fluent code readability - backend timeout now a constant ProcessTimeout - new ValidateRcpt method implimented to add validateRcptChan channel - getNumberOfWorkers() renamed to workersSize() - saveMailWorker() renamed to workDispatcher() so the name better reflects what it does - Change the Processor interface to add a new task param, allows the selecting of what task to do (so far, save or validate) - Update all processors to use new Processor interface - Renamed Envelope.Info to Envelope.Values - New Canned.FailRcptCmd response message for when recipient does not exist - Added PushRcpt and PopRcpt to Envelope

…make sense. *email.Envelope reads better - renamed saveMailChan to conveyor to be more concise - merged validateRcptChan to conveyor - simplified - ProcessorLine renamed ProcessorStack to better describe structure - Default processor now a constant, used when no processor set in config - Renamed Shutdown and Initialize types used for helping to pass closures that meet Shutdowner/Initializer interfaces, improves readability - config extractor now supports omitempty json struct tag - server now checks RCPTO using backend gateway - rename RemoteAddress to RemoteIP for clarity (form a module point of view, it now looks like mail.Address{} instead of envelope.EmailAddress{}) - updated the sample config - backend_name removed from config

- test mysql connection & access to table - guerrilla-redis-db processor bug fix

- rename constants for the mysql processor - update readme

- remove name from nackends.New - Update readme to reflect code

…different processing stack for validating. Better efficiency and ability to control

- update glide - add envelope.go tests

jordanschalm · 2017-03-13T19:02:53Z

.travis.yml

jordanschalm · 2017-03-13T19:04:26Z

api.go

+	"time"
+)
+
+type Daemon struct {


Could add a comment here about how Daemon differs from the Guerrilla struct

jordanschalm · 2017-03-13T19:07:12Z

api.go

+		return errors.New("d.Config nil")
+	}
+	var oldConfig AppConfig
+	oldConfig = *d.Config


can merge lines 130-131

jordanschalm · 2017-03-13T19:15:42Z

api.go

+	if err == nil {
+		l.SetLevel("info")
+	}
+	return l


This should probably do some error checking or return an error. Looks like if an error is returned on line 174 this will return nil and caller will panic. Although this is probably used to frequently for error checking on the caller to be reasonable. Maybe cache a log in the daemon/guerrilla struct when logs are rotated/changed and get that directly afterward. Error check on getting log when rotating, afterward isn't required.

jordanschalm · 2017-03-13T19:25:18Z

backends/gateway.go

+		return NewResult(response.Canned.FailBackendNotRunning + gw.State.String())
+	}
+	// place on the channel so that one of the save mail workers can pick it up
+	savedNotify := make(chan *notifyMsg)


Considering how frequently we're calling Process and ValidateRcpt, these notifyMsg channels might be a good candidate for pooling

jordanschalm · 2017-03-13T20:03:40Z

config.go

+		sc.Hostname = h
+		sc.MaxClients = 100
+		sc.Timeout = 30
+		sc.MaxSize = 10 << 20 // 10 Mebibytes


typo: Megabytes

jordanschalm · 2017-03-13T20:04:23Z

config.go

+				c.Servers[i].Timeout = 20
+			}
+			if c.Servers[i].MaxSize == 0 {
+				c.Servers[i].MaxSize = 10 << 20 // 10 Mebibytes


typo: megabytes

jordanschalm · 2017-03-13T20:09:48Z

guerrilla.go

 	}
 	return nil
 }

 func (g *guerrilla) Shutdown() {
+
+	// shot down the servers first


typo: shut down

jordanschalm · 2017-03-13T20:11:33Z

mail/envelope.go

+	"time"
+)
+
+const maxHeaderChunk = iota + (3 << 10) // 3KB


Why use iota here instead of just 1?

jordanschalm · 2017-03-13T20:16:12Z

server.go

@@ -136,6 +137,19 @@ func (s *server) configureLog() {
 	}
 }

+// setBackend Sets the backend to use for processing email envelopes


flashmob · 2017-03-14T20:20:59Z

Thanks @jordanschalm for the review - will be going through these today.

flashmob · 2017-03-15T13:30:01Z

@jordanschalm still going through it (and fixing problems), marking the ones that are fixed. Just want to say thanks again. Your reviews are very thorough with lots of attention to detail! Will have the new README ready for review soon too.

flashmob · 2017-03-16T01:27:12Z

Worked on adding the pooling of notifyMsg chans. Now working on another optimization: In ValidateRcpt, don't need to place on the channel if we know that there were no validators set in the config.

…tors configured - fix issues found during code review on PR #71 - fix bug with server ignoring the -v flag - default config now named as 'goguerrilla.conf.json' for syntax highlighting - changed api so AppConfig is passed by value, so the caller can't modify it / prevent using it as global state value - Safeguard api functions from calling out of order (check for nil) - removed CmdConfig

flashmob · 2017-03-16T13:01:26Z

@jordanschalm Here is the new README ready for review - https://github.com/flashmob/go-guerrilla/blob/more-backends-refactoring/README.md - things to check: typos, grammar & flow. Also feedback on if it's using the right words to describe things. Thanks!

- fix race condition with the client pool

jordanschalm

This looks good to me. The README was getting pretty long, and I think this makes it a lot easier to get into. Ordering of topics is good. Intro->Features->Standalone->Package->Backend/Processors. Maybe could move the Starting/Usage and Using NGINX as a Proxy to where you first introduce the daemon in Getting Started as subsections or relegate them to the wiki. Besides that noticed a couple of typos.

jordanschalm · 2017-03-18T02:47:30Z

README.md

 ====================

-An minimalist SMTP server written in Go, made for receiving large volumes of mail.
+An lightweight & modern SMTP server written in Go, made for receiving large volumes of mail.


A lightweight

jordanschalm · 2017-03-18T02:50:03Z

README.md

-### What is Go Guerrilla SMTPd?
+### What is Go-Guerrilla?
+
+It's an SMTP written in Go, for the purpose of receiving large volume of email.


an SMTP server

jordanschalm · 2017-03-18T02:58:55Z

README.md

-                        add_header Auth-Server 127.0.0.1;
-                        add_header Auth-Port 2525;
-                    }
+The main job of a go-guerrilla backend is to validate recipients and deliver emails. The term


Depending on how pedantic you want to be, could standardize the capitalization of "Go-Guerrilla". The README has Go-guerrilla, Go-Guerrilla, and go-guerrilla ;-)

jordanschalm · 2017-03-18T03:08:18Z

README.md

-### What is Go Guerrilla SMTPd?
+### What is Go-Guerrilla?
+
+It's an SMTP written in Go, for the purpose of receiving large volume of email.


"receiving large volumes" or "receiving a large volume"

…patcher can be re-entered

flashmob · 2017-03-20T23:56:33Z

@dvcrn @peterkrejci @mrcpvn @eliezedeck
Doing some final testing on this PR.
If all good - to be merged to master today!

dvcrn · 2017-03-21T01:30:42Z

@flashmob amazing 😄 fantastic job!

flashmob added 8 commits February 9, 2017 23:02

add decorator pattern

dd5094c

Merge branch 'master' into more-backends-refactoring

20cf073

dummy backend uses the decorator pattern with two decorators: "debugg…

6476239

…er" and header parser

dummy backend uses the decorator pattern with two decorators: "debugg…

44ad353

…er" and header parser

more progress with decorators:

b19bac2

- Hasher decorator - MySQL decorator also fixes a bug with header parsing Todo: redis decorator, cleanup / remove legacy code, finalise interfaces

refactored Processor initialization and shutdown: now using initializ…

87c442a

…er and shutdowner

flashmob added 21 commits February 14, 2017 15:00

- redis: when not compressed, delivery header was not added

e2d7375

- add comments at the start of all processors

- synchronise BackendService

91d4e66

- Remove initializers / shutdowners once backend is shutdown - redis processor: propagate close error - mysql processor: propaget close error - add more comments

- backend logger refactoring

0c84f5e

forgot to set the log level

35f069b

add error checking for backend processor initialization

0d1e35c

trim whitespace from user part

7dbe397

rename process_line to process_stack to better reflect name

0c160b6

add old backend compatibility

1d59402

old version of guerrilla-db-redis, to remove

5764ae4

ported guerrilla_db_redis to new backend system

9c09c57

remove old deprecated backend system

60494c1

return ErrProcessorNotFound error if procesor not found

2f62ce7

update readme to include info about the new backend system

98b297c

- debugger processor name, make case insensitive

0966eaa

- test mysql connection & access to table - guerrilla-redis-db processor bug fix

- tweak debug messages

bc52714

- rename constants for the mysql processor - update readme

- clean up comments

83f98cc

- remove name from nackends.New - Update readme to reflect code

fix broken build

224edf4

flashmob added 2 commits March 11, 2017 02:54

add testrace to makefile

a452c06

update sample config

ef7202c

flashmob mentioned this pull request Mar 10, 2017

add support for wildcards in allowed_hosts, add tests for allowsHost() #73

Closed

flashmob added 3 commits March 11, 2017 19:16

separate process stack for validating recipients - it means it use a …

29db21a

…different processing stack for validating. Better efficiency and ability to control

- remove /sloonz/go-qprintable and replace with go stdlib equivalent

b532c91

- update glide - add envelope.go tests

Readme update: edit description and add features list

e82cf80

jordanschalm reviewed Mar 13, 2017

View reviewed changes

flashmob added 2 commits March 16, 2017 16:21

new Readme

78f13b2

- fix race conditions with log handling

ca6924e

- fix race condition with the client pool

jordanschalm reviewed Mar 18, 2017

View reviewed changes

flashmob added 9 commits March 18, 2017 18:40

edit & fix readme after review by @jordanschalm

63fcd92

tidy up Redis and MySQL processors

9df7fca

add goguerrilla.conf.json to gitignore

8534c5b

resolve readme merge conflict

c86d966

envelope: reset headers and values after each transaction

809342e

add additional columns to mysql processor

cdd2038

delete config

e655604

update gitignore

0638131

added a panic recovery mechanism for when a processor panics, the dis…

592dead

…patcher can be re-entered

update TODOs

dd4f0a8

flashmob merged commit 14a83f6 into master Mar 21, 2017

flashmob deleted the more-backends-refactoring branch March 21, 2017 06:39

flashmob mentioned this pull request Mar 21, 2017

backends: Backend interface should not have private methods #60

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More backends refactoring #71

More backends refactoring #71

flashmob commented Feb 10, 2017 •

edited

Loading

flashmob commented Feb 14, 2017 •

edited

Loading

jordanschalm Mar 13, 2017

jordanschalm Mar 13, 2017

flashmob Mar 14, 2017

jordanschalm Mar 13, 2017

jordanschalm Mar 13, 2017

jordanschalm Mar 13, 2017

jordanschalm Mar 13, 2017

jordanschalm Mar 13, 2017

jordanschalm Mar 13, 2017

jordanschalm Mar 13, 2017

jordanschalm Mar 13, 2017

flashmob commented Mar 14, 2017

flashmob commented Mar 15, 2017

flashmob commented Mar 16, 2017

flashmob commented Mar 16, 2017

jordanschalm left a comment

jordanschalm Mar 18, 2017

jordanschalm Mar 18, 2017

jordanschalm Mar 18, 2017

jordanschalm Mar 18, 2017

flashmob commented Mar 20, 2017

dvcrn commented Mar 21, 2017 •

edited

Loading

More backends refactoring #71

More backends refactoring #71

Conversation

flashmob commented Feb 10, 2017 • edited Loading

flashmob commented Feb 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flashmob commented Mar 14, 2017

flashmob commented Mar 15, 2017

flashmob commented Mar 16, 2017

flashmob commented Mar 16, 2017

jordanschalm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flashmob commented Mar 20, 2017

dvcrn commented Mar 21, 2017 • edited Loading

flashmob commented Feb 10, 2017 •

edited

Loading

flashmob commented Feb 14, 2017 •

edited

Loading

dvcrn commented Mar 21, 2017 •

edited

Loading