Skip to content

Commit 5129d9f

Browse files
trejjamtrejjam
authored
Fix quoted keys in YAML serialization and add comprehensive service test (#192)
* Add serialization test for complex service with healthcheck and deploy Test covers comprehensive Docker Compose service configuration including: - HealthCheck (test command, interval, timeout, retries, start_period) - Deploy (labels, replicas, update_config, restart_policy, placement, resources) - Ports, extra_hosts, command, volumes, labels, environment, networks The test currently fails due to quoted keys in serialization output. Keys 'ports', 'extra_hosts', 'command', 'deploy' are incorrectly quoted. * Fix quoted keys in YAML serialization output The issue was that custom type converters (ServiceVolumeCollectionConverter, YamlValueCollectionConverter) were emitting YAML events directly to IEmitter, bypassing the event emitter chain. This caused ForceQuotedStringValuesEventEmitter to lose track of key/value positions, resulting in quoted keys like "ports". Changes: - Refactor converters to use serializer callback instead of direct emitter.Emit() - Simplify ForceQuotedStringValuesEventEmitter to use boolean toggle for key/value tracking - Remove ServiceVolumeConverter (functionality merged into ServiceVolumeCollectionConverter)
1 parent 075c1b9 commit 5129d9f

File tree

5 files changed

+199
-78
lines changed

5 files changed

+199
-78
lines changed

‎src/DockerComposeBuilder.Tests/ComposeBuilderTests.cs‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using DockerComposeBuilder.Builders;
22
using DockerComposeBuilder.Extensions;
3+
using DockerComposeBuilder.Model;
34
using DockerComposeBuilder.Model.Services;
45
using DockerComposeBuilder.Model.Services.BuildArguments;
56
using System.Collections.Generic;
@@ -495,4 +496,149 @@ public void SerializeWithVolumesLongSyntaxTest()
495496
result
496497
);
497498
}
499+
500+
[Fact]
501+
public void SerializeJaegerServiceTest()
502+
{
503+
var compose = Builder.MakeCompose()
504+
.WithServices(
505+
Builder.MakeService("jaeger")
506+
.WithImage("jaegertracing/jaeger:2.14.1")
507+
.WithHealthCheck(healthCheck =>
508+
{
509+
healthCheck.TestCommand =
510+
[
511+
"CMD-SHELL",
512+
"wget --no-verbose --tries=1 -q -O - http://localhost:14269/status | grep -q '\"healthy\":true' > /dev/null || exit 1"
513+
];
514+
healthCheck.Interval = "1m";
515+
healthCheck.Timeout = "10s";
516+
healthCheck.Retries = 3;
517+
healthCheck.StartPeriod = "15s";
518+
})
519+
.WithHostname("jaeger")
520+
.WithNetworks("app_network")
521+
.WithLabels(labels =>
522+
{
523+
labels.Add("app.service", "jaeger");
524+
labels.Add("app.metrics.port", "14269");
525+
labels.Add("app.container.group", "services");
526+
labels.Add("app.stack.name", "production");
527+
})
528+
.WithEnvironment(env => { env.Add("GOMEMLIMIT", "1280MiB"); })
529+
.WithVolumes("/data/config/jaeger:/config:ro")
530+
.WithPortMappings(
531+
new Port
532+
{
533+
Target = 16686,
534+
Published = 16686,
535+
Protocol = "tcp",
536+
},
537+
new Port
538+
{
539+
Target = 14269,
540+
Protocol = "tcp",
541+
}
542+
)
543+
.WithExtraHosts(
544+
"prometheus:${PROMETHEUS_HOST}",
545+
"elasticsearch:192.168.1.100"
546+
)
547+
.WithCommands(
548+
"--config=/config/jaeger.yaml",
549+
"--set=extensions.jaeger_query.base_path=/tracing"
550+
)
551+
.WithSwarm()
552+
.WithDeploy(deploy => deploy
553+
.WithLabels(labels =>
554+
{
555+
labels.Add("app.service", "jaeger");
556+
labels.Add("app.metrics.port", "14269");
557+
labels.Add("app.container.group", "services");
558+
labels.Add("app.stack.name", "production");
559+
})
560+
.WithReplicas(2)
561+
.WithUpdateConfig(c => c.WithProperty("delay", "30s"))
562+
.WithRestartPolicy(c => c
563+
.WithProperty("delay", "30s")
564+
.WithProperty("max_attempts", 10)
565+
.WithProperty("window", "120s")
566+
)
567+
.WithPlacement(c => c
568+
.WithProperty("constraints", new[] { "node.role == worker" })
569+
.WithProperty("preferences", new[] { new PlacementPreferences { Spread = "node.labels.zone" } })
570+
)
571+
.WithResources(c => c.WithProperty("limits", new Dictionary<string, string>
572+
{
573+
["memory"] = "1600m",
574+
}))
575+
)
576+
.Build()
577+
)
578+
.Build();
579+
580+
var result = compose.Serialize();
581+
582+
Assert.Equal(
583+
// language=yaml
584+
"""
585+
version: "3.8"
586+
services:
587+
jaeger:
588+
image: "jaegertracing/jaeger:2.14.1"
589+
healthcheck:
590+
test: ["CMD-SHELL", "wget --no-verbose --tries=1 -q -O - http://localhost:14269/status | grep -q '\"healthy\":true' > /dev/null || exit 1"]
591+
interval: "1m"
592+
timeout: "10s"
593+
retries: 3
594+
start_period: "15s"
595+
hostname: "jaeger"
596+
networks:
597+
- "app_network"
598+
labels:
599+
app.service: "jaeger"
600+
app.metrics.port: "14269"
601+
app.container.group: "services"
602+
app.stack.name: "production"
603+
environment:
604+
GOMEMLIMIT: "1280MiB"
605+
volumes:
606+
- "/data/config/jaeger:/config:ro"
607+
ports:
608+
- target: 16686
609+
published: 16686
610+
protocol: "tcp"
611+
- target: 14269
612+
protocol: "tcp"
613+
extra_hosts:
614+
- "prometheus:${PROMETHEUS_HOST}"
615+
- "elasticsearch:192.168.1.100"
616+
command:
617+
- "--config=/config/jaeger.yaml"
618+
- "--set=extensions.jaeger_query.base_path=/tracing"
619+
deploy:
620+
labels:
621+
app.service: "jaeger"
622+
app.metrics.port: "14269"
623+
app.container.group: "services"
624+
app.stack.name: "production"
625+
replicas: 2
626+
update_config:
627+
delay: "30s"
628+
restart_policy:
629+
delay: "30s"
630+
max_attempts: 10
631+
window: "120s"
632+
placement:
633+
constraints: ["node.role == worker"]
634+
preferences:
635+
- spread: "node.labels.zone"
636+
resources:
637+
limits:
638+
memory: "1600m"
639+
640+
""",
641+
result
642+
);
643+
}
498644
}

