Skip to content
This repository has been archived by the owner on Aug 3, 2024. It is now read-only.
/ ServerCommon Public archive

Add status classes and tests #170

Merged
merged 14 commits into from
Jul 13, 2018
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions NuGet.Server.Common.sln
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NuGet.Services.Sql", "src\N
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NuGet.Services.Sql.Tests", "tests\NuGet.Services.Sql.Tests\NuGet.Services.Sql.Tests.csproj", "{FA2B3447-7242-495F-A8E1-D94181C0C9A5}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NuGet.Services.Status", "src\NuGet.Services.Status\NuGet.Services.Status.csproj", "{D3AB8DBD-EF83-41A5-AF25-0A7E7FACC056}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NuGet.Services.Status.Tests", "tests\NuGet.Services.Status.Tests\NuGet.Services.Status.Tests.csproj", "{EA54BC71-F9AD-4863-8DC6-8CC41776C881}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -168,6 +172,14 @@ Global
{FA2B3447-7242-495F-A8E1-D94181C0C9A5}.Debug|Any CPU.Build.0 = Debug|Any CPU
{FA2B3447-7242-495F-A8E1-D94181C0C9A5}.Release|Any CPU.ActiveCfg = Release|Any CPU
{FA2B3447-7242-495F-A8E1-D94181C0C9A5}.Release|Any CPU.Build.0 = Release|Any CPU
{D3AB8DBD-EF83-41A5-AF25-0A7E7FACC056}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{D3AB8DBD-EF83-41A5-AF25-0A7E7FACC056}.Debug|Any CPU.Build.0 = Debug|Any CPU
{D3AB8DBD-EF83-41A5-AF25-0A7E7FACC056}.Release|Any CPU.ActiveCfg = Release|Any CPU
{D3AB8DBD-EF83-41A5-AF25-0A7E7FACC056}.Release|Any CPU.Build.0 = Release|Any CPU
{EA54BC71-F9AD-4863-8DC6-8CC41776C881}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{EA54BC71-F9AD-4863-8DC6-8CC41776C881}.Debug|Any CPU.Build.0 = Debug|Any CPU
{EA54BC71-F9AD-4863-8DC6-8CC41776C881}.Release|Any CPU.ActiveCfg = Release|Any CPU
{EA54BC71-F9AD-4863-8DC6-8CC41776C881}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -198,6 +210,8 @@ Global
{E29F54DF-DFB8-4E27-940D-21ECCB9B6FC1} = {7783A106-0F4C-4055-9AB4-413FB2C7B8F0}
{F5121B0A-669F-48BD-86DC-27C546D1A825} = {8415FED7-1BED-4227-8B4F-BB7C24E041CD}
{FA2B3447-7242-495F-A8E1-D94181C0C9A5} = {7783A106-0F4C-4055-9AB4-413FB2C7B8F0}
{D3AB8DBD-EF83-41A5-AF25-0A7E7FACC056} = {8415FED7-1BED-4227-8B4F-BB7C24E041CD}
{EA54BC71-F9AD-4863-8DC6-8CC41776C881} = {7783A106-0F4C-4055-9AB4-413FB2C7B8F0}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {AA413DB0-5475-4B5D-A3AF-6323DA8D538B}
Expand Down
6 changes: 4 additions & 2 deletions build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ Invoke-BuildStep 'Set version metadata in AssemblyInfo.cs' { `
"$PSScriptRoot\src\NuGet.Services.ServiceBus\Properties\AssemblyInfo.g.cs", `
"$PSScriptRoot\src\NuGet.Services.Validation\Properties\AssemblyInfo.g.cs", `
"$PSScriptRoot\src\NuGet.Services.Validation.Issues\Properties\AssemblyInfo.g.cs", `
"$PSScriptRoot\src\NuGet.Services.Sql\Properties\AssemblyInfo.g.cs"
"$PSScriptRoot\src\NuGet.Services.Sql\Properties\AssemblyInfo.g.cs", `
"$PSScriptRoot\src\NuGet.Services.Status\Properties\AssemblyInfo.g.cs"

$versionMetadata | ForEach-Object {
Set-VersionInfo -Path $_ -Version $SimpleVersion -Branch $Branch -Commit $CommitSHA
Expand Down Expand Up @@ -105,7 +106,8 @@ Invoke-BuildStep 'Creating artifacts' { `
"src\NuGet.Services.ServiceBus\NuGet.Services.ServiceBus.csproj", `
"src\NuGet.Services.Validation\NuGet.Services.Validation.csproj", `
"src\NuGet.Services.Validation.Issues\NuGet.Services.Validation.Issues.csproj", `
"src\NuGet.Services.Sql\NuGet.Services.Sql.csproj"
"src\NuGet.Services.Sql\NuGet.Services.Sql.csproj", `
"src\NuGet.Services.Status\NuGet.Services.Status.csproj"
Copy link
Contributor

Choose a reason for hiding this comment

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

csproj [](start = 61, length = 6)

Shouldn't you update test.ps1 too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!


$projects | ForEach-Object {
New-Package (Join-Path $PSScriptRoot $_) -Configuration $Configuration -Symbols -IncludeReferencedProjects -MSBuildVersion "15"
Expand Down
36 changes: 36 additions & 0 deletions src/NuGet.Services.Status/Component.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;

namespace NuGet.Services.Status
{
/// <summary>
/// Default implementation of <see cref="IComponent"/>.
/// </summary>
public abstract class Component : ReadOnlyComponent, IComponent
Copy link
Member

Choose a reason for hiding this comment

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

Do you have examples somewhere of what the the hierarchy of these components is? What LeafComponents, PrimarySecondaryComponents, TreeComponents, etc.

Could there be more specific name? NuGetServiceComponent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify:

A consumer of status (gallery, status page) is going to only work with IReadOnlyComponent and its implementation, ReadOnlyComponent. Consumers can't set or modify status, so the status of any component doesn't change and status doesn't need to be recalculated at any point.

IComponent and its implementations (which are subclasses of Component) are to be used by status producers (status job) and allow setting status. Because they allow setting status, the status of any component can change at any moment. A change in the status of a subcomponent can affect the status of a component that is its root. The subclasses of Component propagate changes in status differently from subcomponents to root components.

I'll provide some examples for each Component subclass I have here:

TreeComponent:
V3 restore is a component with two subcomponents, one for each of its regions--Global and China.
If either Global or China is down, V3 restore is degraded because both Global and China are used by customers to restore using V3.

PrimarySecondaryComponent:
The gallery is a component with two subcomponents, one for each of its regions--USNC and USSC.
If USNC is up, we don't care about the status of USSC because it's only used if USNC is down.d

LeafComponent:
The USNC region of Gallery is a LeafComponent because it can't be broken down into any subcomponents.
(Although TreeComponent and PrimarySecondaryComponent both support not having subcomponents and behave as LeafComponent does, I created this subclass so that status producers don't have to choose between the two arbitrarily.)

Copy link
Member

Choose a reason for hiding this comment

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

I suggest adding comments w/ your examples to the code.

{
public new abstract ComponentStatus Status { get; set; }
public new IEnumerable<IComponent> SubComponents { get; }
Copy link
Member

Choose a reason for hiding this comment

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

Is there another way to achieve this without the new to replace base properties? It makes the code more complex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately no, but I think from a consumer side this makes these classes a lot easier to use because I'm replacing the base properties in an intuitive way. I tried doing this without replacing any properties and it made the code substantially uglier.

I replace Status to add a setter to a property that is read-only on the base interface.

I replace SubComponents to provide the additional restriction that all members of the property on the base interface are of a specific subclass (IComponent instead of IReadOnlyComponent).


public Component(
string name,
string description)
: base(name, description)
{
SubComponents = Enumerable.Empty<IComponent>();
}

public Component(
string name,
string description,
IEnumerable<IComponent> subComponents)
: base(name, description, subComponents)
{
SubComponents = subComponents?.Select(s => new ComponentWrapper(s, this)).ToList()
?? throw new ArgumentNullException(nameof(subComponents));
}
}
}
26 changes: 26 additions & 0 deletions src/NuGet.Services.Status/ComponentStatus.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace NuGet.Services.Status
{
/// <summary>
/// Describes whether or not a <see cref="IReadOnlyComponent"/> is performing as expected.
/// </summary>
public enum ComponentStatus
{
/// <summary>
/// The <see cref="IReadOnlyComponent"/> is performing as expected.
/// </summary>
Up,

/// <summary>
/// Some portion of the <see cref="IReadOnlyComponent"/> is not performing as expected.
/// </summary>
Degraded,

/// <summary>
/// The <see cref="IReadOnlyComponent"/> is completely unfunctional.
/// </summary>
Down
}
}
86 changes: 86 additions & 0 deletions src/NuGet.Services.Status/ComponentUtility.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;

namespace NuGet.Services.Status
{
public static class ComponentUtility
{
/// <summary>
/// Concatenates a set of <see cref="IReadOnlyComponent.Name"/>s into a <see cref="IReadOnlyComponent.Path"/>.
/// </summary>
public static string GetPath(params string[] componentNames)
{
componentNames = componentNames ?? throw new ArgumentNullException(nameof(componentNames));

return string.Join(Constants.ComponentPathDivider.ToString(), componentNames);
}

/// <summary>
/// Gets the subcomponent of <paramref name="component"/> with <see cref="IReadOnlyComponent.Path"/> <paramref name="path"/>.
/// If none exists, returns <c>null</c>.
/// </summary>
public static TComponent GetByPath<TComponent>(this TComponent component, string path)
where TComponent : class, IReadOnlyComponent, IComponentRoot<TComponent>
{
if (path == null)
{
return null;
}

var componentNames = path.Split(Constants.ComponentPathDivider);
return component.GetByNames(componentNames);
}

/// <summary>
/// Gets the subcomponent of <paramref name="component"/> by iterating its subcomponents in the order of <paramref name="componentNames"/>.
/// If none exists, returns <c>null</c>.
/// </summary>
public static TComponent GetByNames<TComponent>(this TComponent component, params string[] componentNames)
Copy link
Contributor

Choose a reason for hiding this comment

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

component [](start = 72, length = 9)

Extension methods let the this parameter be null, thus, you should check if component is null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch

where TComponent : class, IReadOnlyComponent, IComponentRoot<TComponent>
{
if (component == null)
{
return null;
}

if (componentNames == null)
{
return null;
}

TComponent currentComponent = null;
IComponentRoot<TComponent> currentRoot = new GetByNamesHelper<TComponent>(component);
foreach (var componentName in componentNames)
{
currentComponent = currentRoot.SubComponents.FirstOrDefault(c => c.Name == componentName);

if (currentComponent == null)
{
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

break [](start = 20, length = 5)

If I run GetByPath('root', 'this', 'doesnt', 'exist') and only the root component exists, this will return the root IReadOnlyComponent. Would it be better if this returned null instead? If so, I would rename the method to be GetByPathOrNull.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a bug. I added tests to make sure it doesn't break again.

}

currentRoot = currentComponent;
}

return currentComponent;
}

/// <remarks>
/// Dummy implementation of <see cref="IComponentRoot{TComponent}"/> used by <see cref="GetByNames{TComponent}(TComponent, string[])"/>.
/// </remarks>
private class GetByNamesHelper<TComponent> : IComponentRoot<TComponent>
where TComponent : IReadOnlyComponent
{
public IEnumerable<TComponent> SubComponents { get; }

public GetByNamesHelper(TComponent root)
{
SubComponents = new[] { root };
}
}
}
}
29 changes: 29 additions & 0 deletions src/NuGet.Services.Status/ComponentWrapper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Linq;

namespace NuGet.Services.Status
{
/// <summary>
/// Wrapper class for <see cref="IComponent"/> that sets <see cref="IReadOnlyComponent.Path"/> as expected.
/// </summary>
internal class ComponentWrapper : ReadOnlyComponentWrapper, IComponent
{
private readonly IComponent _component;
private readonly IComponent _parent;

public new ComponentStatus Status { get { return _component.Status; } set { _component.Status = value; } }
public new IEnumerable<IComponent> SubComponents { get; }

public ComponentWrapper(IComponent component, IComponent parent)
: base(component, parent)
{
_component = component;
_parent = parent;
SubComponents = _component.SubComponents?.Select(s => new ComponentWrapper(s, this)).ToList()
?? Enumerable.Empty<IComponent>();
}
}
}
17 changes: 17 additions & 0 deletions src/NuGet.Services.Status/Constants.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace NuGet.Services.Status
{
public static class Constants
{
/// <summary>
/// Used to construct <see cref="IReadOnlyComponent.Path"/>.
/// </summary>
/// <example>
/// Suppose C is a subcomponent of B which is a subcomponent of A.
/// The path of C is <code>"A" + ComponentPathDivider + "B" + ComponentPathDivider + "C"</code>.
/// </example>
public static char ComponentPathDivider = '/';
}
}
48 changes: 48 additions & 0 deletions src/NuGet.Services.Status/Event.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using Newtonsoft.Json;

namespace NuGet.Services.Status
{
public class Event : IEvent
{
public string AffectedComponentPath { get; }
public ComponentStatus AffectedComponentStatus { get; }
public DateTime StartTime { get; }
public DateTime? EndTime { get; }
public IEnumerable<IMessage> Messages { get; }

public Event(
string affectedComponentPath,
ComponentStatus affectedComponentStatus,
DateTime startTime,
DateTime? endTime,
IEnumerable<IMessage> messages)
{
AffectedComponentPath = affectedComponentPath;
AffectedComponentStatus = affectedComponentStatus;
StartTime = startTime;
EndTime = endTime;
Messages = messages;
}

[JsonConstructor]
public Event(
string affectedComponentPath,
ComponentStatus affectedComponentStatus,
DateTime startTime,
DateTime? endTime,
IEnumerable<Message> messages)
: this(
affectedComponentPath,
affectedComponentStatus,
startTime,
endTime,
(IEnumerable<IMessage>)messages)
{
}
}
}
23 changes: 23 additions & 0 deletions src/NuGet.Services.Status/IComponent.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;

namespace NuGet.Services.Status
{
/// <summary>
/// A writable <see cref="IReadOnlyComponent"/> that allows setting its status.
/// </summary>
public interface IComponent : IReadOnlyComponent, IComponentRoot<IComponent>
{
/// <summary>
/// The status of this part of the service.
/// </summary>
new ComponentStatus Status { get; set; }

/// <summary>
/// A list of writable subcomponents that make up this part of the service.
/// </summary>
new IEnumerable<IComponent> SubComponents { get; }
Copy link
Member

Choose a reason for hiding this comment

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

Is new necessary? Why can't it reuse IReadOnlyComponent.SubComponentsorIComponentRoot.SubComponents`?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove this, IComponentRoot<IComponent>.SubComponents is sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot that this needs to exist so that IComponent.SubComponents is not ambiguous between IEnumerable<IReadOnlyComponent> and IEnumerable<IComponent>.

}
}
13 changes: 13 additions & 0 deletions src/NuGet.Services.Status/IComponentRoot.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;

namespace NuGet.Services.Status
{
public interface IComponentRoot<out TComponent>
where TComponent : IReadOnlyComponent
{
IEnumerable<TComponent> SubComponents { get; }
}
}
39 changes: 39 additions & 0 deletions src/NuGet.Services.Status/IEvent.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;

namespace NuGet.Services.Status
{
/// <summary>
/// Represents an event affecting a <see cref="IReadOnlyComponent"/>.
/// </summary>
public interface IEvent
{
/// <summary>
/// The <see cref="IReadOnlyComponent.Path"/> of the <see cref="IReadOnlyComponent"/> affected.
/// </summary>
string AffectedComponentPath { get; }

/// <summary>
/// The <see cref="IReadOnlyComponent.Status"/> of the <see cref="IReadOnlyComponent"/> affected.
/// </summary>
ComponentStatus AffectedComponentStatus { get; }

/// <summary>
/// When the event began.
/// </summary>
DateTime StartTime { get; }

/// <summary>
/// When the event ended, or <c>null</c> if the event is still active.
/// </summary>
DateTime? EndTime { get; }

/// <summary>
/// A set of <see cref="IMessage"/>s related to this event.
/// </summary>
IEnumerable<IMessage> Messages { get; }
}
}
Loading