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

Removes obsolete network settings from system.xml #5482

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Changes from 2 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
8348145
New xml migration
BaronGreenback Mar 11, 2021
e96671b
Moved to program.cs
BaronGreenback Mar 11, 2021
6d4e544
Moved migration
BaronGreenback Mar 15, 2021
a9f4cf8
Update ApplicationHost.cs
BaronGreenback Mar 15, 2021
08aeb4f
Update ApplicationHost.cs
BaronGreenback Mar 15, 2021
d5051d7
Update Program.cs
BaronGreenback Mar 15, 2021
1b9b775
Changed to xmlTextReader
BaronGreenback Mar 15, 2021
a43ffc1
reworking
BaronGreenback Mar 22, 2021
14ce842
Simplified
BaronGreenback Mar 22, 2021
5d28ac0
removed unused file.
BaronGreenback Mar 22, 2021
6617fc3
sln correction
BaronGreenback Mar 22, 2021
72b3684
Merge branch 'master' into SettingsMigration
BaronGreenback Mar 23, 2021
23aad2d
Updated from master
BaronGreenback Apr 15, 2021
1bd8259
Used fixed config.
BaronGreenback Apr 15, 2021
1c95184
Merge branch 'master' into SettingsMigration
BaronGreenback Apr 21, 2021
26c6d90
fixed merge
BaronGreenback Apr 21, 2021
ed638d2
Update MigrationRunner.cs
BaronGreenback May 6, 2021
1fd43bc
Create delete me
BaronGreenback May 6, 2021
0d4e0bf
Rename Jellyfin.Server/Migrations/Legacy/V703/NetworkConfiguration.cs…
BaronGreenback May 6, 2021
b12443a
Update and rename Jellyfin.Server/Migrations/Legacy/V703/ServerConfig…
BaronGreenback May 6, 2021
13a8616
Delete delete me
BaronGreenback May 6, 2021
d391257
Update NetworkConfiguration.cs
BaronGreenback May 6, 2021
2d1452e
Improved
BaronGreenback May 7, 2021
6de8c96
changed path to configuration folder
BaronGreenback May 7, 2021
c09b0f7
Merge branch 'master' into SettingsMigration
BaronGreenback May 29, 2021
File filter
Filter file types
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.

Always

Just for now

@@ -14,6 +14,8 @@
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
using System.Xml.Serialization;
using Emby.Dlna;
using Emby.Dlna.Main;
using Emby.Dlna.Ssdp;
@@ -84,6 +86,7 @@
using MediaBrowser.Controller.TV;
using MediaBrowser.LocalMetadata.Savers;
using MediaBrowser.MediaEncoding.BdInfo;
using MediaBrowser.Model.Configuration;
using MediaBrowser.Model.Cryptography;
using MediaBrowser.Model.Dlna;
using MediaBrowser.Model.Globalization;
@@ -266,16 +269,14 @@ public virtual bool CanLaunchWebBrowser
LoggerFactory = loggerFactory;
_fileSystemManager = fileSystem;

Logger = LoggerFactory.CreateLogger<ApplicationHost>();

ConfigurationManager = new ServerConfigurationManager(ApplicationPaths, LoggerFactory, _xmlSerializer, _fileSystemManager);
// Have to migrate settings here as migration subsystem not yet initialised.
MigrateNetworkConfiguration();

// Have to pre-register the NetworkConfigurationFactory, as the configuration sub-system is not yet initialised.
ConfigurationManager.RegisterConfiguration<NetworkConfigurationFactory>();
NetManager = new NetworkManager((IServerConfigurationManager)ConfigurationManager, LoggerFactory.CreateLogger<NetworkManager>());

Logger = LoggerFactory.CreateLogger<ApplicationHost>();

_startupOptions = options;
_startupConfig = startupConfig;

@@ -299,22 +300,6 @@ public virtual bool CanLaunchWebBrowser
ApplicationVersion);
}

/// <summary>
/// Temporary function to migration network settings out of system.xml and into network.xml.
/// TODO: remove at the point when a fixed migration path has been decided upon.
/// </summary>
private void MigrateNetworkConfiguration()
{
string path = Path.Combine(ConfigurationManager.CommonApplicationPaths.ConfigurationDirectoryPath, "network.xml");
if (!File.Exists(path))
{
var networkSettings = new NetworkConfiguration();
ClassMigrationHelper.CopyProperties(ServerConfigurationManager.Configuration, networkSettings);
_xmlSerializer.SerializeToFile(networkSettings, path);
Logger?.LogDebug("Successfully migrated network settings.");
}
}

