feat(phase-1): frame codec — TcrMessage / TcrPart / BigEndian{Reader,Writer} - #7
Merged
Merged
Conversation
Five small changes to get the Phase 0 skeleton building cleanly on a developer machine that has both nuget.org and a corporate feed configured, and to stop opinionated analyzer rules from blocking walking-skeleton work: - NuGet.config: <clear/> + nuget.org only, fixes NU1507 caused by Central Package Management with multiple inherited sources. - Directory.Build.props: AnalysisLevel latest-recommended -> latest-default during MVP. TODO marker to tighten back before Phase 5 / first NuGet release. - samples/Geode.Client.Sample/Program.cs: add missing 'using Microsoft.Extensions.DependencyInjection;' so GetRequiredService resolves (it is an extension method on IServiceProvider in that ns). - tests/.../GeodeFixture.cs: SuppressMessage CA1711 on GeodeCollection. xUnit's [CollectionDefinition(nameof(...))] convention uses the class name as the collection identifier; renaming would break the call sites. - .gitignore: ignore .cr/ (Visual Studio extension cache). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Visual Studio 2022 added these on first open: - VisualStudioVersion / MinimumVisualStudioVersion stamps. - 'src', 'test', 'sample' solution folders nesting the four projects for a tidier Solution Explorer view. - Solution items entries re-sorted alphabetically. Pure IDE metadata — dotnet CLI ignores solution folders, so no effect on build, restore, or CI. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Foundation layer for the Phase 1 frame codec. - MessageType: full enum mirroring cppcache TcrMessage.hpp::MsgType (99 values incl. -2/-1 sentinels and the 4 numeric gaps preserved). Naming: SCREAMING_SNAKE_CASE -> PascalCase, _MSG_TYPE / _TYPE redundant suffixes dropped (e.g. EXECUTECQ_MSG_TYPE -> ExecuteCq). - BigEndianBinaryWriter: sequential big-endian writer over an internal MemoryStream. C# counterpart of cppcache DataOutput. Phase 1 implements WriteByte / WriteBool / WriteInt32 / WriteInt64 / WriteBytesOnly / ToArray / Length; the rest (WriteSByte, WriteInt16, WriteUInt16/32/64, WriteFloat, WriteDouble, WriteBytes, WriteArrayLen, WriteJavaModifiedUtf8, WriteUtf16Huge) are prototype stubs that throw NotImplementedException so the API surface is stable across phases. - BigEndianBinaryReader: sequential big-endian reader over a ReadOnlyMemory<byte>. Symmetric stub set. ReadBytesOnly returns a zero-copy slice. EndOfStreamException on overrun. - TcrPart: record (i32 length + u8 isObject + raw payload) modelling the inline 3-step encoding used by every cppcache TcrMessage::write*Part helper. Equals / GetHashCode overridden so equality is byte-content based (record default would be reference-based on ReadOnlyMemory). Buffer-based design (reader takes ReadOnlyMemory, writer owns internal buffer) committed as the long-term shape — matches modern .NET codec patterns (System.Text.Json, MessagePack-CSharp, Pipelines) where async lives at the I/O boundary and the codec itself is sync over Memory/Span. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Code: - TcrMessage record (header + Parts) with two-pass Encode and strict Decode validation. Mirrors cppcache TcrMessage::writeHeader / handleByteArrayResponse / writeMessageLength. - Custom Equals/GetHashCode so Parts list compares element-wise (record default would be reference equality on the list). - Drop _Phase1Placeholder.cs now that the real Protocol/ files exist. Tests (xUnit native Assert, no FluentAssertions): - Protocol/BigEndianBinaryWriterTests — 8 facts: primitives, concat, length tracking. - Protocol/BigEndianBinaryReaderTests — 9 facts: primitives, zero-copy slice (proven by mutating source), bounds, position tracking. - Protocol/TcrPartTests — 7 facts: round-trip (simple / empty / isObject), validation, content-based equality. - Protocol/TcrMessageTests — 10 facts: round-trip, Ping and Put-with-byte-part byte fixtures derived from cppcache wire format, malformed-frame validation, element-wise Parts equality. FluentAssertions removed: - v8.x switched to a custom (non-OSI) Xceed license; rather than audit the new terms for our Apache-2.0 use case, drop the dependency entirely. xUnit native Assert.* covers everything we used. - Existing Phase 0 tests (SmokeTests, GeodeContainerSmokeTests) also converted from .Should() to Assert.*, so the codebase has zero FA references. - Removed from Directory.Packages.props and from both test csprojs. Routine dependency bumps (accepted while VS auto-updated them): - Microsoft.NET.Test.Sdk 17.12.0 -> 18.5.1 - xunit.v3 1.0.0 -> 3.2.2 - xunit.runner.visualstudio 3.0.0 -> 3.1.5 - coverlet.collector 6.0.2 -> 10.0.0 Geode.Client.Tests.csproj also picked up <PrivateAssets>all</PrivateAssets> + <IncludeAssets> on xunit.runner.visualstudio and coverlet.collector — the standard NuGet pattern for dev-only packages, kept as-is. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 frame codec — pure encode/decode of
TcrMessage↔ bytes. All MVP wire-format primitives in place. No sockets yet.Phase / Roadmap
Changes
Phase 1 codec (
src/Geode.Client/Protocol/)MessageType— enum mirroringcppcache/src/TcrMessage.hpp::enum MsgTypein full (99 wire values + 2 sentinels). PascalCase, dropped redundant_MSG_TYPE/_TYPEsuffixes. Numeric gaps (57, 95, 101, 102, 104) preserved.BigEndianBinaryWriter— sequential big-endian writer over an internalMemoryStream. C# counterpart of cppcacheDataOutput. Implemented:WriteByte/WriteBool/WriteInt32/WriteInt64/WriteBytesOnly. Stubs (NotImplementedExceptionwith phase markers) forWriteSByte/WriteInt16/WriteUInt16/32/64/WriteFloat/WriteDouble/WriteBytes/WriteArrayLen/WriteJavaModifiedUtf8/WriteUtf16Huge.BigEndianBinaryReader— symmetric reader overReadOnlyMemory<byte>, with zero-copyReadBytesOnly. ThrowsEndOfStreamExceptionon overrun.TcrPartrecord —i32 length + u8 IsObject + ReadOnlyMemory<byte> Payload. CustomEquals/GetHashCodefor byte-content equality (record default would be reference-based onReadOnlyMemory).TcrMessagerecord — 17-byte header (msgType / msgLength / numParts / txId / earlyAck) + Parts. Two-passEncode(parts first to learn length, then header + parts).Decodestrict-validatesMessageLengthmatches actual parts byte count andNumParts >= 0._Phase1Placeholder.cs.Repo plumbing carried on this branch
NuGet.config—<clear/>+ nuget.org only, fixes NU1507 on machines with extra sources configured at user / machine level (CPM requires either single source or explicit mapping).Directory.Build.props—AnalysisLevelrelaxed fromlatest-recommendedtolatest-defaultduring MVP. TODO marker to tighten back before Phase 5 / first NuGet release. FixedCHANGE_ME→ real GitHub URL.samples/.../Program.cs— added missingusing Microsoft.Extensions.DependencyInjection;(CS1061 fix).tests/.../GeodeFixture.cs—[SuppressMessage("Naming", "CA1711")]onGeodeCollection(xUnit[CollectionDefinition(nameof(...))]convention requires the class name).geode-dotnet.sln— VS 2022 addedsrc/test/samplesolution folders and version stamps.Assert.*covers everything we used. Phase 0SmokeTests/GeodeContainerSmokeTestsalso converted.Microsoft.NET.Test.Sdk17.12 → 18.5.1,xunit.v31.0 → 3.2.2,xunit.runner.visualstudio3.0 → 3.1.5,coverlet.collector6.0.2 → 10.0.0..gitignore— ignore.cr/(VS 2022 extension cache).Tests
tests/Geode.Client.Tests/Protocol/(~34 facts):BigEndianBinaryWriterTests— primitives, concat, length tracking.BigEndianBinaryReaderTests— primitives, zero-copy slice (verified by mutating source array), bounds, position tracking.TcrPartTests— round-trip (simple / empty / isObject), validation, content-based equality.TcrMessageTests— round-trip + byte-fixture tests for Ping (17 bytes) and Put-with-byte-part (23 bytes), derived from cppcache wire format (citewriteHeader/writeMessageLength/writeBytePartline numbers in code comments). Validation: negativeNumParts,MessageLengthmismatch.Assert.*after FA removal.Notes for reviewer
ReadOnlyMemory<byte>, Writer owns internal buffer +ToArray(). Aligns with modern .NET codec patterns (System.Text.Json,MessagePack-CSharp,System.IO.Pipelines): codec is sync over Memory/Span; async lives at the I/O boundary (Phase 2'sTcpConnection).CONTRIBUTING.md§5.3); local VS build was the only validation. Re-enable target: before Phase 5 / first NuGet preview.Typebyte — verified againstcppcache/src/TcrMessage.cppline 812-816 that Part is 3 fields:length / isObject / payload(the C++Typeis the first byte of payload whenisObject=1). This implementation matches the actual cppcache code.