Update the cache web api call #1456
Conversation
costin-zaharia-sonarsource
force-pushed
the
costin/update-cache-web-api
branch
from
ca9d9c5 to
190937b
Compare
costin-zaharia-sonarsource
force-pushed
the
costin/update-cache-web-api
branch
2 times, most recently
from
f2b6b34 to
1aaabce
Compare
costin-zaharia-sonarsource
force-pushed
the
costin/update-cache-web-api
branch
3 times, most recently
from
f163970 to
ad740c7
Compare
| @@ -1169,7 +1182,7 @@ public async Task GetProperties_RequestUrl(string hostUrl, string versionUrl, st | |||
| private static Stream CreateCacheStream(IMessage message) | |||
| { | |||
| var stream = new MemoryStream(); | |||
| message.WriteTo(stream); | |||
| message.WriteDelimitedTo(stream); | |||
There was a problem hiding this comment.
This is an important change that's well hidden.
The stream we receive from the server contains a sequence of objects (not a single one), and this sequence does not have a parent. In order to correctly emulate the serialization/deserialization and make it work properly, we have to use WriteDelimitedTo instead of WriteTo.
| // - add it to your SonarQube | ||
| // - start SonarQube and analyze `IncrementalPRAnalysis` project with `/d:sonar.scanner.keepReport=true` | ||
| // - archive the content of `.sonarqube\out\.sonar\scanner-report\` (only the content, without parent folder) as `scanner-report.zip` | ||
| File reportZip = projectDir.resolve("scanner-report.zip").toFile(); |
There was a problem hiding this comment.
The previous strategy was to upload a full analysis result that included a cache. When this was implemented on the scanner, the analyzers did not have the cache feature implemented so we missed a component that could populate the cache.
Now that we have the analyzers updated we can just run 2 analyses, the first one is full and it will populate the cache, and the second one is for a PR, and is downloading the cache.
| @@ -67,7 +68,7 @@ public async Task Execute() | |||
| else | |||
| { | |||
| logger.LogInfo(Resources.MSG_Processing_PullRequest_Branch, baseBranch); | |||
| if (await server.DownloadCache(localSettings.ProjectKey, baseBranch) is { } cache) | |||
| if (await server.DownloadCache(localSettings.ProjectKey, baseBranch) is { Count: > 0 } cache) | |||
There was a problem hiding this comment.
Now we return an empty collection (empty cache) instead of null.
| message AnalysisCacheMsg { | ||
| map<string, bytes> map = 1; | ||
|
|
||
| message SensorCacheEntry { |
There was a problem hiding this comment.
This is the main contract change. Instead of one AnalysisCacheMsg instance, we receive a stream of SensorCacheEntry entries.
src/SonarScanner.MSBuild.PreProcessor/SonarScanner.MSBuild.PreProcessor.csproj
Show resolved
Hide resolved
| { | ||
| while (dataStream.Position < dataStream.Length) | ||
| { | ||
| cacheEntries.Add(SensorCacheEntry.Parser.ParseDelimitedFrom(dataStream)); |
There was a problem hiding this comment.
This part is a bit tricky. Just by looking at the stream, we don't know how many objects there are and neither their type. We have to rely on the information we get from SonarQube team. If at any point they change the format without letting us know, the parser will try to match whatever it finds in the stream to the current data structure, leading to wrong data without raising an exception.
costin-zaharia-sonarsource
force-pushed
the
costin/update-cache-web-api
branch
6 times, most recently
from
47e7e3a to
636be4f
Compare
costin-zaharia-sonarsource
force-pushed
the
costin/update-cache-web-api
branch
from
51e9230 to
86157b0
Compare
Tests/SonarScanner.MSBuild.PreProcessor.Test/Infrastructure/MockSonarWebService.cs
Outdated
Show resolved
Hide resolved
|
Kudos, SonarCloud Quality Gate passed! |
Fixes #1454