‎src/DockerComposeBuilder/Converters/ServiceVolumeCollectionConverter.cs‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using DockerComposeBuilder.Model.Services;
22
using System;
3+
using System.Collections.Generic;
4+
using System.Linq;
35
using YamlDotNet.Core;
46
using YamlDotNet.Core.Events;
57
using YamlDotNet.Serialization;
@@ -8,8 +10,6 @@ namespace DockerComposeBuilder.Converters;
810

911
public class ServiceVolumeCollectionConverter : IYamlTypeConverter
1012
{
11-
private readonly ServiceVolumeConverter _itemConverter = new();
12-
1313
public bool Accepts(Type type) => typeof(ServiceVolumeCollection).IsAssignableFrom(type);
1414

1515
public object? ReadYaml(IParser parser, Type type, ObjectDeserializer rootDeserializer)
@@ -24,10 +24,23 @@ public class ServiceVolumeCollectionConverter : IYamlTypeConverter
2424

2525
while (parser.Current is not SequenceEnd)
2626
{
27-
var item = _itemConverter.ReadYaml(parser, typeof(ServiceVolume), rootDeserializer);
28-
if (item is ServiceVolume volume)
27+
if (parser.Current is Scalar scalar)
28+
{
29+
var value = scalar.Value;
30+
parser.MoveNext();
31+
collection.Add(ServiceVolume.FromShortSyntax(value));
32+
}
33+
else if (parser.Current is MappingStart)
34+
{
35+
var item = rootDeserializer(typeof(ServiceVolume));
36+
if (item is ServiceVolume volume)
37+
{
38+
collection.Add(volume);
39+
}
40+
}
41+
else
2942
{
30-
collection.Add(volume);
43+
parser.MoveNext();
3144
}
3245
}
3346

@@ -42,13 +55,10 @@ public void WriteYaml(IEmitter emitter, object? value, Type type, ObjectSerializ
4255
return;
4356
}
4457

45-
emitter.Emit(new SequenceStart(AnchorName.Empty, TagName.Empty, false, SequenceStyle.Block));
46-
47-
foreach (var item in collection)
48-
{
49-
_itemConverter.WriteYaml(emitter, item, typeof(ServiceVolume), serializer);
50-
}
58+
var items = collection.Select<ServiceVolume, object>(item =>
59+
item.ShortSyntax != null ? item.ShortSyntax : item
60+
).ToList();
5161

52-
emitter.Emit(new SequenceEnd());
62+
serializer(items, typeof(List<object>));
5363
}
5464
}

