Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue when finding a value of 'null' for a platform type in some instances #14

Open
bbaldino opened this issue Jul 24, 2020 · 0 comments

Comments

@bbaldino
Copy link
Member

I've come across a problem where, when retrieving a platform type with a value of null, the null value won't trigger an exception (since we use a non-nullable type) and fall-through but instead will actually trickle all the way up as a supplied value.

The specific scenario where I saw this was:
I was converting the retrieval of the keepAliveStrategy enum in JVB IceConfig.kt. I had changed it from the old method of doing retrieving as a string and converting via KeepAliveStrategy.fromString:

retrieve("videobridge.ice.keep-alive-strategy"
    .from(JitsiConfig.newConfig)
    .asType<String>()
    .andConvertBy { KeepAliveStrategy.fromString(it) }
)

to retrieving the enum directly:

retrieve("videobridge.ice.keep-alive-strategy".from(JitsiConfig.newConfig))

but I didn't want to break support for old config files, so needed to support both. I put the 'old' parsing first, so that it would get null and fail if the string didn't match, but instead the null was used as the final value. I filed a bug, so something may come from that.

@bbaldino bbaldino changed the title Issue with retrieving null platform types in some instances Issue when finding a value of 'null' for a platform type in some instances Jul 24, 2020
bbaldino added a commit to bbaldino/jitsi-videobridge that referenced this issue Jul 24, 2020
bbaldino added a commit to jitsi/jitsi-videobridge that referenced this issue Jul 29, 2020
* add metaconfig dep

* (temporarily) add a new core JitsiConfig and a TypesafeConfigSource

eventually these will both be done in Jicoco

* port health config over to metaconfig

* port octo config to metaconfig

* fix octo config

* port expire thread config to metaconfig

* port endpointconnectionstatusconfig to metaconfig

* use NewJitsiConfig from jicoco

* inject new legacy config service from NewJitsiConfig

* port ice config to metaconfig

* fix iceconfig enum parsing

* fix ice config

* port bandwidthprobing config to metaconfig

* port xmpp client connection config to metaconfig

* port websocketserviceconfig to metaconfig

* port videobridge config to metaconfig

* change xhow mppclientconnectionconfig builds the client configs from a legacy source

* wip: port stats config

* port stats manager bundle activator config to metaconfig

* we no longer need to use new forks of the jvm to run tests (thanks to differences in metaconfig vs old config).

* remove unneeded NewTypesafeConfigSource (moved to jicoco)

* add OctoConfig tests, fix a bug with legacy 'enabled' prop definition

* add xmppclientconnectionconfigtests, fix bug for filtering incomplete configs

* tweak method used for ConfigTest

* point to jitpack for metaconfig

* remove config debug logs

* port transportconfig

* port bitratecontrollerconfig

* port jvbapiconfig

* fixup: port bitratecontrollerconfig cont.

* update to new config class names in jicoco

* updates tests to new config class names in jicoco

* fix name of StatsManagerBundleActivatorConfig

* fix name of StatsManagerBundleActivatorConfig

* fix typos

* revert back to using string/conversion for enums (see jitsi/jitsi-metaconfig#14)

* add missing retrieve in nominationStrategy config

* remove (old) reload of config before initializing ice4j

the new legacy shim acts like the old one, so it actively checks the
system properties when retrieving (whereas the old-new config cached
them) so this is no longer necessary.

* remove (old) reload before starting jvb

the (old-new) config cached system properties on start, so a reload was
needed before reading them if we change them.  the new legacy config
shim still reads system props on demand, so the reload isn't needed.

* update XmppClientConnectionConfig to new config syntax

* update WebsocketServiceConfig to new config syntax

* update EndpointConnectionStatusConfig to new config syntax

* update VideobridgeConfig to new config syntax

* update VideobridgeExpireThreadConfig to new config syntax

* update BandwidthProbingConfig to new config syntax

* update BitrateControllerConfig to new config syntax

* update HealthConfig to new config syntax

* update IceConfig to new config syntax

* update OctoConfig to new config syntax

* update StatsManagerBundleActivatorConfig to new config syntax

* update jmc version

* update jicoco version

* update jmt dep

* remove extra spaces

* move iceconfig member to iceconfig class

* re-add license header

* wire up metaconfig logger
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant