Add DisableRegLog configuration setting.

Unit tests for DisablePostIP. Awkward but does the job for now.

common/misc_logs.go

@@ -4,7 +4,7 @@ import (
 	"database/sql"
 	"time"
 
-	"github.com/Azareal/Gosora/query_gen"
+	qgen "github.com/Azareal/Gosora/query_gen"
 )
 
 var RegLogs RegLogStore
@@ -16,7 +16,7 @@ type RegLogItem struct {
 	Email         string
 	FailureReason string
 	Success       bool
-	IP     string
+	IP            string
 	DoneAt        string
 }
 
@@ -81,7 +81,7 @@ func (s *SQLRegLogStore) Count() (count int) {
 	return count
 }
 
-func (s *SQLRegLogStore) GetOffset(offset int, perPage int) (logs []RegLogItem, err error) {
+func (s *SQLRegLogStore) GetOffset(offset, perPage int) (logs []RegLogItem, err error) {
 	rows, err := s.getOffset.Query(offset, perPage)
 	if err != nil {
 		return logs, err
@@ -102,11 +102,11 @@ func (s *SQLRegLogStore) GetOffset(offset int, perPage int) (logs []RegLogItem,
 }
 
 type LoginLogItem struct {
-	ID        int
+	ID      int
-	UID       int
+	UID     int
-	Success   bool
+	Success bool
-	IP string
+	IP      string
-	DoneAt    string
+	DoneAt  string
 }
 
 type LoginLogStmts struct {

common/site.go

@@ -99,6 +99,7 @@ type config struct {
 	DisableLastIP bool
 	DisablePostIP bool
 	DisablePollIP bool
+	DisableRegLog bool
 
 	DisableLiveTopicList bool
 	DisableJSAntispam    bool

misc_test.go

@@ -449,12 +449,19 @@ func TestTopicStore(t *testing.T) {
 	tcache := c.NewMemoryTopicCache(c.Config.TopicCacheCapacity)
 	c.Topics, err = c.NewDefaultTopicStore(tcache)
 	expectNilErr(t, err)
-	topicStoreTest(t, 2)
+	c.Config.DisablePostIP = false
+	topicStoreTest(t, 2, "::1")
+	c.Config.DisablePostIP = true
+	topicStoreTest(t, 3, "0")
+
 	c.Topics, err = c.NewDefaultTopicStore(nil)
 	expectNilErr(t, err)
-	topicStoreTest(t, 3)
+	c.Config.DisablePostIP = false
+	topicStoreTest(t, 4, "::1")
+	c.Config.DisablePostIP = true
+	topicStoreTest(t, 5, "0")
 }
-func topicStoreTest(t *testing.T, newID int) {
+func topicStoreTest(t *testing.T, newID int, ip string) {
 	var topic *c.Topic
 	var err error
 
@@ -469,18 +476,15 @@ func topicStoreTest(t *testing.T, newID int) {
 
 	// TODO: Add BulkGetMap() to the TopicStore
 
-	ok := c.Topics.Exists(-1)
+	expect(t, !c.Topics.Exists(-1), "TID #-1 shouldn't exist")
-	expect(t, !ok, "TID #-1 shouldn't exist")
+	expect(t, !c.Topics.Exists(0), "TID #0 shouldn't exist")
-	ok = c.Topics.Exists(0)
+	expect(t, c.Topics.Exists(1), "TID #1 should exist")
-	expect(t, !ok, "TID #0 shouldn't exist")
-	ok = c.Topics.Exists(1)
-	expect(t, ok, "TID #1 should exist")
 
 	count := c.Topics.Count()
 	expect(t, count == 1, fmt.Sprintf("Global count for topics should be 1, not %d", count))
 
 	//Create(fid int, topicName string, content string, uid int, ip string) (tid int, err error)
-	tid, err := c.Topics.Create(2, "Test Topic", "Topic Content", 1, "::1")
+	tid, err := c.Topics.Create(2, "Test Topic", "Topic Content", 1, ip)
 	expectNilErr(t, err)
 	expect(t, tid == newID, fmt.Sprintf("TID for the new topic should be %d, not %d", newID, tid))
 	expect(t, c.Topics.Exists(newID), fmt.Sprintf("TID #%d should exist", newID))
@@ -522,27 +526,27 @@ func topicStoreTest(t *testing.T, newID int) {
 		expectNilErr(t, err)
 	}
 
-	testTopic(newID, "Test Topic", "Topic Content", 1, "::1", 2, false, false)
+	testTopic(newID, "Test Topic", "Topic Content", 1, ip, 2, false, false)
 
 	expectNilErr(t, topic.Lock())
 	shouldNotBeIn(newID)
-	testTopic(newID, "Test Topic", "Topic Content", 1, "::1", 2, true, false)
+	testTopic(newID, "Test Topic", "Topic Content", 1, ip, 2, true, false)
 
 	expectNilErr(t, topic.Unlock())
 	shouldNotBeIn(newID)
-	testTopic(newID, "Test Topic", "Topic Content", 1, "::1", 2, false, false)
+	testTopic(newID, "Test Topic", "Topic Content", 1, ip, 2, false, false)
 
 	expectNilErr(t, topic.Stick())
 	shouldNotBeIn(newID)
-	testTopic(newID, "Test Topic", "Topic Content", 1, "::1", 2, false, true)
+	testTopic(newID, "Test Topic", "Topic Content", 1, ip, 2, false, true)
 
 	expectNilErr(t, topic.Unstick())
 	shouldNotBeIn(newID)
-	testTopic(newID, "Test Topic", "Topic Content", 1, "::1", 2, false, false)
+	testTopic(newID, "Test Topic", "Topic Content", 1, ip, 2, false, false)
 
 	expectNilErr(t, topic.MoveTo(1))
 	shouldNotBeIn(newID)
-	testTopic(newID, "Test Topic", "Topic Content", 1, "::1", 1, false, false)
+	testTopic(newID, "Test Topic", "Topic Content", 1, ip, 1, false, false)
 	// TODO: Add more tests for more *Topic methods
 
 	expectNilErr(t, topic.Delete())
@@ -907,6 +911,13 @@ func TestReplyStore(t *testing.T) {
 	_, err = c.Rstore.Get(0)
 	recordMustNotExist(t, err, "RID #0 shouldn't exist")
 
+	c.Config.DisablePostIP = false
+	testReplyStore(t, 2, 1, "::1")
+	c.Config.DisablePostIP = true
+	testReplyStore(t, 5, 3, "0")
+}
+
+func testReplyStore(t *testing.T, newID int, newPostCount int, ip string) {
 	replyTest2 := func(reply *c.Reply, err error, rid int, parentID int, createdBy int, content string, ip string) {
 		expectNilErr(t, err)
 		expect(t, reply.ID == rid, fmt.Sprintf("RID #%d has the wrong ID. It should be %d not %d", rid, rid, reply.ID))
@@ -928,42 +939,42 @@ func TestReplyStore(t *testing.T) {
 	//_, err = c.Rstore.GetCache().Get(1)
 	//recordMustNotExist(t, err, "RID #1 shouldn't be in the cache")
 
-	_, err = c.Rstore.Get(2)
+	_, err := c.Rstore.Get(newID)
 	recordMustNotExist(t, err, "RID #2 shouldn't exist")
 
 	topic, err := c.Topics.Get(1)
 	expectNilErr(t, err)
-	expect(t, topic.PostCount == 1, fmt.Sprintf("TID #1's post count should be one, not %d", topic.PostCount))
+	expect(t, topic.PostCount == newPostCount, fmt.Sprintf("TID #%d's post count should be %d, not %d", topic.ID, newPostCount, topic.PostCount))
 
-	_, err = c.Rstore.GetCache().Get(2)
+	_, err = c.Rstore.GetCache().Get(newID)
-	recordMustNotExist(t, err, "RID #2 shouldn't be in the cache")
+	recordMustNotExist(t, err, "RID #%d shouldn't be in the cache", newID)
 
-	rid, err := c.Rstore.Create(topic, "Fofofo", "::1", 1)
+	rid, err := c.Rstore.Create(topic, "Fofofo", ip, 1)
 	expectNilErr(t, err)
-	expect(t, rid == 2, fmt.Sprintf("The next reply ID should be 2 not %d", rid))
+	expect(t, rid == newID, fmt.Sprintf("The next reply ID should be %d not %d", newID, rid))
-	expect(t, topic.PostCount == 1, fmt.Sprintf("The old TID #1 in memory's post count should be one, not %d", topic.PostCount))
+	expect(t, topic.PostCount == newPostCount, fmt.Sprintf("The old TID #1 in memory's post count should be %d, not %d", newPostCount+1, topic.PostCount))
 	// TODO: Test the reply count on the topic
 
-	replyTest(2, 1, 1, "Fofofo", "::1")
+	replyTest(newID, 1, 1, "Fofofo", ip)
 
 	topic, err = c.Topics.Get(1)
 	expectNilErr(t, err)
-	expect(t, topic.PostCount == 2, fmt.Sprintf("TID #1's post count should be two, not %d", topic.PostCount))
+	expect(t, topic.PostCount == newPostCount+1, fmt.Sprintf("TID #1's post count should be %d, not %d", newPostCount+1, topic.PostCount))
 
-	err = topic.CreateActionReply("destroy", "::1", 1)
+	err = topic.CreateActionReply("destroy", ip, 1)
 	expectNilErr(t, err)
-	expect(t, topic.PostCount == 2, fmt.Sprintf("The old TID #1 in memory's post count should be two, not %d", topic.PostCount))
+	expect(t, topic.PostCount == newPostCount+1, fmt.Sprintf("The old TID #1 in memory's post count should be %d, not %d", newPostCount+1, topic.PostCount))
-	replyTest(3, 1, 1, "", "::1")
+	replyTest(newID+1, 1, 1, "", ip)
 	// TODO: Check the actionType field of the reply, this might not be loaded by TopicStore, maybe we should add it there?
 
 	topic, err = c.Topics.Get(1)
 	expectNilErr(t, err)
-	expect(t, topic.PostCount == 3, fmt.Sprintf("TID #1's post count should be three, not %d", topic.PostCount))
+	expect(t, topic.PostCount == newPostCount+2, fmt.Sprintf("TID #1's post count should be %d, not %d", newPostCount+2, topic.PostCount))
 
 	// TODO: Expand upon this
-	rid, err = c.Rstore.Create(topic, "hiii", "::1", 1)
+	rid, err = c.Rstore.Create(topic, "hiii", ip, 1)
 	expectNilErr(t, err)
-	replyTest(rid, topic.ID, 1, "hiii", "::1")
+	replyTest(rid, topic.ID, 1, "hiii", ip)
 
 	reply, err := c.Rstore.Get(rid)
 	expectNilErr(t, err)
@@ -1001,28 +1012,34 @@ func TestProfileReplyStore(t *testing.T) {
 	_, err = c.Prstore.Get(1)
 	recordMustNotExist(t, err, "PRID #1 shouldn't exist")
 
+	c.Config.DisablePostIP = false
+	testProfileReplyStore(t, 1, "::1")
+	c.Config.DisablePostIP = true
+	testProfileReplyStore(t, 2, "0")
+}
+func testProfileReplyStore(t *testing.T, newID int, ip string) {
 	// ? - Commented this one out as strong constraints like this put an unreasonable load on the database, we only want errors if a delete which should succeed fails
 	//profileReply := c.BlankProfileReply(1)
 	//err = profileReply.Delete()
 	//expect(t,err != nil,"You shouldn't be able to delete profile replies which don't exist")
 
 	profileID := 1
-	prid, err := c.Prstore.Create(profileID, "Haha", 1, "::1")
+	prid, err := c.Prstore.Create(profileID, "Haha", 1, ip)
 	expectNilErr(t, err)
-	expect(t, prid == 1, "The first profile reply should have an ID of 1")
+	expect(t, prid == newID, fmt.Sprintf("The first profile reply should have an ID of %d", newID))
 
-	profileReply, err := c.Prstore.Get(1)
+	profileReply, err := c.Prstore.Get(newID)
 	expectNilErr(t, err)
-	expect(t, profileReply.ID == 1, fmt.Sprintf("The profile reply should have an ID of 1 not %d", profileReply.ID))
+	expect(t, profileReply.ID == newID, fmt.Sprintf("The profile reply should have an ID of %d not %d", newID, profileReply.ID))
 	expect(t, profileReply.ParentID == 1, fmt.Sprintf("The parent ID of the profile reply should be 1 not %d", profileReply.ParentID))
 	expect(t, profileReply.Content == "Haha", fmt.Sprintf("The profile reply's contents should be 'Haha' not '%s'", profileReply.Content))
 	expect(t, profileReply.CreatedBy == 1, fmt.Sprintf("The profile reply's creator should be 1 not %d", profileReply.CreatedBy))
-	expect(t, profileReply.IP == "::1", fmt.Sprintf("The profile reply's IP should be '::1' not '%s'", profileReply.IP))
+	expect(t, profileReply.IP == ip, fmt.Sprintf("The profile reply's IP should be '%s' not '%s'", ip, profileReply.IP))
 
 	err = profileReply.Delete()
 	expectNilErr(t, err)
-	_, err = c.Prstore.Get(1)
+	_, err = c.Prstore.Get(newID)
-	expect(t, err != nil, "PRID #1 shouldn't exist after being deleted")
+	expect(t, err != nil, fmt.Sprintf("PRID #%d shouldn't exist after being deleted", newID))
 
 	// TODO: Test profileReply.SetBody() and profileReply.Creator()
 }
@@ -1447,7 +1464,7 @@ func TestWidgets(t *testing.T) {
 	ewidget := &c.WidgetEdit{widget, map[string]string{"Name": "Test", "Text": "Testing"}}
 	wid, err := ewidget.Create()
 	expectNilErr(t, err)
-	expect(t,wid==1,"wid should be 1")
+	expect(t, wid == 1, "wid should be 1")
 
 	// TODO: Do a test for the widget body
 	widget2, err := c.Widgets.Get(1)

routes/account.go

@@ -262,9 +262,11 @@ func AccountRegisterSubmit(w http.ResponseWriter, r *http.Request, user c.User)
 	}
 
 	regLog := c.RegLogItem{Username: name, Email: email, FailureReason: regErrReason, Success: regSuccess, IP: user.GetIP()}
-	_, err = regLog.Create()
+	if !c.Config.DisableRegLog {
-	if err != nil {
+		_, err := regLog.Create()
-		return c.InternalError(err, w, r)
+		if err != nil {
+			return c.InternalError(err, w, r)
+		}
 	}
 	if !regSuccess {
 		return c.LocalError(regErrMsg, w, r, user)
@@ -280,27 +282,32 @@ func AccountRegisterSubmit(w http.ResponseWriter, r *http.Request, user c.User)
 		group = c.Config.ActivationGroup
 	}
 
+	addReason := func(reason string) error {
+		if c.Config.DisableRegLog {
+			return nil
+		}
+		regLog.FailureReason += reason
+		return regLog.Commit()
+	}
+
 	// TODO: Do the registration attempt logging a little less messily (without having to amend the result after the insert)
 	uid, err := c.Users.Create(name, password, email, group, active)
 	if err != nil {
 		regLog.Success = false
 		if err == c.ErrAccountExists {
-			regLog.FailureReason += "username-exists"
+			err = addReason("username-exists")
-			err = regLog.Commit()
 			if err != nil {
 				return c.InternalError(err, w, r)
 			}
 			return c.LocalError(p.GetErrorPhrase("register_username_unavailable"), w, r, user)
 		} else if err == c.ErrLongUsername {
-			regLog.FailureReason += "username-too-long"
+			err = addReason("username-too-long")
-			err = regLog.Commit()
 			if err != nil {
 				return c.InternalError(err, w, r)
 			}
 			return c.LocalError(p.GetErrorPhrase("register_username_too_long_prefix")+strconv.Itoa(c.Config.MaxUsernameLength), w, r, user)
 		}
-		regLog.FailureReason += "internal-error"
+		err2 := addReason("internal-error")
-		err2 := regLog.Commit()
 		if err2 != nil {
 			return c.InternalError(err2, w, r)
 		}

tickloop.go

@@ -170,6 +170,12 @@ func asmMatches() {
 func dailies() {
 	asmMatches()
 
+	if c.Config.DisableRegLog {
+		_, err := qgen.NewAcc().Purge("registration_logs").Exec()
+		if err != nil {
+			c.LogError(err)
+		}
+	}
 	if c.Config.LogPruneCutoff > -1 {
 		f := func(tbl string) {
 			_, err := qgen.NewAcc().Delete(tbl).DateOlderThan("doneAt", c.Config.LogPruneCutoff, "day").Run()