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

fix: use parent xml to parse child #470

Merged
merged 3 commits into from
Oct 11, 2021
Merged

Conversation

shetzel
Copy link
Contributor

@shetzel shetzel commented Oct 8, 2021

What does this PR do?

Reuses the parsed custom labels file when parsing the children to improve conversion performance.

What issues does this PR fix or reference?

forcedotcom/cli#1194
@W-9990964@

Functionality Before

converting CustomLabels would take a long time since it would read and parse the CustomLabels.labels file for each label (child) within the file.

Functionality After

The CustomLabels.labels file is read and parsed once, then reused to parse the children.

Testing Notes:

  • Run the source plugin label NUTs with this branch linked and ensure everything passes.
  • Manually test conversion (force:source:convert) of all custom labels using sourcepath, metadata, and manifest flags. Use a project with lots of labels and multiple package directories. Ensure the converted file contents and package.xml looks correct.
  • Test converting individual labels, both from within the same package directory and within multiple package dirs. Ensure the converted file contents and package.xml looks correct.
  • Test converting CustomObjects and ensure nothing has regressed
  • Test converting other non-decomposed types with children such as Workflows or AssignmentRules

@shetzel shetzel requested review from a team as code owners October 8, 2021 18:32
@shetzel shetzel requested a review from randi274 October 8, 2021 18:32
@WillieRuemmele
Copy link
Member

QA notes:

  • converts CustomLabels across MPDs, single packages
  • plugin-source NUTs pass
  • 8k convert 10mins -> 13s
  • CustomObjects work
  • other nonDecomposed types (AssignmentRule) work

@WillieRuemmele WillieRuemmele merged commit 440d2be into main Oct 11, 2021
@WillieRuemmele WillieRuemmele deleted the sh/CustomLabels-transform branch October 11, 2021 21:49
@@ -49,9 +49,12 @@ export interface RecompositionState {
class RecompositionFinalizer extends ConvertTransactionFinalizer<RecompositionState> {
protected _state: RecompositionState = {};

// A cache of SourceComponent xml file paths to parsed contents so that identical child xml
// files are not read and parsed multiple times.
private parsedXmlCache = new Map<string, JsonMap>();
Copy link
Contributor

@mshanemc mshanemc Oct 11, 2021

Choose a reason for hiding this comment

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

why keep this around at the class level if it's only used inside one recompose method? It could be kinda big for those large-number-of-labels scenarios.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be really small actually. In the case of custom labels there would only be 1 entry per CustomLabels.labels-meta.xml file, which is only 1 per package directory. This could possibly be moved to within the recompose function but I wasn't sure if there were situations where multiple parent CustomLabels would exist in a ComponentSet so to be safe I added this as an instance var.

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.

3 participants