skwasjer / IbanNet

C# .NET IBAN validator, parser, builder and generator
Apache License 2.0
119 stars 31 forks source link

Branch not calculated for french iban #77

Closed DaRochaRomain closed 1 year ago

DaRochaRomain commented 1 year ago

Hello,

It seems that there is an issue parsing french iban. For exemple, if you try to parse this iban :

var ibanParser = new IbanParser(IbanRegistry.Default);
var isValid = ibanParser.TryParse("FR7630001007941234567890185", out var iban);
var bank = iban.BankIdentifier;
var branch = iban.BranchIdentifier;

the BranchIdentifier is null instead of being 00794

Is there a way to fix that on my side?

DaRochaRomain commented 1 year ago

Issue seems to be there : https://github.com/skwasjer/IbanNet/blob/458835aa98b94210261ca437c7752cbbd90a0c5a/src/IbanNet/Registry/Swift/SwiftRegistryProvider.cs#L915 There is no branch property set

Should be something like that if I'm correct

Branch = new BranchStructure(new SwiftPattern("5!n"), 9)
{
    Example = "01005"
}
skwasjer commented 1 year ago

It seems to not be part of the official SWIFT spec, but I don't know why. The registry is generated directly from that spec. Hence it is not really feasible to maintain a manual fix because it would be overwritten again the next time a new version of the spec is released.

You can however write a new registry provider (see IIbanRegistryProvider) which just declares the (adjusted) French registry definition and load it before loading the default SwiftRegistryProvider (https://github.com/skwasjer/IbanNet/wiki/Options#other-registry-providers) is loaded. Or you can write a decorator for the SwiftRegistryProvider. Either way, I don't see an easy way to update the existing spec.

skwasjer commented 1 year ago

I will give it some more thought for a future version of IbanNet. A way to define 'unofficial fixes' could perhaps be an opt-in addition to the library.

DaRochaRomain commented 1 year ago

I don't realy get how it doesn't seems to be part of the swift spec, when your current pattern "5!n5!n11!c2!n" is already specifying 5 digits for the bank, 5 digits for the branch and 11 digits for the account number.

Edit : I get it, I just checked the pdf and they missed the branch pattern...

Edit 2 : I can't realy write a custom provider just to copy and fix the French country since some of your classes, like SwiftPattern, are internal.

Edit 3 : I managed to do something using reflection.

public class FixIbanRegistryProvider : IIbanRegistryProvider
    {
        private SwiftRegistryProvider _swiftRegistryProvider = new SwiftRegistryProvider();

        public int Count => _swiftRegistryProvider.Count;

        public IEnumerator<IbanCountry> GetEnumerator()
        {
            foreach(var country in _swiftRegistryProvider)
            {
                if (country.TwoLetterISORegionName == "FR")
                {
                    var ibanNetAssembly = Assembly.Load("IbanNet");
                    var swiftPattern = ibanNetAssembly.CreateInstance("IbanNet.Registry.Swift.SwiftPattern", false, BindingFlags.Default, null, new object[] { "5!n" }, null, new object[0]);
                    var countryType = country.GetType();
                    var branchProperty = countryType.GetProperty(nameof(IbanCountry.Branch));
                    var branchStructure = (BranchStructure)Activator.CreateInstance(typeof(BranchStructure), new object[] { swiftPattern, 9 });

                    branchProperty.SetValue(country, branchStructure);
                }

                yield return country;
            }
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }
    }

you should consider making some class public and some property settable.

skwasjer commented 1 year ago

Yea, clearly this is not so nice. I will probably include an 'unofficial' fix for this and perhaps revisit some of the type visibility so reflection is not needed. I would still like to retain that the registry related types keep their immutable behavior once the registry is fully loaded, so it requires some rework (sounds like a good use case for record type and with).

skwasjer commented 1 year ago

I don't know why I didn't think of it before, but it can be addressed at the code gen level, see PR #78. Let me know if this satisfies your need and I will merge and deploy the fix.

DaRochaRomain commented 1 year ago

Looks fine to me.

I didn't check other countries but you should also take a look about wise.com for iban structure. With swift.com you're also missing the AccountNumber and NationalCode extraction.