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

refactor(bbb-config): More robust updates of recordings' "metadata.xml" when issuing "bbb-conf --setip" #12682

Merged

Conversation

steffen-moser
Copy link

What does this PR do?

"bbb-conf --setip " updates a lot of configuration files. Among other things, it also updates the "metadata.xml" files of recordings in order to have URLs pointing to the new hostname. The primarily used "sed" commands did not touch URLs when linebreaks have been between the opening XML tags and the URLs themselves or when the XML attributes are in a different order.

The PR should fix this by using "xmlstarlet" which has been already used by "bbb-conf" for other changes/updates.

Closes Issue(s)

Closes #12628

Motivation

Altering XML files with a line-based stream editor (like "sed") is not a good idea in general. It is always prone to exceptions to the pattern a programmer has in mind. Especially, the XML files for rather old recordings might have been generated by scripts that have been exchanged for years. Nobody thinks about them. Therefore, we need a robust mechanism to update the URLs. Of course, "sed" could be tweaked to parse patterns across multiple lines, but it still remains a fragile construct, e.g., when the format of "metadata.xml" might be updated in th future.

@github-actions
Copy link

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@steffen-moser steffen-moser changed the base branch from develop to v2.3.x-release June 29, 2021 11:38
@steffen-moser steffen-moser changed the title More robust updates of recordings' "metadata.xml" when using "bbb-conf --setip" More robust updates of recordings' "metadata.xml" when issuing "bbb-conf --setip" Jun 29, 2021
@steffen-moser steffen-moser marked this pull request as ready for review June 29, 2021 11:40
@sonarcloud
Copy link

sonarcloud bot commented Jun 29, 2021

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 11 Code Smells

No Coverage information No Coverage information
6.4% 6.4% Duplication

@steffen-moser
Copy link
Author

Unfortunately, I cannot see what kind of code smells SonarCloud has detected – otherwise I would have been fixing it already.
I cannot log in with my Github account to SonarCloud to find details.

O.K., there is still some redundancy left (which probably could have been resolved by using another loop around the "xmlstarlet" calls). Nevertheless, when comparing to the former version of the code (containing a lot of "sed" commands), it should have reduced possible redundancies a lot.

@antobinary antobinary removed the request for review from ffdixon November 10, 2021 19:31
@antobinary antobinary changed the base branch from v2.3.x-release to v2.4.x-release November 10, 2021 19:31
@antobinary antobinary self-requested a review November 10, 2021 19:32
@antobinary antobinary added this to the Release 2.4 milestone Nov 10, 2021
@antobinary antobinary merged commit 82d8e92 into bigbluebutton:v2.4.x-release Nov 19, 2021
@antobinary antobinary changed the title More robust updates of recordings' "metadata.xml" when issuing "bbb-conf --setip" refactor(bbb-config): More robust updates of recordings' "metadata.xml" when issuing "bbb-conf --setip" Nov 23, 2021
@GhaziTriki
Copy link
Member

GhaziTriki commented Dec 9, 2021

Tested in production with 2.3 and working. @antobinary Can we please port back this PR to 2.3.x? Very hand when moving the recordings from the server to another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrating recordings to a new server: "bbb-conf --setip" does not update all types of URLs
3 participants