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

Feature/ircd 59 decouple configuration #8

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

joshoxe
Copy link
Collaborator

@joshoxe joshoxe commented May 23, 2021

No description provided.

@joshoxe joshoxe linked an issue May 23, 2021 that may be closed by this pull request
* LoggerElements are now initialised using IOptions
* Attributes now must be specified under a JSON section in the config

Task: #9
* Fixes a lot of the configuration not being deserialized correctly
* Removes a lot of code duplication from validation

Task: #9
Copy link
Member

@lewishazell lewishazell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ready. I'm happy to check server linking if need be.

Looks good!

using Microsoft.Extensions.Options;

namespace cmpctircd.Validation {
public class ConfigurationOptionsValidator : AbstractValidator<IOptions<ConfigurationOptions>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NICE

services.AddOptions<ServerOptions>().Bind(configuration);
services.AddOptions<LoggerOptions>().Bind(configuration);
//services.AddOptions<ConfigurationOptions>().Bind(configuration);
services.Configure<ConfigurationOptions>(configuration);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly how I imagined this working, I like!

@@ -65,7 +66,7 @@ public bool Handle(String packet, IRCd ircd, object sender, HandlerArgs args, Li

try {
// Restrict the commands which non-registered (i.e. pre PONG, pre USER/NICK) users can execute
if ((client.State.Equals(ClientState.PreAuth) || (ircd.Config.Advanced.ResolveHostnames && client.ResolvingHost)) && !registrationCommands.Contains(packet.ToUpper())) {
if ((client.State.Equals(ClientState.PreAuth) || (ircd.Config.Value.Advanced.ResolveHostnames && client.ResolvingHost)) && !registrationCommands.Contains(packet.ToUpper())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might be able to inject specific configs info PacketManager soon anyway. No problem with this.

@@ -49,12 +49,12 @@ public class OperController : ControllerBase {
}
// Instantiate the algorithm through reflection, if not already instantiated.
HashAlgorithm algorithm;
if(!_algorithms.TryGetValue(ircop.Algorithm, out algorithm))
algorithm = _algorithms[ircop.Algorithm] = (ircop.Algorithm.GetConstructor(Type.EmptyTypes)?.Invoke(new object[0]) as HashAlgorithm) ?? throw new IrcErrNoOperHostException(client);
if(!_algorithms.TryGetValue(ircop.AlgorithmType, out algorithm))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about this part. We'll figure something out.

We should test server linking is still working.

@joshoxe joshoxe marked this pull request as ready for review December 8, 2021 13:17
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

Successfully merging this pull request may close these issues.

Decouple configuration
2 participants