Merge pull request #811 from slingamn/history_nits.4

tweaks to persistent history

irc/channel.go

@@ -113,8 +113,8 @@ func (channel *Channel) IsLoaded() bool {
 }
 
 func (channel *Channel) resizeHistory(config *Config) {
-	_, ephemeral, _ := channel.historyStatus(config)
-	if ephemeral {
+	status, _ := channel.historyStatus(config)
+	if status == HistoryEphemeral {
 		channel.history.Resize(config.History.ChannelLength, config.History.AutoresizeWindow)
 	} else {
 		channel.history.Resize(0, 0)
@@ -588,9 +588,9 @@ func (channel *Channel) IsEmpty() bool {
 
 // figure out where history is being stored: persistent, ephemeral, or neither
 // target is only needed if we're doing persistent history
-func (channel *Channel) historyStatus(config *Config) (persistent, ephemeral bool, target string) {
-	if !config.History.Persistent.Enabled {
-		return false, config.History.Enabled, ""
+func (channel *Channel) historyStatus(config *Config) (status HistoryStatus, target string) {
+	if !config.History.Enabled {
+		return HistoryDisabled, ""
 	}
 
 	channel.stateMutex.RLock()
@@ -599,18 +599,17 @@ func (channel *Channel) historyStatus(config *Config) (persistent, ephemeral boo
 	registered := channel.registeredFounder != ""
 	channel.stateMutex.RUnlock()
 
-	historyStatus = historyEnabled(config.History.Persistent.RegisteredChannels, historyStatus)
-
 	// ephemeral history: either the channel owner explicitly set the ephemeral preference,
 	// or persistent history is disabled for unregistered channels
 	if registered {
-		ephemeral = (historyStatus == HistoryEphemeral)
-		persistent = (historyStatus == HistoryPersistent)
+		return historyEnabled(config.History.Persistent.RegisteredChannels, historyStatus), target
 	} else {
-		ephemeral = config.History.Enabled && !config.History.Persistent.UnregisteredChannels
-		persistent = config.History.Persistent.UnregisteredChannels
+		if config.History.Persistent.UnregisteredChannels {
+			return HistoryPersistent, target
+		} else {
+			return HistoryEphemeral, target
+		}
 	}
-	return
 }
 
 func (channel *Channel) AddHistoryItem(item history.Item) (err error) {
@@ -618,14 +617,13 @@ func (channel *Channel) AddHistoryItem(item history.Item) (err error) {
 		return
 	}
 
-	persistent, ephemeral, target := channel.historyStatus(channel.server.Config())
-	if ephemeral {
+	status, target := channel.historyStatus(channel.server.Config())
+	if status == HistoryPersistent {
+		err = channel.server.historyDB.AddChannelItem(target, item)
+	} else if status == HistoryEphemeral {
 		channel.history.Add(item)
 	}
-	if persistent {
-		return channel.server.historyDB.AddChannelItem(target, item)
-	}
-	return nil
+	return
 }
 
 // Join joins the given client to this channel (if they can be joined).

irc/client.go

@@ -354,8 +354,8 @@ func (server *Server) AddAlwaysOnClient(account ClientAccount, chnames []string,
 }
 
 func (client *Client) resizeHistory(config *Config) {
-	_, ephemeral, _ := client.historyStatus(config)
-	if ephemeral {
+	status, _ := client.historyStatus(config)
+	if status == HistoryEphemeral {
 		client.history.Resize(config.History.ClientLength, config.History.AutoresizeWindow)
 	} else {
 		client.history.Resize(0, 0)
@@ -749,16 +749,16 @@ func (session *Session) playResume() {
 		for _, member := range channel.Members() {
 			friends.Add(member)
 		}
-		_, ephemeral, _ := channel.historyStatus(config)
-		if ephemeral {
+		status, _ := channel.historyStatus(config)
+		if status == HistoryEphemeral {
 			lastDiscarded := channel.history.LastDiscarded()
 			if oldestLostMessage.Before(lastDiscarded) {
 				oldestLostMessage = lastDiscarded
 			}
 		}
 	}
-	_, cEphemeral, _ := client.historyStatus(config)
-	if cEphemeral {
+	cHistoryStatus, _ := client.historyStatus(config)
+	if cHistoryStatus == HistoryEphemeral {
 		lastDiscarded := client.history.LastDiscarded()
 		if oldestLostMessage.Before(lastDiscarded) {
 			oldestLostMessage = lastDiscarded
@@ -1551,29 +1551,21 @@ func (client *Client) attemptAutoOper(session *Session) {
 	}
 }
 
-func (client *Client) historyStatus(config *Config) (persistent, ephemeral bool, target string) {
+func (client *Client) historyStatus(config *Config) (status HistoryStatus, target string) {
 	if !config.History.Enabled {
-		return
-	} else if !config.History.Persistent.Enabled {
-		ephemeral = true
-		return
+		return HistoryDisabled, ""
 	}
 
 	client.stateMutex.RLock()
-	alwaysOn := client.alwaysOn
+	loggedIn := client.account != ""
 	historyStatus := client.accountSettings.DMHistory
 	target = client.nickCasefolded
 	client.stateMutex.RUnlock()
 
-	if !alwaysOn {
-		ephemeral = true
-		return
+	if !loggedIn {
+		return HistoryEphemeral, ""
 	}
-
-	historyStatus = historyEnabled(config.History.Persistent.DirectMessages, historyStatus)
-	ephemeral = (historyStatus == HistoryEphemeral)
-	persistent = (historyStatus == HistoryPersistent)
-	return
+	return historyEnabled(config.History.Persistent.DirectMessages, historyStatus), target
 }
 
 // these are bit flags indicating what part of the client status is "dirty"

irc/config.go

@@ -185,25 +185,40 @@ func historyStatusToString(status HistoryStatus) string {
 	}
 }
 
+// XXX you must have already checked History.Enabled before calling this
 func historyEnabled(serverSetting PersistentStatus, localSetting HistoryStatus) (result HistoryStatus) {
-	if serverSetting == PersistentDisabled {
-		return HistoryDisabled
-	} else if serverSetting == PersistentMandatory {
+	switch serverSetting {
+	case PersistentMandatory:
 		return HistoryPersistent
-	} else if serverSetting == PersistentOptOut {
+	case PersistentOptOut:
 		if localSetting == HistoryDefault {
 			return HistoryPersistent
 		} else {
 			return localSetting
 		}
-	} else if serverSetting == PersistentOptIn {
-		if localSetting >= HistoryEphemeral {
-			return localSetting
-		} else {
+	case PersistentOptIn:
+		switch localSetting {
+		case HistoryPersistent:
+			return HistoryPersistent
+		case HistoryEphemeral, HistoryDefault:
+			return HistoryEphemeral
+		default:
 			return HistoryDisabled
 		}
-	} else {
-		return HistoryDisabled
+	case PersistentDisabled:
+		if localSetting == HistoryDisabled {
+			return HistoryDisabled
+		} else {
+			return HistoryEphemeral
+		}
+	default:
+		// PersistentUnspecified: shouldn't happen because the deserializer converts it
+		// to PersistentDisabled
+		if localSetting == HistoryDefault {
+			return HistoryEphemeral
+		} else {
+			return localSetting
+		}
 	}
 }
 

irc/handlers.go

@@ -551,8 +551,15 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.IrcMessage, r
 	if maxChathistoryLimit == 0 {
 		return
 	}
+	preposition := strings.ToLower(msg.Params[0])
+	target = msg.Params[1]
 
 	parseQueryParam := func(param string) (msgid string, timestamp time.Time, err error) {
+		if param == "*" && (preposition == "before" || preposition == "between") {
+			// XXX compatibility with kiwi, which as of February 2020 is
+			// using BEFORE * as a synonym for LATEST *
+			return
+		}
 		err = utils.ErrInvalidParams
 		pieces := strings.SplitN(param, "=", 2)
 		if len(pieces) < 2 {
@@ -580,8 +587,6 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.IrcMessage, r
 		return
 	}
 
-	preposition := strings.ToLower(msg.Params[0])
-	target = msg.Params[1]
 	channel, sequence, err = server.GetHistorySequence(nil, client, target)
 	if err != nil || sequence == nil {
 		return
@@ -1995,17 +2000,19 @@ func dispatchMessageToTarget(client *Client, tags map[string]string, histType hi
 		}
 		targetedItem := item
 		targetedItem.Params[0] = tnick
-		cPersistent, cEphemeral, _ := client.historyStatus(config)
-		tPersistent, tEphemeral, _ := user.historyStatus(config)
+		cStatus, _ := client.historyStatus(config)
+		tStatus, _ := user.historyStatus(config)
 		// add to ephemeral history
-		if cEphemeral {
+		if cStatus == HistoryEphemeral {
 			targetedItem.CfCorrespondent = tDetails.nickCasefolded
 			client.history.Add(targetedItem)
 		}
-		if tEphemeral && client != user {
+		if tStatus == HistoryEphemeral && client != user {
 			item.CfCorrespondent = details.nickCasefolded
 			user.history.Add(item)
 		}
+		cPersistent := cStatus == HistoryPersistent
+		tPersistent := tStatus == HistoryPersistent
 		if cPersistent || tPersistent {
 			item.CfCorrespondent = ""
 			server.historyDB.AddDirectMessage(details.nickCasefolded, user.NickCasefolded(), cPersistent, tPersistent, targetedItem)

irc/server.go

@@ -864,46 +864,52 @@ func (server *Server) setupListeners(config *Config) (err error) {
 // privilege checking.
 func (server *Server) GetHistorySequence(providedChannel *Channel, client *Client, target string) (channel *Channel, sequence history.Sequence, err error) {
 	config := server.Config()
+	// 4 cases: {persistent, ephemeral} x {normal, conversation}
+	// with ephemeral history, recipient is implicit in the choice of `hist`,
+	// and sender is "" if we're retrieving a channel or *, and the correspondent's name
+	// if we're retrieving a DM conversation ("query buffer"). with persistent history,
+	// recipient is always nonempty, and sender is either empty or nonempty as before.
+	var status HistoryStatus
 	var sender, recipient string
 	var hist *history.Buffer
-	if target == "*" {
-		persistent, ephemeral, target := client.historyStatus(config)
-		if persistent {
-			recipient = target
-		} else if ephemeral {
-			hist = &client.history
-		} else {
-			return
-		}
-	} else {
-		channel = providedChannel
-		if channel == nil {
+	channel = providedChannel
+	if channel == nil {
+		if strings.HasPrefix(target, "#") {
 			channel = server.channels.Get(target)
-		}
-		if channel != nil {
-			if !channel.hasClient(client) {
-				err = errInsufficientPrivs
+			if channel == nil {
 				return
 			}
-			persistent, ephemeral, cfTarget := channel.historyStatus(config)
-			if persistent {
-				recipient = cfTarget
-			} else if ephemeral {
-				hist = &channel.history
-			} else {
-				return
-			}
-		} else {
-			sender = client.NickCasefolded()
-			var cfTarget string
-			cfTarget, err = CasefoldName(target)
+		}
+	}
+	if channel != nil {
+		if !channel.hasClient(client) {
+			err = errInsufficientPrivs
+			return
+		}
+		status, recipient = channel.historyStatus(config)
+		switch status {
+		case HistoryEphemeral:
+			hist = &channel.history
+		case HistoryPersistent:
+			// already set `recipient`
+		default:
+			return
+		}
+	} else {
+		status, recipient = client.historyStatus(config)
+		if target != "*" {
+			sender, err = CasefoldName(target)
 			if err != nil {
 				return
 			}
-			recipient = cfTarget
-			if !client.AlwaysOn() {
-				hist = &client.history
-			}
+		}
+		switch status {
+		case HistoryEphemeral:
+			hist = &client.history
+		case HistoryPersistent:
+			// already set `recipient`, and `sender` if necessary
+		default:
+			return
 		}
 	}
 
@@ -921,8 +927,9 @@ func (server *Server) GetHistorySequence(providedChannel *Channel, client *Clien
 	if !cutoff.IsZero() {
 		cutoff = cutoff.Add(-time.Duration(config.History.Restrictions.GracePeriod))
 	}
+
 	if hist != nil {
-		sequence = hist.MakeSequence(recipient, cutoff)
+		sequence = hist.MakeSequence(sender, cutoff)
 	} else if recipient != "" {
 		sequence = server.historyDB.MakeSequence(sender, recipient, cutoff)
 	}