Removed duplicated condition for allowedUrls during interceptor logic and make it optional

[adrianbenjuya]

Before this change user needed to define both allowedUrls and customUrlValidation to make customUrlValidation work, otherwise it was never executed.

[jeroenheijmans]

Took me some time, I think the title of the PR suggests only a deduplication effort, but you are in fact proposing a minor breaking change I think? That is, your new version behaves different in this example scenario:

{
  resourceServer: { 
    customUrlValidation: url => false, // might be complex logic
    allowedUrls: undefined, // !!
  }
}

That's intended, right?

(Shame we have no tests yet, they would help a lot here...)

[adrianbenjuya]

Took me some time, I think the title of the PR suggests only a deduplication effort, but you are in fact proposing a minor breaking change I think? That is, your new version behaves different in this example scenario:

{
  resourceServer: { 
    customUrlValidation: url => false, // might be complex logic
    allowedUrls: undefined, // !!
  }
}

That's intended, right?

(Shame we have no tests yet, they would help a lot here...)

Yep, the title is incorrect, I just edited it

Indeed, that's what I want to achieve... right now, to make customUrlValidation work I need to define something like this

{
  resourceServer: { 
    customUrlValidation: url => false,
    allowedUrls: []
  }
}

But I expect to define this

{
  resourceServer: { 
    customUrlValidation: url => false
  }
}

Because I'm not interested in setup allowedUrls, just the validations function

[jeroenheijmans]

👍 LGTM then. Something for a major release, even though the breaking change is small.

[jeroenheijmans]

Fixes the linked issue AFAICT.

Tiny breaking change (for the better), something for v9?

[davebaol]

Tiny breaking change (for the better), something for v9?

Yep, technically this PR is a breaking change.
However I'm not sure it can be reasonably considered a breaking change, not even minor.

I mean, the only difference is that currently the config below does nothing while with this PR it would execute custom URL validation

{
  resourceServer: { 
    customUrlValidation: url => { /* some complex logic */ },
    allowedUrls: undefined
 }
}

So the real question is "How likely is there someone out there currently using a pointless configuration like that?"
I'd say nobody at all...... Why waiting for v9?

My Two Cents