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

ArrayClassUnique, or property-based uniqueness testing #602

Closed
lgarczyn opened this issue May 2, 2020 · 5 comments · Fixed by #830
Closed

ArrayClassUnique, or property-based uniqueness testing #602

lgarczyn opened this issue May 2, 2020 · 5 comments · Fixed by #830
Labels
status: done/released Issue has been completed, no further action is needed. type: feature Issues related to new features.

Comments

@lgarczyn
Copy link

lgarczyn commented May 2, 2020

I would be picturing something like

@ArrayClassUnique(User, (u:User) => u.name)
users:User[]

with

function arrayClassUnique2(array: unknown, args: ValidationArguments): boolean {
    if (!(array instanceof Array))
        return false;

    const constructor = args.constraints[0];
    const fn = args.constraints[1];
    const ids = array.map((v: unknown) => {
        if (isInstance(v, constructor)) {
            return fn(v);
        } else {
            return null;
        }
    });

    const uniqueItems = ids.filter((a, b, c) => { return c.indexOf(a) === b; });

    return array.length === uniqueItems.length;
}

export function arrayClassUnique(value: unknown, args: ValidationArguments): boolean {

    if (!(value instanceof Array))
        return false;

    const constructor = args.constraints[0];
    const fn = args.constraints[1];
    const ids = value.map((v: unknown) => {
        if (isInstance(v, constructor)) {
            return fn(v);
        } else {
            return null;
        }
    });
    const uids = new Set(ids);

    return uids.size == value.length
        && uids.has(null) == false
        && uids.has(undefined) == false;
}

/**
 * Checks if all array's values are unique. Comparison for objects is reference-based.
 * If null or undefined is given then this function returns false.
 */
export function ArrayClassUnique<T>(
    targetType: new (...args: unknown[]) => T,
    uidFn: (T) => Primitive,
    validationOptions?: ValidationOptions): PropertyDecorator {
    return ValidateBy(
        {
            name: ARRAY_CLASS_UNIQUE,
            constraints: [targetType, uidFn],
            validator: {
                validate: (value, args): boolean => { return arrayClassUnique(value, args); },
                defaultMessage: buildMessage(
                    (eachPrefix) => { return eachPrefix + 'All $property\'s elements must be unique'; },
                    validationOptions
                )
            }
        },
        validationOptions
    );
}

Type checking is a bit overboard, and should probably be done using IsInstance, but it allowed for stronger typing inside the lambda function. If it is kept, it should probably have its own message.

@vlapo vlapo added the type: feature Issues related to new features. label May 4, 2020
@vlapo
Copy link
Contributor

vlapo commented May 4, 2020

Maybe something more general. E.g. @ArrayUnique<T>((value: T) => any)?

@lgarczyn
Copy link
Author

lgarczyn commented May 5, 2020

That would work, but limiting it to objects, which currently can only be made unique through refs, makes type safety easier

@vlapo
Copy link
Contributor

vlapo commented May 5, 2020

Why limiting this validator only to objects? I think it is common functionality validate array of unique strings or numbers too.

Signature should be like:

@ArrayUnique<T = any>(selector?: (value: T) => any)
property: T[];

And possible cases:

@ArrayUnique()
property: string[] = ['hello', 'world', 'world']; // error

@ArrayUnique()
property2: number[] = [3, 3, 3, 4]; // error

@IsInstance(User, { each: true })
@ArrayUnique<User>(u => u.name)
property3: User[] = [new User('John), new User('Doe'), new User('John')]; // error

@Diluka
Copy link
Contributor

Diluka commented Dec 7, 2020

@vlapo I made a PR. Could you please review it.

@NoNameProvided NoNameProvided added the status: fixed Issues with merged PRs, but not released yet. label Jan 11, 2021
@NoNameProvided NoNameProvided added status: done/released Issue has been completed, no further action is needed. and removed status: fixed Issues with merged PRs, but not released yet. labels Jan 14, 2021
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: done/released Issue has been completed, no further action is needed. type: feature Issues related to new features.
Development

Successfully merging a pull request may close this issue.

4 participants