better code: store undefined state instead of cloning defaultValue
Massimo Melina committed
Apr 29, 2022 at 18:27 UTC
d7542ae5794d6afb8a3fd30462306150f802be61
7 files changed
+63
-55
server/src/adminApis.ts
+2
-5
@@ -175,12 +175,9 @@ export const adminApis: ApiHandlers = {
175
}
176
if (config) {
177
config = _.pickBy(config, v => v !== null)
178
- const o = { ...pluginsConfig.get() }
178
if (_.isEmpty(config))
180
- delete o[id]
181
- else
182
- o[id] = config
183
- pluginsConfig.set( _.isEmpty(o) ? undefined : o )
179
+ config = undefined
180
+ pluginsConfig.set({ ...pluginsConfig.get(), [id]: config })
181
}
182
return {}
183
},
server/src/config.ts
+38
-37
@@ -5,7 +5,7 @@ import { argv } from './const'
5
import { watchLoad } from './watchLoad'
6
import yaml from 'yaml'
7
import _ from 'lodash'
8
-import { debounceAsync, objSameKeys, onOff, wait } from './misc'
8
+import { debounceAsync, same, objSameKeys, onOff, wait } from './misc'
9
import { exists } from 'fs'
10
import { promisify } from 'util'
11
@@ -35,42 +35,43 @@ export function defineConfig<T>(k: string, defaultValue?: T) {
35
key() {
36
return k
37
},
38
- get() {
38
+ get(): T {
39
return getConfig(k)
40
},
41
sub(cb: (v:T, was?:T)=>void) {
42
return subscribeConfig(k, cb)
43
},
44
set(v: T) {
45
- setConfig({ [k]: v })
45
+ return setConfig1(k, v)
46
}
47
}
48
}
49
50
+const stack: any[] = []
51
function subscribeConfig<T>(k:string, cb: (v:T, was?:T)=>void) {
51
- const { defaultValue } = configProps[k] ?? {}
52
+ if (started) // initial event already passed, we'll make the first call
53
+ cb(getConfig(k))
54
const eventName = 'new.'+k
53
- if (started) {
54
- let v = state[k]
55
- if (v === undefined)
56
- state[k] = v = _.cloneDeep(defaultValue) // clone to avoid changing
57
- if (v !== undefined)
58
- cb(v)
59
- }
60
- return onOff(cfgEvents, { [eventName]: cb })
55
+ return onOff(cfgEvents, {
56
+ [eventName]() {
57
+ if (stack.includes(cb)) return // avoid infinite loop in case a subscriber changes the value
58
+ stack.push(cb) // @ts-ignore arguments
59
+ try { return cb.apply(this,arguments) }
60
+ finally { stack.pop() }
61
+ }
62
+ })
63
}
64
65
function getConfig(k:string) {
64
- return k in state ? state[k] : configProps[k]?.defaultValue
66
+ return state[k] ?? _.cloneDeep(configProps[k]?.defaultValue) // clone to avoid changing
67
}
68
67
-export function getWholeConfig({ omit=[], only=[] }: { omit:string[], only:string[] }) {
68
- let copy = Object.assign(
69
- objSameKeys(configProps, x => x.defaultValue),
70
- state,
71
- )
72
- copy = _.omit(copy, omit)
73
- if (only.length)
69
+export function getWholeConfig({ omit, only }: { omit?:string[], only?:string[] }) {
70
+ const defs = objSameKeys(configProps, x => x.defaultValue)
71
+ let copy = _.defaults({}, state, defs)
72
+ if (omit?.length)
73
+ copy = _.omit(copy, omit)
74
+ if (only)
75
copy = _.pick(copy, only)
76
return _.cloneDeep(copy)
77
}
@@ -85,7 +86,7 @@ export function setConfig(newCfg: Record<string,any>, save?: boolean) {
86
}
87
}
88
for (const k in newCfg)
88
- check(k)
89
+ apply(k, newCfg[k])
90
if (save) {
91
saveConfigAsap().then()
92
return
@@ -94,35 +95,35 @@ export function setConfig(newCfg: Record<string,any>, save?: boolean) {
95
if (save === false) // false is used when loading whole config, and in such case we should not leave previous values untreated. Also, we need this only after we already `started`.
96
for (const k of Object.keys(state))
97
if (!newCfg.hasOwnProperty(k))
97
- check(k)
98
+ apply(k, newCfg[k])
99
return
100
}
101
// first time we emit also for the default values
102
for (const k of Object.keys(configProps))
103
if (!newCfg.hasOwnProperty(k))
103
- check(k)
104
+ apply(k, newCfg[k])
105
started = true
106
106
- function check(k: string) {
107
- const oldV = started ? getConfig(k) : state[k] // from second time consider also defaultValue
108
- const newV = newCfg[k]
109
- const { defaultValue } = configProps[k] ?? {}
110
- let v = newV ?? _.cloneDeep(defaultValue) // if we have an object we may get into troubles letting others change ours
111
- const j = JSON.stringify(v)
112
- if (j === JSON.stringify(oldV)) return // no change
113
- state[k] = v
114
- cfgEvents.emit('new.'+k, v, oldV)
115
- if (save === undefined)
116
- saveConfigAsap().then()
107
+ function apply(k: string, newV: any) {
108
+ return setConfig1(k, newV, save === undefined)
109
}
110
}
111
112
+function setConfig1(k: string, newV: any, saveChanges=true) {
113
+ if (same(newV, configProps[k]?.defaultValue))
114
+ newV = undefined
115
+ if (started && same(newV, state[k])) return // no change
116
+ const was = getConfig(k) // include cloned default, if necessary
117
+ state[k] = newV
118
+ cfgEvents.emit('new.'+k, getConfig(k), was)
119
+ if (saveChanges)
120
+ saveConfigAsap().then()
121
+}
122
+
123
export const saveConfigAsap = debounceAsync(async () => {
124
while (!started)
125
await wait(100)
123
- const diff = objSameKeys(state, (v,k) =>
124
- JSON.stringify(v) === JSON.stringify(configProps[k]?.defaultValue) ? undefined : v)
125
- let txt = yaml.stringify(diff, { lineWidth:1000 })
126
+ let txt = yaml.stringify(state, { lineWidth:1000 })
127
if (txt.trim() === '{}') // most users wouldn't understand
128
if (await promisify(exists)(path)) // if a file exists then empty it, else don't bother creating it
129
txt = ''
server/src/listen.ts
+7
-5
@@ -36,17 +36,19 @@ const cert = defineConfig<string>('cert')
36
const privateKey = defineConfig<string>('private_key')
37
const httpsNeeds = [cert, privateKey]
38
const httpsNeedsNames = { cert: 'certificate', private_key: 'private key' }
39
+const httpsOptions = { key: '', cert: '' }
40
for (const cfg of httpsNeeds) {
41
let unwatch: ReturnType<typeof watchLoad>['unwatch']
42
cfg.sub(async v => {
43
unwatch?.()
43
- cfg.set(v)
44
+ const k = cfg.key() === 'private_key' ? 'key' : 'cert'
45
+ httpsOptions[k] = v
46
if (!v || v.includes('\n'))
47
return considerHttps()
46
- // it's a path
47
- cfg.set('')
48
+ // v is a path
49
+ httpsOptions[k] = ''
50
unwatch = watchLoad(v, data => {
49
- cfg.set(data)
51
+ httpsOptions[k] = data
52
considerHttps()
53
}).unwatch
54
await considerHttps()
@@ -63,7 +65,7 @@ async function considerHttps() {
65
while (!app)
66
await wait(100)
67
httpsSrv = Object.assign(
66
- https.createServer(port < 0 ? {} : { key: privateKey.get(), cert: cert.get() }, app.callback()),
68
+ https.createServer(port < 0 ? {} : httpsOptions, app.callback()),
69
{ name: 'https' }
70
)
71
const missingCfg = httpsNeeds.find(x => !x.get())
server/src/misc.ts
+8
@@ -11,6 +11,7 @@ import glob from 'fast-glob'
11
import { IS_WINDOWS } from './const'
12
import { execFile } from 'child_process'
13
import { Connection } from './connections'
14
+import assert from 'assert'
15
16
export type Callback<IN=void, OUT=void> = (x:IN) => OUT
17
export type Dict<T = any> = Record<string, T>
@@ -281,3 +282,10 @@ export function run(cmd: string, args: string[] = []): Promise<string> {
282
}))
283
}
284
285
+export function same(a: any, b: any) {
286
+ try {
287
+ assert.deepStrictEqual(a, b)
288
+ return true
289
+ }
290
+ catch { return false }
291
+}
server/src/plugins.ts
+1
-1
@@ -146,7 +146,7 @@ watchDir(PATH, rescanAsap)
146
export const enablePlugins = defineConfig('enable_plugins', ['antibrute'])
147
enablePlugins.sub(rescanAsap)
148
149
-export const pluginsConfig = defineConfig('plugins_config')
149
+export const pluginsConfig = defineConfig('plugins_config', {} as Record<string,any>)
150
151
async function rescan() {
152
console.debug('scanning plugins')
server/src/serveFile.ts
+3
-3
@@ -13,7 +13,7 @@ import path from 'path'
13
import { promisify } from 'util'
14
import { updateConnection } from './connections'
15
16
-const allowedReferer = defineConfig('allowed_referer')
16
+const allowedReferer = defineConfig('allowed_referer', '')
17
18
export function serveFileNode(node: VfsNode) : Koa.Middleware {
19
const { source, mime } = node
@@ -39,7 +39,7 @@ export function serveFileNode(node: VfsNode) : Koa.Middleware {
39
}
40
}
41
42
-const mimeCfg = defineConfig('mime', { '*.jpg|*.png|*.mp3|*.txt': 'auto' })
42
+const mimeCfg = defineConfig<Record<string,string>>('mime', { '*.jpg|*.png|*.mp3|*.txt': 'auto' })
43
44
export function serveFile(source:string, mime?:string, modifier?:(s:string)=>string) : Koa.Middleware {
45
return async (ctx) => {
@@ -48,7 +48,7 @@ export function serveFile(source:string, mime?:string, modifier?:(s:string)=>str
48
const { range } = ctx.request.header
49
ctx.set('Accept-Ranges', 'bytes')
50
const fn = path.basename(source)
51
- mime = mime ?? _.find(mimeCfg.get(), (v,k) => k && mm.isMatch(fn, k))
51
+ mime = mime ?? _.find(mimeCfg.get(), (v,k) => k>'' && mm.isMatch(fn, k)) // isMatch throws on an empty string
52
if (mime === MIME_AUTO)
53
mime = mimetypes.lookup(source) || ''
54
if (mime)
server/src/throttler.ts
+4
-4
@@ -10,8 +10,8 @@ import _ from 'lodash'
10
11
const mainThrottleGroup = new ThrottleGroup(Infinity)
12
13
-defineConfig('max_kbps', null).sub(v =>
14
- mainThrottleGroup.updateLimit(v ?? Infinity))
13
+defineConfig('max_kbps', Infinity).sub(v =>
14
+ mainThrottleGroup.updateLimit(v))
15
16
const ip2group: Record<string, {
17
count: number
@@ -22,7 +22,7 @@ const ip2group: Record<string, {
22
const SymThrStr = Symbol('stream')
23
const SymTimeout = Symbol('timeout')
24
25
-const maxKbpsPerIp = defineConfig('max_kbps_per_ip', null)
25
+const maxKbpsPerIp = defineConfig('max_kbps_per_ip', Infinity)
26
27
export const throttler: Koa.Middleware = async (ctx, next) => {
28
await next()
@@ -35,7 +35,7 @@ export const throttler: Koa.Middleware = async (ctx, next) => {
35
const group = new ThrottleGroup(Infinity, doLimit && mainThrottleGroup)
36
37
const unsub = doLimit && maxKbpsPerIp.sub(v =>
38
- group.updateLimit(v ?? Infinity))
38
+ group.updateLimit(v))
39
return { group, count:0, destroy: unsub }
40
})
41
const conn = ctx.state.connection