public string ExpandVirtualPath(string path)
{
var appPaths = ApplicationPaths;

This file was deleted.

@@ -103,16 +103,6 @@ public string BaseUrl
/// <value>The public mapped port.</value>
public int PublicPort { get; set; } = DefaultHttpPort;

/// <summary>
/// Gets or sets a value indicating whether the http port should be mapped as part of UPnP automatic port forwarding.
/// </summary>
public bool UPnPCreateHttpPortMap { get; set; }

/// <summary>
/// Gets or sets the UDPPortRange.
/// </summary>
public string UDPPortRange { get; set; } = string.Empty;

/// <summary>
/// Gets or sets a value indicating whether gets or sets IPV6 capability.
/// </summary>
@@ -123,29 +113,6 @@ public string BaseUrl
/// </summary>
public bool EnableIPV4 { get; set; } = true;

/// <summary>
/// Gets or sets a value indicating whether detailed SSDP logs are sent to the console/log.
/// "Emby.Dlna": "Debug" must be set in logging.default.json for this property to have any effect.
/// </summary>
public bool EnableSSDPTracing { get; set; }

/// <summary>
/// Gets or sets the SSDPTracingFilter
/// Gets or sets a value indicating whether an IP address is to be used to filter the detailed ssdp logs that are being sent to the console/log.
/// If the setting "Emby.Dlna": "Debug" msut be set in logging.default.json for this property to work.
/// </summary>
public string SSDPTracingFilter { get; set; } = string.Empty;

/// <summary>
/// Gets or sets the number of times SSDP UDP messages are sent.
/// </summary>
public int UDPSendCount { get; set; } = 2;

/// <summary>
/// Gets or sets the delay between each groups of SSDP messages (in ms).
/// </summary>
public int UDPSendDelay { get; set; } = 100;

/// <summary>
/// Gets or sets a value indicating whether address names that match <see cref="VirtualInterfaceNames"/> should be Ignore for the purposes of binding.
/// </summary>
@@ -156,43 +123,18 @@ public string BaseUrl
/// </summary>
public string VirtualInterfaceNames { get; set; } = "vEthernet*";

/// <summary>
/// Gets or sets the time (in seconds) between the pings of SSDP gateway monitor.
/// </summary>
public int GatewayMonitorPeriod { get; set; } = 60;

/// <summary>
/// Gets a value indicating whether multi-socket binding is available.
/// </summary>
public bool EnableMultiSocketBinding { get; } = true;

/// <summary>
/// Gets or sets a value indicating whether all IPv6 interfaces should be treated as on the internal network.
/// Depending on the address range implemented ULA ranges might not be used.
/// </summary>
public bool TrustAllIP6Interfaces { get; set; }

/// <summary>
/// Gets or sets the ports that HDHomerun uses.
/// </summary>
public string HDHomerunPortRange { get; set; } = string.Empty;

/// <summary>
/// Gets or sets the PublishedServerUriBySubnet
/// Gets or sets PublishedServerUri to advertise for specific subnets.
/// </summary>
public string[] PublishedServerUriBySubnet { get; set; } = Array.Empty<string>();

/// <summary>
/// Gets or sets a value indicating whether Autodiscovery tracing is enabled.
/// </summary>
public bool AutoDiscoveryTracing { get; set; }

/// <summary>
/// Gets or sets a value indicating whether Autodiscovery is enabled.
/// </summary>
public bool AutoDiscovery { get; set; } = true;

/// <summary>
/// Gets or sets the filter for remote IP connectivity. Used in conjuntion with <seealso cref="IsRemoteIPFilterBlacklist"/>.
/// </summary>
@@ -9,13 +9,18 @@
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
using System.Xml.Serialization;
using CommandLine;
using Emby.Server.Implementations;
using Emby.Server.Implementations.IO;
using Emby.Server.Implementations.Serialization;
using Jellyfin.Api.Controllers;
using Jellyfin.Networking.Configuration;
using MediaBrowser.Common.Configuration;
using MediaBrowser.Common.Net;
using MediaBrowser.Controller.Extensions;
using MediaBrowser.Model.Configuration;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Server.Kestrel.Core;
using Microsoft.Extensions.Configuration;
@@ -158,6 +163,8 @@ private static async Task StartApp(StartupOptions options)
ApplicationHost.LogEnvironmentInfo(_logger, appPaths);

PerformStaticInitialization();
MigrateNetworkSettings(_logger, appPaths);

var serviceCollection = new ServiceCollection();

var appHost = new CoreAppHost(
@@ -647,5 +654,78 @@ private static string NormalizeCommandLineArgument(string arg)

return "\"" + arg + "\"";
}

private static void MigrateNetworkSettings(ILogger logger, ServerApplicationPaths appPaths)

This comment has been minimized.

@cvium

cvium Mar 13, 2021
Member Outdated

This should really be in a separate file. I also don't really like that it uses current classes. If someone were to apply this migration in 10.8 it might not work as expected. I really want migrations to be version-agnostic if possible.

This comment has been minimized.

@BaronGreenback

BaronGreenback Mar 13, 2021
Author Contributor Outdated

You mean a static oldXml.value -> newXml value migration?

This comment has been minimized.

@cvium

cvium Mar 13, 2021
Member Outdated

Pretty much yes. Something that can be run if you're migrating from 10.6 to whatever.

This comment has been minimized.

@BaronGreenback

BaronGreenback Mar 15, 2021
Author Contributor Outdated

Moved migration code into the Jellyfin.Server\Migrations where the rest resides.

Tried static migration but it turned into coding nighmare with pages of string comparisons, so i decided to look at the original code to see what can be done.

For the properties to be absent from ServerConfiguration, the file has to be treated as an XML. The callback function pemits us to intercept properties that are no longer in the class, so the migration only works with properties that are not longer valid in ServerConfiguration. If any properties are redefined in ServerConfiguration in the future, they will not be migrated.

The second part is not ideal - instead of migrating if network.xml is not present, it only migrates once using the migrate.xml similar to other migration functions. However, since CommonConfiguration overwrites system.xml with a 'clean' version at creation, the existing Migration routine cannot be used, so i've had to add the config by hand. (not ideal, but i can't find another solution without altering lots of other code).

