Skip to content

Commit 1856a35

Browse files
authored
Plugin Tool CLI: avoid NRE during package creation (#406)
* Update error handling during plugin CLI packaging * Follow docstrings * Organize imports * Rename variables * Move check * Return false
1 parent 855c985 commit 1856a35

File tree

1 file changed

+124
-14
lines changed

1 file changed

+124
-14
lines changed

src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Processing/PluginArtifactsProcessor.cs

Lines changed: 124 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
using Microsoft.Performance.Toolkit.Plugins.Core;
1515
using Microsoft.Performance.Toolkit.Plugins.Core.Metadata;
1616
using NuGet.Versioning;
17+
using LicenseInfo = Microsoft.Performance.Toolkit.Plugins.Core.Metadata.LicenseInfo;
1718
using ProcessingSourceInfo = Microsoft.Performance.Toolkit.Plugins.Core.Metadata.ProcessingSourceInfo;
19+
using ProjectInfo = Microsoft.Performance.Toolkit.Plugins.Core.Metadata.ProjectInfo;
1820

1921
namespace Microsoft.Performance.Toolkit.Plugins.Cli.Processing
2022
{
@@ -298,42 +300,150 @@ private bool TryGetAboutInfo(ProcessingSourceReference psr, [NotNullWhen(true)]
298300
{
299301
processingSourceInfo = null;
300302

303+
// Check if the GetAboutInfo method is overridden
304+
MethodInfo? methodInfo = psr.Instance.GetType().GetMethod(nameof(ProcessingSource.GetAboutInfo));
305+
if (methodInfo == null || methodInfo.DeclaringType == typeof(ProcessingSource))
306+
{
307+
this.logger.LogError($"Unable to generate processing source info: {psr.Name} does not override the virtual method {nameof(ProcessingSource)}.{nameof(ProcessingSource.GetAboutInfo)}.");
308+
return false;
309+
}
310+
301311
var aboutInfo = psr.Instance.GetAboutInfo();
302312

303313
if (aboutInfo == null)
304314
{
305-
this.logger.LogError($"Unable to generate processing source info: {psr.Name} does not contain about info.");
315+
this.logger.LogError($"Unable to generate processing source info: {psr.Name}'s {nameof(IProcessingSource.GetAboutInfo)} returned null.");
306316
return false;
307317
}
308318

309-
// Check if the GetAboutInfo method is overridden
310-
MethodInfo? methodInfo = psr.Instance.GetType().GetMethod(nameof(ProcessingSource.GetAboutInfo));
311-
if (methodInfo == null || methodInfo.DeclaringType == typeof(ProcessingSource))
319+
List<string> errors = new();
320+
321+
if (!TryGetOwners(psr, aboutInfo, out Core.Metadata.ContactInfo[]? owners, out string? ownersError))
312322
{
313-
this.logger.LogError($"Unable to generate processing source info: {psr.Name} does not override the virtual method {nameof(ProcessingSource)}.{nameof(ProcessingSource.GetAboutInfo)}.");
323+
errors.Add(ownersError);
324+
}
325+
326+
if (!TryGetProjectInfo(psr, aboutInfo, out ProjectInfo? projectInfo, out string? projectInfoError))
327+
{
328+
errors.Add(projectInfoError);
329+
}
330+
331+
if (!TryGetLicenseInfo(psr, aboutInfo, out LicenseInfo? licenseInfo, out string? licenseInfoError))
332+
{
333+
errors.Add(licenseInfoError);
314334
}
315335

316-
var owners = aboutInfo.Owners
317-
.Select(o => new Core.Metadata.ContactInfo(o.Name, o.Address, o.EmailAddresses, o.PhoneNumbers))
336+
if (errors.Any())
337+
{
338+
this.logger.LogError($"Unable to generate processing source info. The following error{ (errors.Count == 1 ? " was" : "s were") } encountered:{string.Join("", errors.Select(e => $"\n\t - {e}"))}");
339+
return false;
340+
}
341+
342+
processingSourceInfo = new ProcessingSourceInfo(owners, projectInfo, licenseInfo, aboutInfo.CopyrightNotice, aboutInfo.AdditionalInformation);
343+
return true;
344+
}
345+
346+
private bool TryGetOwners(
347+
ProcessingSourceReference processSource,
348+
Microsoft.Performance.SDK.Processing.ProcessingSourceInfo info,
349+
[NotNullWhen(true)] out Core.Metadata.ContactInfo[]? owners,
350+
[NotNullWhen(false)] out string? error)
351+
{
352+
error = null;
353+
owners = null;
354+
355+
if (info.Owners == null)
356+
{
357+
error = $"The {nameof(ProcessingSourceInfo.Owners)} property of {processSource.Name}'s {nameof(ProcessingSourceInfo)} is null.";
358+
return false;
359+
}
360+
361+
owners = info.Owners
362+
.Select(o => new Core.Metadata.ContactInfo(o.Name, o.Address, o.EmailAddresses ?? [], o.PhoneNumbers ?? []))
318363
.ToArray();
319364

320-
if (!Uri.TryCreate(aboutInfo.ProjectInfo.Uri, UriKind.Absolute, out Uri? projectUri))
365+
if (owners.Any(o => o.EmailAddresses.Any(string.IsNullOrWhiteSpace)))
321366
{
322-
this.logger.LogError($"Unable to generate processing source info: {aboutInfo.ProjectInfo.Uri} is not an absolute URI.");
367+
error = $"One or more of the {nameof(ProcessingSourceInfo.Owners)}'s {nameof(Core.Metadata.ContactInfo.EmailAddresses)} on {processSource.Name}'s {nameof(ProcessingSourceInfo)} is null or whitespace.";
323368
return false;
324369
}
325370

326-
Core.Metadata.ProjectInfo projectInfo = new(projectUri);
371+
if (owners.Any(o => o.PhoneNumbers.Any(string.IsNullOrWhiteSpace)))
372+
{
373+
error = $"One or more of the {nameof(ProcessingSourceInfo.Owners)}'s {nameof(Core.Metadata.ContactInfo.PhoneNumbers)} on {processSource.Name}'s {nameof(ProcessingSourceInfo)} is null or whitespace.";
374+
return false;
375+
}
376+
377+
return true;
378+
}
327379

328-
if (!Uri.TryCreate(aboutInfo.LicenseInfo.Uri, UriKind.Absolute, out Uri? licenseUri))
380+
private bool TryGetProjectInfo(
381+
ProcessingSourceReference processSource,
382+
Microsoft.Performance.SDK.Processing.ProcessingSourceInfo processingSourceInfo,
383+
out ProjectInfo? projectInfo,
384+
[NotNullWhen(false)] out string? error)
385+
{
386+
error = null;
387+
projectInfo = null;
388+
389+
if (processingSourceInfo.ProjectInfo == null)
329390
{
330-
this.logger.LogError($"Unable to generate processing source info: {aboutInfo.LicenseInfo.Uri} is not an absolute URI.");
391+
// This is an optional field
392+
projectInfo = null;
393+
return true;
394+
}
395+
396+
if (processingSourceInfo.ProjectInfo.Uri == null)
397+
{
398+
error = $"The {nameof(ProcessingSourceInfo.ProjectInfo)}.{nameof(ProcessingSourceInfo.ProjectInfo.Uri)} property of {processSource.Name}'s {nameof(ProcessingSourceInfo)} is null.";
331399
return false;
332400
}
333401

334-
Core.Metadata.LicenseInfo licenseInfo = new(aboutInfo.LicenseInfo.Name, licenseUri, aboutInfo.LicenseInfo.Text);
402+
if (!Uri.TryCreate(processingSourceInfo.ProjectInfo.Uri, UriKind.Absolute, out Uri? uri))
403+
{
404+
error = $"{processingSourceInfo.ProjectInfo.Uri} is not an absolute URI.";
405+
return false;
406+
}
335407

336-
processingSourceInfo = new ProcessingSourceInfo(owners, projectInfo, licenseInfo, aboutInfo.CopyrightNotice, aboutInfo.AdditionalInformation);
408+
projectInfo = new ProjectInfo(uri);
409+
return true;
410+
}
411+
412+
private bool TryGetLicenseInfo(
413+
ProcessingSourceReference processSource,
414+
Microsoft.Performance.SDK.Processing.ProcessingSourceInfo processingSourceInfo,
415+
out LicenseInfo? licenseInfo,
416+
[NotNullWhen(false)] out string? error)
417+
{
418+
error = null;
419+
licenseInfo = null;
420+
421+
if (processingSourceInfo.LicenseInfo == null)
422+
{
423+
// This is an optional field
424+
licenseInfo = null;
425+
return true;
426+
}
427+
428+
if (processingSourceInfo.LicenseInfo.Uri == null)
429+
{
430+
error = $"The {nameof(ProcessingSourceInfo.LicenseInfo)}.{nameof(ProcessingSourceInfo.LicenseInfo.Uri)} property of {processSource.Name}'s {nameof(ProcessingSourceInfo)} is null.";
431+
return false;
432+
}
433+
434+
if (string.IsNullOrWhiteSpace(processingSourceInfo.LicenseInfo.Name))
435+
{
436+
error = $"The {nameof(ProcessingSourceInfo.LicenseInfo)}.{nameof(ProcessingSourceInfo.LicenseInfo.Name)} property of {processSource.Name}'s {nameof(ProcessingSourceInfo)} is null or whitespace.";
437+
return false;
438+
}
439+
440+
if (!Uri.TryCreate(processingSourceInfo.LicenseInfo.Uri, UriKind.Absolute, out Uri? uri))
441+
{
442+
error = $"{processingSourceInfo.LicenseInfo.Uri} is not an absolute URI.";
443+
return false;
444+
}
445+
446+
licenseInfo = new LicenseInfo(processingSourceInfo.LicenseInfo.Name, uri, processingSourceInfo.LicenseInfo.Text);
337447
return true;
338448
}
339449

0 commit comments

Comments
 (0)