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

feat: Support Wildcard and * contract filters #567

Merged

Conversation

Kevin101Zhang
Copy link
Contributor

@Kevin101Zhang Kevin101Zhang commented Feb 16, 2024

added the * filter for contracts.
A valid contract is defined as

function validateContractId(accountId) {
  accountId = accountId.trim();
  
  const isLengthValid = accountId.length >= 2 && accountId.length <= 64;
  if (!isLengthValid) return false; 

  //test if the string starts with a '*.' and remove it if it does
  const isWildCard = WILD_CARD_REGEX.test(accountId);
  isWildCard ? accountId = accountId.slice(2) : null;

  //test if rest of string is valid accounting for/not isWildCard
  const isRegexValid = CONTRACT_NAME_REGEX.test(accountId);
  return isRegexValid;
}

note: early termination for performance

The function take in account all invalid contract names on nomicon
Please view test for more details.

Regex Broken down Below:

CONTRACT_NAME_REGEX:

/^(([a-z\d]+[-_])*[a-z\d]+(\.([a-z\d]+[-_])*[a-z\d]+)*\.([a-z\d]+)|([a-z\d]+))$/

  1. ^: Asserts the start of the string.

  2. (: Begins a capturing group.

  3. ([a-z\d]+[-_])*: Matches a sequence of lowercase letters or digits, possibly followed by hyphens or underscores. The sequence may repeat zero or more times.

  4. [a-z\d]+: Matches a sequence of one or more lowercase letters or digits.

  5. (.([a-z\d]+[-_])[a-z\d]+): Allows for zero or more occurrences of a dot . followed by another sequence.

  6. .([a-z\d]+): Matches a dot followed by a sequence of lowercase letters or digits.

  7. |: OR operator.

  8. ([a-z\d]+): Matches a standalone sequence of lowercase letters or digits.

  9. )$: Asserts the end of the string.
    WILD_CARD_REGEX:

/\*\./

  1. *: Matches a literal asterisk.
  2. .: Matches a literal dot.

EDIT: Added more test please review
Added jest to support function testing.

It seems there's a problem getting Babel and the VM to work smoothly together. I'm going to dig deeper into it. Right now, I'm adjusting the test file to recreate the functions instead of using the usual require/import method, which seems to cause trouble with the VM code. It does mean there's some duplicated code in the testing files, but the test work.

@Kevin101Zhang Kevin101Zhang requested a review from a team as a code owner February 16, 2024 21:51
Copy link
Collaborator

@darunrs darunrs left a comment

Choose a reason for hiding this comment

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

Some comments on the updated regex, and testing methodology. Great job adding jest here!

Copy link
Collaborator

@darunrs darunrs left a comment

Choose a reason for hiding this comment

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

Mostly looks good to me.

I just want to clarify once more that we want to support* as that seems correct to me. I checked with Pavel and he confirmed what I thought about supporting * alone.

Also dropped a suggestion for fixing that babel issue. If it works, awesome. If not, no big deal, we can defer it. Leave a TODO in the test file to replace with imports later.

frontend/src/utils/validators.js Outdated Show resolved Hide resolved
@@ -1 +1,2 @@
export const CONTRACT_NAME_REGEX = RegExp(/^(\*|([a-z\d]+[-_])*[a-z\d]+)(\.*(\*|([a-z\d]+[-_])*[a-z\d]+))*\.(\w+)$/);
export const CONTRACT_NAME_REGEX = RegExp(/^(([a-z\d]+[-_])*[a-z\d]+(\.([a-z\d]+[-_])*[a-z\d]+)*\.([a-z\d]+)|([a-z\d]+))$/);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice job on the regex!

Copy link
Collaborator

@darunrs darunrs left a comment

Choose a reason for hiding this comment

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

Looks good to me. There's a merge conflict but otherwise, should be good.

Add a comment above the original validate functions that they're duplicated in the test files due to a Babel config issue, and a TODO to address that.

Also, can you rename the PR to something like feat: Support Wildcard and * contract filters?

@darunrs darunrs mentioned this pull request Feb 21, 2024
@Kevin101Zhang Kevin101Zhang changed the title 541 update the frontend to dis allow use of this filter feat: Support Wildcard and * contract filters Feb 22, 2024
@Kevin101Zhang Kevin101Zhang changed the title feat: Support Wildcard and * contract filters Feat: Support Wildcard and * contract filters Feb 22, 2024
@Kevin101Zhang Kevin101Zhang merged commit 761a9a9 into main Feb 26, 2024
@Kevin101Zhang Kevin101Zhang deleted the 541-update-the-frontend-to-dis-allow-use-of-this-filter branch February 26, 2024 19:03
@darunrs darunrs mentioned this pull request Mar 8, 2024
@Kevin101Zhang Kevin101Zhang changed the title Feat: Support Wildcard and * contract filters feat: Support Wildcard and * contract filters Jul 24, 2024
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.

Update the frontend to dis-/allow use of this filter
3 participants