The advantage of this is that all migration results are in one place.

This comment has been minimized.

@cvium

cvium Mar 16, 2021
Member Outdated

If it makes it easier I would just make a config_migrations.xml.

And then to make it even simpler I would create two private classes that contains the properties you want to migrate; one for system.xml and one for network.xml. Then utilize the xmlserializer to move it from A to B. The end.

This comment has been minimized.

@BaronGreenback

BaronGreenback Mar 22, 2021
Author Contributor Outdated

One phrase for this one - ball ache!

What i've learnt so far, on what should have been a simple thing.

Serialization classes cannot be private - best they can be is internal.
You cannot typecast on a serialiser.

I had originally thought to declare two classes, the original inheritied from the network - that way properties would only need to declared once. The plan was then to read from source into ServerConfig, and write typecasting. Nope - writing saves the original class' object into the file.

So i've done the next best thing. desearialised into legacy class. Copied properties. Save into network class.

{
var destFile = Path.Combine(appPaths.ConfigurationDirectoryPath, "network.xml");
if (!File.Exists(destFile))
{
var settings = new NetworkConfiguration();
var settingsType = typeof(NetworkConfiguration);
var props = settingsType.GetProperties().Where(x => x.CanWrite).ToList();

// manually load source xml file.
var serializer = new XmlSerializer(typeof(ServerConfiguration));
serializer.UnknownElement += (object? sender, XmlElementEventArgs e) =>
{
var p = props.Find(x => x.Name == e.Element.Name);
if (p != null)
{
if (p.PropertyType == typeof(bool))
{
bool.TryParse(e.Element.InnerText, out var boolVal);
p.SetValue(settings, boolVal);
}
else if (p.PropertyType == typeof(int))
{
int.TryParse(e.Element.InnerText, out var intVal);
p.SetValue(settings, intVal);
}
else if (p.PropertyType == typeof(string[]))
{
var items = new List<string>();
foreach (XmlNode el in e.Element.ChildNodes)
{
items.Add(el.InnerText);
}

p.SetValue(settings, items.ToArray());
}
else
{
try
{
p.SetValue(settings, e.Element.InnerText ?? string.Empty);
}
catch
{
logger.LogDebug(
"Unable to migrate value {Name}. Unknown datatype {DataType}. Value {Value}.",
e.Element.Name,
p.PropertyType,
e.Element.InnerText);
}
}
}
};

ServerConfiguration? deserialized;
try
{
using (StreamReader reader = new StreamReader(appPaths.SystemConfigurationFilePath))
{
deserialized = (ServerConfiguration?)serializer.Deserialize(reader);
}
}
catch (Exception ex)
{
// Catch everything, so we don't bomb out JF.
logger.LogDebug(ex, "Exception occurred migrating settings.");
}

var xmlSerializer = new MyXmlSerializer();
xmlSerializer.SerializeToFile(settings, destFile);
}
}
}
}
ProTip! Use n and p to navigate between commits in a pull request.