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

Changed the way we match mappings in NameToInternalName, modified and added new testcases #199

Merged
merged 3 commits into from
Jun 2, 2022

Conversation

isc-svelury
Copy link
Contributor

@isc-svelury isc-svelury commented May 16, 2022

Closes #198.

The new mapping match method is more robust than the previous one. It is described in detail below. We also provide more detailed explanations on what went wrong when adding a file to source control.

Matching mappings

For every mapping we have we can compute a composite score that describes how strong the match is. There are 3 main factors that need to be considered here.

Path matches

Here we have to check if the path in the ExternalName matches the path specified by the mapping.

  1. Not a match
  2. Path match

Extension matches

We confirm that the extension of the file in the ExternalName is the same as the extension in the mapping.

  1. Not a match
  2. Extension match

Note: There is a possibility that the extension of some files might be different from the one in the mapping. e.g. .dfi files might be exported out as XML files. If the extension does the export, then we export the .dfi file as-is, so we don't have to worry about this edge case. However, we also do a Load() of the file to try and get the InternalName directly and that should catch cases like the one in the example.

Coverage matches

  1. Not a match
  2. "*" match
  3. Specific package match

Composite scores

Based on the match types above we have 12 possible composite scores.

image

Tie-breaker rules

If we need a tie-breaker, we consider a specfic match to be a better match. Specificity, for both Coverage and Path, is defined as being directly proportional to the length of the string. Thus, there are 4 possible scenarios:

  1. Coverage is more specific in one and Path is the same.
  2. Path is more specific in one and Coverage is the same.
  3. Coverage is more specific in one while Path is more specific in the other.
  4. Both are more specific in one.

We give coverage a higher priority. Note that a file extension has no notion of specificity.

Algorithm

We iterate through all the configured mappings and compute a composite score for each with respect to the Name function parameter. We keep track of the best match found up till that point and update it as we get a better match. In case, we get a mapping with the same composite score as the best one yet, we use the tie-breaker rules to resolve the best match. If the best match has a score >= 111, we have found a valid match. Otherwise we have not and provide explanations for what might have gone wrong.

@isc-svelury isc-svelury added bug Something isn't working enhancement New feature or request labels May 16, 2022
@isc-svelury isc-svelury requested a review from isc-tleavitt May 16, 2022 14:56
Copy link
Collaborator

@isc-tleavitt isc-tleavitt left a comment

Choose a reason for hiding this comment

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

Main comment: We need deeper unit tests. Particularly, it would be good to have the various unmapped cases covered.

test/UnitTest/SourceControl/Git/NameToInternalNameTest.cls Outdated Show resolved Hide resolved
@isc-svelury isc-svelury requested a review from isc-tleavitt May 24, 2022 18:32
@isc-tleavitt isc-tleavitt merged commit 31c652e into main Jun 2, 2022
@isc-tleavitt isc-tleavitt deleted the fix-mapping-match branch June 2, 2022 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using 2.0, still unable to use Add command on schemas
2 participants