Merge pull request #1 from mattermost/small-improvements

Improvements around marshalling and status codes

server/main/main.go

@@ -19,8 +19,9 @@ import (
 	"github.com/gorilla/mux"
 )
 
-var wsServer *WSServer
 var config *Configuration
+var wsServer *WSServer
+var store *SQLStore
 
 // ----------------------------------------------------------------------------------------------------
 // HTTP handlers
@@ -56,22 +57,22 @@ func handleGetBlocks(w http.ResponseWriter, r *http.Request) {
 
 	var blocks []string
 	if len(blockType) > 0 && len(parentID) > 0 {
-		blocks = getBlocksWithParentAndType(parentID, blockType)
+		blocks = store.getBlocksWithParentAndType(parentID, blockType)
 	} else if len(blockType) > 0 {
-		blocks = getBlocksWithType(blockType)
+		blocks = store.getBlocksWithType(blockType)
 	} else {
-		blocks = getBlocksWithParent(parentID)
+		blocks = store.getBlocksWithParent(parentID)
 	}
 
 	log.Printf("GetBlocks parentID: %s, type: %s, %d result(s)", parentID, blockType, len(blocks))
 	response := `[` + strings.Join(blocks[:], ",") + `]`
-	jsonResponse(w, 200, response)
+	jsonResponse(w, http.StatusOK, response)
 }
 
 func handlePostBlocks(w http.ResponseWriter, r *http.Request) {
 	requestBody, err := ioutil.ReadAll(r.Body)
 	if err != nil {
-		errorResponse(w, 500, `{}`)
+		errorResponse(w, http.StatusInternalServerError, `{}`)
 		return
 	}
 
@@ -79,41 +80,33 @@ func handlePostBlocks(w http.ResponseWriter, r *http.Request) {
 	defer func() {
 		if r := recover(); r != nil {
 			log.Printf(`ERROR: %v`, r)
-			errorResponse(w, 500, `{}`)
+			errorResponse(w, http.StatusInternalServerError, `{}`)
 			return
 		}
 	}()
 
-	var blockMaps []map[string]interface{}
-	err = json.Unmarshal([]byte(requestBody), &blockMaps)
+	var blocks []Block
+	err = json.Unmarshal([]byte(requestBody), &blocks)
 	if err != nil {
-		errorResponse(w, 500, ``)
+		errorResponse(w, http.StatusInternalServerError, ``)
 		return
 	}
 
 	var blockIDsToNotify = []string{}
 	uniqueBlockIDs := make(map[string]bool)
 
-	for _, blockMap := range blockMaps {
-		jsonBytes, err := json.Marshal(blockMap)
-		if err != nil {
-			errorResponse(w, 500, `{}`)
-			return
-		}
-
-		block := blockFromMap(blockMap)
-
+	for _, block := range blocks {
 		// Error checking
 		if len(block.Type) < 1 {
-			errorResponse(w, 500, fmt.Sprintf(`{"description": "missing type", "id": "%s"}`, block.ID))
+			errorResponse(w, http.StatusInternalServerError, fmt.Sprintf(`{"description": "missing type", "id": "%s"}`, block.ID))
 			return
 		}
 		if block.CreateAt < 1 {
-			errorResponse(w, 500, fmt.Sprintf(`{"description": "invalid createAt", "id": "%s"}`, block.ID))
+			errorResponse(w, http.StatusInternalServerError, fmt.Sprintf(`{"description": "invalid createAt", "id": "%s"}`, block.ID))
 			return
 		}
 		if block.UpdateAt < 1 {
-			errorResponse(w, 500, fmt.Sprintf(`{"description": "invalid updateAt", "id": "%s"}`, block.ID))
+			errorResponse(w, http.StatusInternalServerError, fmt.Sprintf(`{"description": "invalid updateAt", "id": "%s"}`, block.ID))
 			return
 		}
 
@@ -124,13 +117,19 @@ func handlePostBlocks(w http.ResponseWriter, r *http.Request) {
 			blockIDsToNotify = append(blockIDsToNotify, block.ParentID)
 		}
 
-		insertBlock(block, string(jsonBytes))
+		jsonBytes, err := json.Marshal(block)
+		if err != nil {
+			errorResponse(w, http.StatusInternalServerError, `{}`)
+			return
+		}
+
+		store.insertBlock(block, string(jsonBytes))
 	}
 
 	wsServer.broadcastBlockChangeToWebsocketClients(blockIDsToNotify)
 
-	log.Printf("POST Blocks %d block(s)", len(blockMaps))
-	jsonResponse(w, 200, "{}")
+	log.Printf("POST Blocks %d block(s)", len(blocks))
+	jsonResponse(w, http.StatusOK, "{}")
 }
 
 func handleDeleteBlock(w http.ResponseWriter, r *http.Request) {
@@ -139,43 +138,43 @@ func handleDeleteBlock(w http.ResponseWriter, r *http.Request) {
 
 	var blockIDsToNotify = []string{blockID}
 
-	parentID := getParentID(blockID)
+	parentID := store.getParentID(blockID)
 
 	if len(parentID) > 0 {
 		blockIDsToNotify = append(blockIDsToNotify, parentID)
 	}
 
-	deleteBlock(blockID)
+	store.deleteBlock(blockID)
 
 	wsServer.broadcastBlockChangeToWebsocketClients(blockIDsToNotify)
 
 	log.Printf("DELETE Block %s", blockID)
-	jsonResponse(w, 200, "{}")
+	jsonResponse(w, http.StatusOK, "{}")
 }
 
 func handleGetSubTree(w http.ResponseWriter, r *http.Request) {
 	vars := mux.Vars(r)
 	blockID := vars["blockID"]
 
-	blocks := getSubTree(blockID)
+	blocks := store.getSubTree(blockID)
 
 	log.Printf("GetSubTree blockID: %s, %d result(s)", blockID, len(blocks))
 	response := `[` + strings.Join(blocks[:], ",") + `]`
-	jsonResponse(w, 200, response)
+	jsonResponse(w, http.StatusOK, response)
 }
 
 func handleExport(w http.ResponseWriter, r *http.Request) {
-	blocks := getAllBlocks()
+	blocks := store.getAllBlocks()
 
 	log.Printf("EXPORT Blocks, %d result(s)", len(blocks))
 	response := `[` + strings.Join(blocks[:], ",") + `]`
-	jsonResponse(w, 200, response)
+	jsonResponse(w, http.StatusOK, response)
 }
 
 func handleImport(w http.ResponseWriter, r *http.Request) {
 	requestBody, err := ioutil.ReadAll(r.Body)
 	if err != nil {
-		errorResponse(w, 500, `{}`)
+		errorResponse(w, http.StatusInternalServerError, `{}`)
 		return
 	}
 
@@ -183,31 +182,30 @@ func handleImport(w http.ResponseWriter, r *http.Request) {
 	defer func() {
 		if r := recover(); r != nil {
 			log.Printf(`ERROR: %v`, r)
-			errorResponse(w, 500, `{}`)
+			errorResponse(w, http.StatusInternalServerError, `{}`)
 			return
 		}
 	}()
 
-	var blockMaps []map[string]interface{}
-	err = json.Unmarshal([]byte(requestBody), &blockMaps)
+	var blocks []Block
+	err = json.Unmarshal([]byte(requestBody), &blocks)
 	if err != nil {
-		errorResponse(w, 500, ``)
+		errorResponse(w, http.StatusInternalServerError, ``)
 		return
 	}
 
-	for _, blockMap := range blockMaps {
-		jsonBytes, err := json.Marshal(blockMap)
+	for _, block := range blocks {
+		jsonBytes, err := json.Marshal(block)
 		if err != nil {
-			errorResponse(w, 500, `{}`)
+			errorResponse(w, http.StatusInternalServerError, `{}`)
 			return
 		}
 
-		block := blockFromMap(blockMap)
-		insertBlock(block, string(jsonBytes))
+		store.insertBlock(block, string(jsonBytes))
 	}
 
-	log.Printf("IMPORT Blocks %d block(s)", len(blockMaps))
-	jsonResponse(w, 200, "{}")
+	log.Printf("IMPORT Blocks %d block(s)", len(blocks))
+	jsonResponse(w, http.StatusOK, "{}")
 }
 
 // File upload
@@ -378,7 +376,11 @@ func main() {
 
 	http.Handle("/", r)
 
-	connectDatabase(config.DBType, config.DBConfigString)
+	store, err = NewSQLStore(config.DBType, config.DBConfigString)
+	if err != nil {
+		log.Fatal("Unable to start the database", err)
+		panic(err)
+	}
 
 	// Ctrl+C handling
 	handler := make(chan os.Signal, 1)

server/main/octoDatabase.go

@@ -10,42 +10,61 @@ import (
 	_ "github.com/mattn/go-sqlite3"
 )
 
-var db *sql.DB
+// SQLStore is a SQL database
+type SQLStore struct {
+	db     *sql.DB
+	dbType string
+}
 
-func connectDatabase(dbType string, connectionString string) {
+// NewSQLStore creates a new SQLStore
+func NewSQLStore(dbType, connectionString string) (*SQLStore, error) {
 	log.Println("connectDatabase")
 	var err error
 
-	db, err = sql.Open(dbType, connectionString)
+	db, err := sql.Open(dbType, connectionString)
 	if err != nil {
 		log.Fatal("connectDatabase: ", err)
-		panic(err)
+		return nil, err
 	}
 
 	err = db.Ping()
 	if err != nil {
 		log.Println(`Database Ping failed`)
-		panic(err)
+		return nil, err
 	}
 
-	createTablesIfNotExists(dbType)
+	store := &SQLStore{
+		db:     db,
+		dbType: dbType,
+	}
+
+	err = store.createTablesIfNotExists()
+	if err != nil {
+		log.Println(`Table creation failed`)
+		return nil, err
+	}
+	return store, nil
 }
 
 // Block is the basic data unit
 type Block struct {
-	ID       string `json:"id"`
-	ParentID string `json:"parentId"`
-	Type     string `json:"type"`
-	CreateAt int64  `json:"createAt"`
-	UpdateAt int64  `json:"updateAt"`
-	DeleteAt int64  `json:"deleteAt"`
+	ID         string                 `json:"id"`
+	ParentID   string                 `json:"parentId"`
+	Schema     int64                  `json:"schema"`
+	Type       string                 `json:"type"`
+	Title      string                 `json:"title"`
+	Properties map[string]interface{} `json:"properties"`
+	Fields     map[string]interface{} `json:"fields"`
+	CreateAt   int64                  `json:"createAt"`
+	UpdateAt   int64                  `json:"updateAt"`
+	DeleteAt   int64                  `json:"deleteAt"`
 }
 
-func createTablesIfNotExists(dbType string) {
+func (s *SQLStore) createTablesIfNotExists() error {
 	// TODO: Add update_by with the user's ID
 	// TODO: Consolidate insert_at and update_at, decide if the server of DB should set it
 	var query string
-	if dbType == "sqlite3" {
+	if s.dbType == "sqlite3" {
 		query = `CREATE TABLE IF NOT EXISTS blocks (
 			id VARCHAR(36),
 			insert_at DATETIME NOT NULL DEFAULT current_timestamp,
@@ -71,39 +90,16 @@ func createTablesIfNotExists(dbType string) {
 		);`
 	}
 
-	_, err := db.Exec(query)
+	_, err := s.db.Exec(query)
 	if err != nil {
 		log.Fatal("createTablesIfNotExists: ", err)
-		panic(err)
+		return err
 	}
-	log.Printf("createTablesIfNotExists(%s)", dbType)
+	log.Printf("createTablesIfNotExists(%s)", s.dbType)
+	return nil
 }
 
-func blockFromMap(m map[string]interface{}) Block {
-	var b Block
-	b.ID = m["id"].(string)
-	// Parent ID can be nil (for now)
-	if m["parentId"] != nil {
-		b.ParentID = m["parentId"].(string)
-	}
-	// Allow nil type for imports
-	if m["type"] != nil {
-		b.Type = m["type"].(string)
-	}
-	if m["createAt"] != nil {
-		b.CreateAt = int64(m["createAt"].(float64))
-	}
-	if m["updateAt"] != nil {
-		b.UpdateAt = int64(m["updateAt"].(float64))
-	}
-	if m["deleteAt"] != nil {
-		b.DeleteAt = int64(m["deleteAt"].(float64))
-	}
-
-	return b
-}
-
-func getBlocksWithParentAndType(parentID string, blockType string) []string {
+func (s *SQLStore) getBlocksWithParentAndType(parentID string, blockType string) []string {
 	query := `WITH latest AS
 		(
 			SELECT * FROM
@@ -120,7 +116,7 @@ func getBlocksWithParentAndType(parentID string, blockType string) []string {
 		FROM latest
 		WHERE delete_at = 0 and parent_id = $1 and type = $2`
 
-	rows, err := db.Query(query, parentID, blockType)
+	rows, err := s.db.Query(query, parentID, blockType)
 	if err != nil {
 		log.Printf(`getBlocksWithParentAndType ERROR: %v`, err)
 		panic(err)
@@ -129,7 +125,7 @@ func getBlocksWithParentAndType(parentID string, blockType string) []string {
 	return blocksFromRows(rows)
 }
 
-func getBlocksWithParent(parentID string) []string {
+func (s *SQLStore) getBlocksWithParent(parentID string) []string {
 	query := `WITH latest AS
 		(
 			SELECT * FROM
@@ -146,7 +142,7 @@ func getBlocksWithParent(parentID string) []string {
 		FROM latest
 		WHERE delete_at = 0 and parent_id = $1`
 
-	rows, err := db.Query(query, parentID)
+	rows, err := s.db.Query(query, parentID)
 	if err != nil {
 		log.Printf(`getBlocksWithParent ERROR: %v`, err)
 		panic(err)
@@ -155,7 +151,7 @@ func getBlocksWithParent(parentID string) []string {
 	return blocksFromRows(rows)
 }
 
-func getBlocksWithType(blockType string) []string {
+func (s *SQLStore) getBlocksWithType(blockType string) []string {
 	query := `WITH latest AS
 		(
 			SELECT * FROM
@@ -172,7 +168,7 @@ func getBlocksWithType(blockType string) []string {
 		FROM latest
 		WHERE delete_at = 0 and type = $1`
 
-	rows, err := db.Query(query, blockType)
+	rows, err := s.db.Query(query, blockType)
 	if err != nil {
 		log.Printf(`getBlocksWithParentAndType ERROR: %v`, err)
 		panic(err)
@@ -181,7 +177,7 @@ func getBlocksWithType(blockType string) []string {
 	return blocksFromRows(rows)
 }
 
-func getSubTree(blockID string) []string {
+func (s *SQLStore) getSubTree(blockID string) []string {
 	query := `WITH latest AS
 	(
 		SELECT * FROM
@@ -200,7 +196,7 @@ func getSubTree(blockID string) []string {
 		AND (id = $1
 			OR parent_id = $1)`
 
-	rows, err := db.Query(query, blockID)
+	rows, err := s.db.Query(query, blockID)
 	if err != nil {
 		log.Printf(`getSubTree ERROR: %v`, err)
 		panic(err)
@@ -209,7 +205,7 @@ func getSubTree(blockID string) []string {
 	return blocksFromRows(rows)
 }
 
-func getAllBlocks() []string {
+func (s *SQLStore) getAllBlocks() []string {
 	query := `WITH latest AS
 	(
 		SELECT * FROM
@@ -226,7 +222,7 @@ func getAllBlocks() []string {
 	FROM latest
 	WHERE delete_at = 0`
 
-	rows, err := db.Query(query)
+	rows, err := s.db.Query(query)
 	if err != nil {
 		log.Printf(`getAllBlocks ERROR: %v`, err)
 		panic(err)
@@ -255,7 +251,7 @@ func blocksFromRows(rows *sql.Rows) []string {
 	return results
 }
 
-func getParentID(blockID string) string {
+func (s *SQLStore) getParentID(blockID string) string {
 	statement :=
 		`WITH latest AS
 		(
@@ -274,7 +270,7 @@ func getParentID(blockID string) string {
 		WHERE delete_at = 0
 			AND id = $1`
 
-	row := db.QueryRow(statement, blockID)
+	row := s.db.QueryRow(statement, blockID)
 
 	var parentID string
 	err := row.Scan(&parentID)
@@ -285,19 +281,19 @@ func getParentID(blockID string) string {
 	return parentID
 }
 
-func insertBlock(block Block, json string) {
+func (s *SQLStore) insertBlock(block Block, json string) {
 	statement := `INSERT INTO blocks(id, parent_id, type, json, create_at, update_at, delete_at) VALUES($1, $2, $3, $4, $5, $6, $7)`
-	_, err := db.Exec(statement, block.ID, block.ParentID, block.Type, json, block.CreateAt, block.UpdateAt, block.DeleteAt)
+	_, err := s.db.Exec(statement, block.ID, block.ParentID, block.Type, json, block.CreateAt, block.UpdateAt, block.DeleteAt)
 	if err != nil {
 		panic(err)
 	}
 }
 
-func deleteBlock(blockID string) {
+func (s *SQLStore) deleteBlock(blockID string) {
 	now := time.Now().Unix()
 	json := fmt.Sprintf(`{"id":"%s","updateAt":%d,"deleteAt":%d}`, blockID, now, now)
 	statement := `INSERT INTO blocks(id, json, update_at, delete_at) VALUES($1, $2, $3, $4)`
-	_, err := db.Exec(statement, blockID, json, now, now)
+	_, err := s.db.Exec(statement, blockID, json, now, now)
 	if err != nil {
 		panic(err)
 	}

src/client/block.ts

@@ -3,6 +3,7 @@ import { Utils } from "./utils"
 
 class Block implements IBlock {
 	id: string = Utils.createGuid()
+	schema: number
 	parentId: string
 	type: string
 	title: string
@@ -10,6 +11,7 @@ class Block implements IBlock {
 	url?: string
 	order: number
 	properties: Record<string, string> = {}
+	fields: Record<string, any> = {}
 	createAt: number = Date.now()
 	updateAt: number = 0
 	deleteAt: number = 0
@@ -31,26 +33,35 @@ class Block implements IBlock {
 		const now = Date.now()
 
 		this.id = block.id || Utils.createGuid()
+		this.schema = 1
 		this.parentId = block.parentId
 		this.type = block.type
+
+		this.fields = block.fields ? { ...block.fields } : {}
+
 		this.title = block.title
 		this.icon = block.icon
 		this.url = block.url
 		this.order = block.order
+
 		this.createAt = block.createAt || now
 		this.updateAt = block.updateAt || now
 		this.deleteAt = block.deleteAt || 0
 
-		if (Array.isArray(block.properties)) {
-			// HACKHACK: Port from old schema
-			this.properties = {}
-			for (const property of block.properties) {
-				if (property.id) {
-					this.properties[property.id] = property.value
+		if (block.schema !== 1) {
+			if (Array.isArray(block.properties)) {
+				// HACKHACK: Port from old schema
+				this.properties = {}
+				for (const property of block.properties) {
+					if (property.id) {
+						this.properties[property.id] = property.value
+					}
 				}
+			} else {
+				this.properties = { ...block.properties || {} }
 			}
 		} else {
-			this.properties = { ...block.properties || {} }
+			this.properties = { ...block.properties }	// Shallow copy here. Derived classes must make deep copies of their known properties in their constructors.
 		}
 	}
 }

src/client/board.ts

@@ -16,24 +16,37 @@ interface IPropertyTemplate {
 }
 
 class Board extends Block {
-	cardProperties: IPropertyTemplate[] = []
+	get cardProperties(): IPropertyTemplate[] { return this.fields.cardProperties as IPropertyTemplate[] }
+	set cardProperties(value: IPropertyTemplate[]) { this.fields.cardProperties = value }
 
 	constructor(block: any = {}) {
 		super(block)
 		this.type = "board"
-		if (block.cardProperties) {
-			// Deep clone of properties and their options
-			this.cardProperties = block.cardProperties.map((o: IPropertyTemplate) => {
+
+		if (block.fields?.cardProperties) {
+			// Deep clone of card properties and their options
+			this.cardProperties = block.fields?.cardProperties.map((o: IPropertyTemplate) => {
 				return {
 					id: o.id,
 					name: o.name,
 					type: o.type,
-					options: o.options ? o.options.map(option => ({...option})): []
+					options: o.options ? o.options.map(option => ({ ...option })) : []
 				}
 			})
 		} else {
 			this.cardProperties = []
 		}
+
+		if (block.schema !== 1) {
+			this.cardProperties = block.cardProperties?.map((o: IPropertyTemplate) => {
+				return {
+					id: o.id,
+					name: o.name,
+					type: o.type,
+					options: o.options ? o.options.map(option => ({ ...option })) : []
+				}
+			}) || []
+		}
 	}
 }
 

src/client/boardView.ts

@@ -5,21 +5,40 @@ type IViewType = "board" | "table" | "calendar" | "list" | "gallery"
 type ISortOption = { propertyId: "__name" | string, reversed: boolean }
 
 class BoardView extends Block {
-	viewType: IViewType
-	groupById?: string
-	sortOptions: ISortOption[]
-	visiblePropertyIds: string[]
-	filter?: FilterGroup
+	get viewType(): IViewType { return this.fields.viewType }
+	set viewType(value: IViewType) { this.fields.viewType = value }
+
+	get groupById(): string | undefined { return this.fields.groupById }
+	set groupById(value: string | undefined) { this.fields.groupById = value }
+
+	get sortOptions(): ISortOption[] { return this.fields.sortOptions }
+	set sortOptions(value: ISortOption[]) { this.fields.sortOptions = value }
+
+	get visiblePropertyIds(): string[] { return this.fields.visiblePropertyIds }
+	set visiblePropertyIds(value: string[]) { this.fields.visiblePropertyIds = value }
+
+	get filter(): FilterGroup | undefined { return this.fields.filter }
+	set filter(value: FilterGroup | undefined) { this.fields.filter = value }
 
 	constructor(block: any = {}) {
 		super(block)
 
 		this.type = "view"
-		this.viewType = block.viewType || "board"
-		this.groupById = block.groupById
-		this.sortOptions = block.sortOptions ? block.sortOptions.map((o: ISortOption) => ({...o})) : []		// Deep clone
-		this.visiblePropertyIds = block.visiblePropertyIds ? block.visiblePropertyIds.slice() : []
-		this.filter = new FilterGroup(block.filter)
+
+		this.sortOptions = block.properties?.sortOptions?.map((o: ISortOption) => ({ ...o })) || []		// Deep clone
+		this.visiblePropertyIds = block.properties?.visiblePropertyIds?.slice() || []
+		this.filter = new FilterGroup(block.properties?.filter)
+
+		// TODO: Remove this fixup code
+		if (block.schema !== 1) {
+			this.viewType = block.viewType || "board"
+			this.groupById = block.groupById
+			this.sortOptions = block.sortOptions ? block.sortOptions.map((o: ISortOption) => ({ ...o })) : [] 		// Deep clone
+			this.visiblePropertyIds = block.visiblePropertyIds ? block.visiblePropertyIds.slice() : []
+			this.filter = new FilterGroup(block.filter)
+		}
+
+		if (!this.viewType) { this.viewType = "board" }
 	}
 }
 

src/client/octoTypes.ts

@@ -3,12 +3,14 @@ interface IBlock {
 	id: string
 	parentId: string
 
+	schema: number
 	type: string
 	title?: string
 	url?: string			// TODO: Move to properties (_url)
 	icon?: string
 	order: number
 	properties: Record<string, string>
+	fields: Record<string, any>
 
 	createAt: number
 	updateAt: number