Undeprecated password generation methods.

[bodiam]

I've removed the deprecation annotations here, it's really not clear to me why these methods were deprecated, and what their replacement was.

Eg, someone used to be able to do faker.internet().password(). How would someone do this now?

Like this?

          faker.text().text(Text.TextSymbolsBuilder.builder()
                      .len(5)
                      .with(EN_LOWERCASE, 1)
                      .with(EN_UPPERCASE, 1)
                      .with(DIGITS, 1);

That seems overly complex for a password. If I'm missing something, please let me know, because I'm not sure what happened here.

[snuyanzin]

The methods are not doing what they are saying

e.g. there is an arg: upperCase however under the hood it means only english uppercase same for special characters. No chance to customize character set.

I would suggest rather to create some helper methods with useful defaults

[kingthorin]

Shall we have these use the text methods under the hood? (Abstracting the percepted difficulty(ies) from expecting users to use them directly for passwords?)

If so I'd be happy to look at doing the changes.

I think one of the reasons for this change was a limitation or two around the existing (deprecated) methods. I'll have to dig for the issues/PRs and update this later.

Edit: maybe they already do under the hood 🤷‍♂️, I haven't checked anything outside the PR diff.

[bodiam]

The methods are not doing what they are saying

e.g. there is an arg: upperCase however under the hood it means only english uppercase same for special characters.

I think for a password, that's a reasonable assumption to make?

No chance to customize character set.

If you want, there's nothing to stop you from using the text() methods

I would suggest rather to create some helper methods with useful defaults

Isn't that what password() does in essence?

[bodiam]

Shall we have these use the text methods under the hood? (Abstracting the percepted difficulty(ies) from expecting users to use them directly for passwords?)

If so I'd be happy to look at doing the changes.

I think one of the reasons for this change was a limitation or two around the existing (deprecated) methods. I'll have to dig for the issues/PRs and update this later.

Edit: maybe they already do under the hood 🤷‍♂️, I haven't checked anything outside the PR diff.

I just had a quick look, and all password() methods are delegated to return faker.text().text(minimumLength, maximumLength, includeUppercase, includeSpecial, includeDigit);, which is using the TextSymbolsBuilder under the hood.

So, I'm not really sure what the issue is.

[kingthorin]

That's fine with me. Sorry I couldn't actually check earlier, was using my cell on a city bus.

[snuyanzin]

From one side yes, from another side IIRC current implementation of password in Internet historically doesn't guarantee lowercase letters and this leads to such kind of issues https://github.com/datafaker-net/datafaker/issues/1250

[bodiam]

@snuyanzin isn't that the whole point of this library, that the results are random? There's no guarantee or requirement that a password should have a lower case character, a number, or a special character. We could change it to make sure it always has at least one of the above, but either way, it's still a password. Either way, I don't see why we have to break the API for this.

[kingthorin]

I think it's okay to say deprecate them. It's using the newer code under the hood and if users aren't happy with how it behaves they can use text to have even greater control.

Best of all worlds.

[snuyanzin]

There's no guarantee or requirement that a password should have a lower case character, a number, or a special character.

that's the point that the api is unclear

e.g. looking at https://github.com/datafaker-net/datafaker/blob/6659d9388879e5a07747314bdc3162c07c8ba20a/src/main/java/net/datafaker/providers/base/Internet.java#L223

what I should expect passing true for includeUppercase? How to get same behavior for lowercase? Usually there are rules for passwords to use in different companies and the idea is not to generate something random because it is random lib... The idea is to be able to config generation of something satisfying the rules

[erikpragt-connectid]

I don't disagree that the password(many argument) methods are confusing. But to me, that doesn't immediately mean we should deprecate all the password methods and replace it by something much more complex. It's complex enough that we even need an entry in our documentation to generate passwords (https://www.datafaker.net/documentation/password/), and while that's advanced password generation, we no longer have a "simple password generation", and I don't think that's ideal. It took me longer than I needed to generate a password, and I really don't think it should be more complex than internet().password(), or internet.password(min, max). Those (includeLower, includeUpper, includeDigits), well, that's a bit of a different story, but I don't see much harm in keeping those methods. If someone really wants an includeLower(), they can either send a PR to include that, or use the TextSymbolsBuilder, which is a far better and more flexible way, but a bit more complex in usage.

[snuyanzin]

I'm ok with leaving undeprecated internet().password() and internet().password(min, max)

however for others I would vote for deprecation and removal in future

instead imho it would make sense to add internet().password(TextSymbolsBuilder)

[kingthorin]

I think that's also a good middle ground type option.

[bodiam]

I'm ok with leaving undeprecated internet().password() and internet().password(min, max)

however for others I would vote for deprecation and removal in future

instead imho it would make sense to add internet().password(TextSymbolsBuilder)

That I'm okay with, I'm not a fan of password(true, false, true) kind of methods, it's tricky to understand what they do without going into the details of this. I'll deprecate the unintuitive methods.