‎src/DockerComposeBuilder/Converters/ServiceVolumeConverter.cs‎

Lines changed: 0 additions & 45 deletions
This file was deleted.

‎src/DockerComposeBuilder/Converters/YamlValueCollectionConverter.cs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
using DockerComposeBuilder.Model.Infrastructure;
22
using System;
3+
using System.Collections.Generic;
34
using YamlDotNet.Core;
4-
using YamlDotNet.Core.Events;
55
using YamlDotNet.Serialization;
66

77
namespace DockerComposeBuilder.Converters;
@@ -16,21 +16,23 @@ public void WriteYaml(IEmitter emitter, object? value, Type type, ObjectSerializ
1616
{
1717
if (value is IValueCollection valueCollection)
1818
{
19-
emitter.Emit(new SequenceStart(AnchorName.Empty, TagName.Empty, false, SequenceStyle.Block));
20-
19+
var items = new List<string>();
2120
foreach (var item in valueCollection)
2221
{
23-
if (item is IKeyValue keyValue)
22+
var stringValue = item switch
2423
{
25-
emitter.Emit(new Scalar(AnchorName.Empty, TagName.Empty, $"{keyValue.Key}={keyValue.Value}", ScalarStyle.DoubleQuoted, true, false));
26-
}
27-
else if (item is IKey key)
24+
IKeyValue keyValue => $"{keyValue.Key}={keyValue.Value}",
25+
IKey key => key.Key,
26+
_ => null,
27+
};
28+
29+
if (stringValue != null)
2830
{
29-
emitter.Emit(new Scalar(AnchorName.Empty, TagName.Empty, key.Key, ScalarStyle.DoubleQuoted, true, false));
31+
items.Add(stringValue);
3032
}
3133
}
3234

33-
emitter.Emit(new SequenceEnd());
35+
serializer(items, typeof(List<string>));
3436
}
3537
}
3638
}

‎src/DockerComposeBuilder/Emitters/ForceQuotedStringValuesEventEmitter.cs‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ IEventEmitter nextEmitter
2020
public override void Emit(ScalarEventInfo eventInfo, IEmitter emitter)
2121
{
2222
var item = _state.Peek();
23-
item.Move();
23+
var shouldApply = item.ShouldApplyAndToggle();
2424

25-
if (item.ShouldApply() && eventInfo.Source.Type == typeof(string))
25+
if (shouldApply && eventInfo.Source.Type == typeof(string))
2626
{
2727
eventInfo = new ScalarEventInfo(eventInfo.Source)
2828
{
@@ -35,7 +35,7 @@ public override void Emit(ScalarEventInfo eventInfo, IEmitter emitter)
3535

3636
public override void Emit(MappingStartEventInfo eventInfo, IEmitter emitter)
3737
{
38-
_state.Peek().Move();
38+
_state.Peek().ConsumeValue();
3939
_state.Push(new EmitterState(EmitterState.EventType.Mapping));
4040
base.Emit(eventInfo, emitter);
4141
}
@@ -53,7 +53,7 @@ public override void Emit(MappingEndEventInfo eventInfo, IEmitter emitter)
5353

5454
public override void Emit(SequenceStartEventInfo eventInfo, IEmitter emitter)
5555
{
56-
_state.Peek().Move();
56+
_state.Peek().ConsumeValue();
5757
_state.Push(new EmitterState(EmitterState.EventType.Sequence));
5858
base.Emit(eventInfo, emitter);
5959
}
@@ -75,19 +75,27 @@ EmitterState.EventType eventType
7575
{
7676
public EventType Type { get; } = eventType;
7777

78-
private int _currentIndex;
78+
private bool _isAtKey = true;
7979

80-
public void Move()
80+
public bool ShouldApplyAndToggle()
8181
{
82-
_currentIndex++;
82+
if (Type == EventType.Mapping)
83+
{
84+
var wasAtKey = _isAtKey;
85+
_isAtKey = !_isAtKey;
86+
return !wasAtKey;
87+
}
88+
89+
return Type == EventType.Sequence;
8390
}
8491

85-
public bool ShouldApply() => Type switch
92+
public void ConsumeValue()
8693
{
87-
EventType.Mapping => _currentIndex % 2 == 0,
88-
EventType.Sequence => true,
89-
_ => false,
90-
};
94+
if (Type == EventType.Mapping)
95+
{
96+
_isAtKey = true;
97+
}
98+
}
9199

92100
public enum EventType : byte
93101
{

0 commit comments

Comments
 (0)