#3281 fix x.Iterate returns nothing inside session scope with SQLite3

README.md

@@ -3,7 +3,7 @@ Gogs - Go Git Service [![Build Status](https://travis-ci.org/gogits/gogs.svg?bra
 
 ![](https://github.com/gogits/gogs/blob/master/public/img/gogs-large-resize.png?raw=true)
 
-##### Current tip version: 0.9.56 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions)
+##### Current tip version: 0.9.57 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions)
 
 | Web | UI  | Preview  |
 |:-------------:|:-------:|:-------:|

gogs.go

@@ -17,7 +17,7 @@ import (
 	"github.com/gogits/gogs/modules/setting"
 )
 
-const APP_VER = "0.9.56.0726"
+const APP_VER = "0.9.57.0726"
 
 func init() {
 	runtime.GOMAXPROCS(runtime.NumCPU())

models/models.go

@@ -27,9 +27,10 @@ type Engine interface {
 	Exec(string, ...interface{}) (sql.Result, error)
 	Find(interface{}, ...interface{}) error
 	Get(interface{}) (bool, error)
+	Id(interface{}) *xorm.Session
 	Insert(...interface{}) (int64, error)
 	InsertOne(interface{}) (int64, error)
-	Id(interface{}) *xorm.Session
+	Iterate(interface{}, xorm.IterFunc) error
 	Sql(string, ...interface{}) *xorm.Session
 	Where(string, ...interface{}) *xorm.Session
 }

models/ssh_key.go

@@ -5,12 +5,10 @@
 package models
 
 import (
-	"bufio"
 	"encoding/base64"
 	"encoding/binary"
 	"errors"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"math/big"
 	"os"
@@ -24,6 +22,7 @@ import (
 	"github.com/go-xorm/xorm"
 	"golang.org/x/crypto/ssh"
 
+	"github.com/gogits/gogs/modules/base"
 	"github.com/gogits/gogs/modules/log"
 	"github.com/gogits/gogs/modules/process"
 	"github.com/gogits/gogs/modules/setting"
@@ -457,98 +456,22 @@ func ListPublicKeys(uid int64) ([]*PublicKey, error) {
 	return keys, x.Where("owner_id = ?", uid).Find(&keys)
 }
 
-// rewriteAuthorizedKeys finds and deletes corresponding line in authorized_keys file.
-func rewriteAuthorizedKeys(key *PublicKey, p, tmpP string) error {
-	fr, err := os.Open(p)
-	if err != nil {
-		return err
-	}
-	defer fr.Close()
-
-	fw, err := os.OpenFile(tmpP, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600)
-	if err != nil {
-		return err
-	}
-	defer fw.Close()
-
-	isFound := false
-	keyword := fmt.Sprintf("key-%d", key.ID)
-	buf := bufio.NewReader(fr)
-	for {
-		line, errRead := buf.ReadString('\n')
-		line = strings.TrimSpace(line)
-
-		if errRead != nil {
-			if errRead != io.EOF {
-				return errRead
-			}
-
-			// Reached end of file, if nothing to read then break,
-			// otherwise handle the last line.
-			if len(line) == 0 {
-				break
-			}
-		}
-
-		// Found the line and copy rest of file.
-		if !isFound && strings.Contains(line, keyword) && strings.Contains(line, key.Content) {
-			isFound = true
-			continue
-		}
-		// Still finding the line, copy the line that currently read.
-		if _, err = fw.WriteString(line + "\n"); err != nil {
-			return err
-		}
-
-		if errRead == io.EOF {
-			break
-		}
-	}
-
-	if !isFound {
-		log.Warn("SSH key %d not found in authorized_keys file for deletion", key.ID)
-	}
-
-	return nil
-}
-
 // UpdatePublicKey updates given public key.
 func UpdatePublicKey(key *PublicKey) error {
 	_, err := x.Id(key.ID).AllCols().Update(key)
 	return err
 }
 
-func deletePublicKey(e *xorm.Session, keyID int64) error {
-	sshOpLocker.Lock()
-	defer sshOpLocker.Unlock()
-
-	key := &PublicKey{ID: keyID}
-	has, err := e.Get(key)
-	if err != nil {
-		return err
-	} else if !has {
+// deletePublicKeys does the actual key deletion but does not update authorized_keys file.
+func deletePublicKeys(e *xorm.Session, keyIDs ...int64) error {
+	if len(keyIDs) == 0 {
 		return nil
 	}
 
-	if _, err = e.Id(key.ID).Delete(new(PublicKey)); err != nil {
+	_, err := e.In("id", strings.Join(base.Int64sToStrings(keyIDs), ",")).Delete(new(PublicKey))
 	return err
 }
 
-	// Don't need to rewrite this file if builtin SSH server is enabled.
-	if setting.SSH.StartBuiltinServer {
-		return nil
-	}
-
-	fpath := filepath.Join(setting.SSH.RootPath, "authorized_keys")
-	tmpPath := fpath + ".tmp"
-	if err = rewriteAuthorizedKeys(key, fpath, tmpPath); err != nil {
-		return err
-	} else if err = os.Remove(fpath); err != nil {
-		return err
-	}
-	return os.Rename(tmpPath, fpath)
-}
-
 // DeletePublicKey deletes SSH key information both in database and authorized_keys file.
 func DeletePublicKey(doer *User, id int64) (err error) {
 	key, err := GetPublicKeyByID(id)
@@ -570,14 +493,20 @@ func DeletePublicKey(doer *User, id int64) (err error) {
 		return err
 	}
 
-	if err = deletePublicKey(sess, id); err != nil {
+	if err = deletePublicKeys(sess, id); err != nil {
 		return err
 	}
 
-	return sess.Commit()
+	if err = sess.Commit(); err != nil {
+		return err
+	}
+
+	return RewriteAllPublicKeys()
 }
 
 // RewriteAllPublicKeys removes any authorized key and rewrite all keys from database again.
+// Note: x.Iterate does not get latest data after insert/delete, so we have to call this function
+// outsite any session scope independently.
 func RewriteAllPublicKeys() error {
 	sshOpLocker.Lock()
 	defer sshOpLocker.Unlock()
@@ -814,7 +743,7 @@ func DeleteDeployKey(doer *User, id int64) error {
 	if err != nil {
 		return err
 	} else if !has {
-		if err = deletePublicKey(sess, key.KeyID); err != nil {
+		if err = deletePublicKeys(sess, key.KeyID); err != nil {
 			return err
 		}
 	}

models/user.go

@@ -768,10 +768,13 @@ func deleteUser(e *xorm.Session, u *User) error {
 	if err = e.Find(&keys, &PublicKey{OwnerID: u.ID}); err != nil {
 		return fmt.Errorf("get all public keys: %v", err)
 	}
-	for _, key := range keys {
-		if err = deletePublicKey(e, key.ID); err != nil {
-			return fmt.Errorf("deletePublicKey: %v", err)
+
+	keyIDs := make([]int64, len(keys))
+	for i := range keys {
+		keyIDs[i] = keys[i].ID
 	}
+	if err = deletePublicKeys(e, keyIDs...); err != nil {
+		return fmt.Errorf("deletePublicKeys: %v", err)
 	}
 	// ***** END: PublicKey *****
 
@@ -788,7 +791,6 @@ func deleteUser(e *xorm.Session, u *User) error {
 	// Note: There are something just cannot be roll back,
 	//	so just keep error logs of those operations.
 
-	RewriteAllPublicKeys()
 	os.RemoveAll(UserPath(u.Name))
 	os.Remove(u.CustomAvatarPath())
 
@@ -809,7 +811,11 @@ func DeleteUser(u *User) (err error) {
 		return err
 	}
 
-	return sess.Commit()
+	if err = sess.Commit(); err != nil {
+		return err
+	}
+
+	return RewriteAllPublicKeys()
 }
 
 // DeleteInactivateUsers deletes all inactivate users and email addresses.
@@ -818,6 +824,7 @@ func DeleteInactivateUsers() (err error) {
 	if err = x.Where("is_active = ?", false).Find(&users); err != nil {
 		return fmt.Errorf("get all inactive users: %v", err)
 	}
+	// FIXME: should only update authorized_keys file once after all deletions.
 	for _, u := range users {
 		if err = DeleteUser(u); err != nil {
 			// Ignore users that were set inactive by admin.

templates/.VERSION

@@ -1 +1 @@
-0.9.56.0726
+0.9.57.0726