OLD EES-5593 Public API wildcard data set versioning #5559

mmoalin · 2025-01-27T22:52:19Z

Please go to the most recent relevant PR: : #5577
OLD:
This PR adds datasets wildcard versioning support to the following API endpoints:

[get]
data-sets/{dataSetId}/meta
data-sets/{dataSetId}/query
data-sets/{dataSetId}/csv
data-sets/{dataSetId}/versions/{dataSetVersion}
data-sets/{dataSetId}/versions/{dataSetVersion}/changes

[post]
data-sets/{dataSetId}/query

For example:

dataSetVersion=2.* should return the latest minor version in the v2 series
dataSetVersion=2.1 should return v2.1
dataSetVersion=2.0 should return v2.0
dataSetVersion=2 should return v2.0

Thank you in advance; also thank you @mark, @lee & @duncan for your initial feedbacks over our calls!

…or endpoints data-sets/{dataSetId}/meta & data-sets/{dataSetId}/query

…elector.

…e-analytical-services/explore-education-statistics into EES-5593-wildcard-ds-versions

tomjonesdev · 2025-01-28T10:05:50Z

I'll leave the proper review to someone who's familiar with the public API, but just a general comment: before this gets merged in, could you tidy up the commits with a reset please?

leeoades · 2025-01-28T10:59:06Z

src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/VersionUtilsTests.cs

+        [InlineData("v1.*.*")]
+        [InlineData("v1.*")]
+        [InlineData("v1.2.*")]
+        public void ValidWildCardVersion_SuccessfullyParsed(


I think we need to assert what the value is that was parsed. I realise that we're essentially testing the library here.

I've updated the changes so that they are more clear to understand. This tests the validation but not the object returned from 'TryParseWildcard' intentionally.
This is because 'TryParseWildcard' returns a 'SemVersionRange' (which contains a start and end range) - this doesn't contain a specific version that we can test against versionString, it contains the attributes 'start' and 'end' which are useful in other contexts but not in ours here.
Have added a comment & renamed the test cases to make this more clear.

OK. The reason we've wrapped SemVersionRange.TryParse is to pass in the parameter OptionalMinorPatch - This isn't being tested though. I can remove it and no tests fail.

src/GovUk.Education.ExploreEducationStatistics.Common/Utils/VersionUtils.cs

...on.ExploreEducationStatistics.Public.Data.Api.Tests/UnitTests/WildCardDatasetVersionTests.cs

leeoades · 2025-01-28T11:15:38Z

...on.ExploreEducationStatistics.Public.Data.Api.Tests/UnitTests/WildCardDatasetVersionTests.cs

+    [InlineData("v1.2.*", 1, 2, 0)]
+    [InlineData("v1.1.*", 1, 1, 1)]
+    [InlineData("v1.*", 1, 3, 0)]
+    public async Task TestDataSetVersions_ReturnsExpectedVersion(string versionString,


A couple of extra edge cases:

Higher version than is available - what should that return? e,.g. v6.* Not found I'm assuming

In-between version: remove 3.0.0 from your available and query for v3.0.0. Show that it returns not found

Lower: I would remove 0.0.* from your available dataset versions and then query for 0.0.* to see what happens. Should be not found.

Cheers Lee, I've added 3 extra edge cases. I've tested the last edge case locally and got the expected result but i havent removed the 0.0.1 because i want to use it in the other test.

I'm sure you can test it with 1.0.1 instead for example. By not including a test for a lower version, there's no knowing what it will do or if it changes. I doubt for example in reality that we have dataset versions 0.0.1 so if the lowest version is 1.0.0 or 0.6.0, what happens if we request 0.0.1?

No worries thats done

...on.ExploreEducationStatistics.Public.Data.Api.Tests/UnitTests/WildCardDatasetVersionTests.cs

...ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionQueryableExtensions.cs

leeoades · 2025-01-28T11:38:44Z

...ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionQueryableExtensions.cs

+        if (!VersionUtils.TryParseWildcard(wildcardedVersion, out var semVersionRange)) 
+            return new NotFoundResult();
+
+        var parts = wildcardedVersion.Trim('v').Split('.');


So you don't need to manually parse the string again here, you have the semVersionRange object which has done the work for you.

Unfortunately because the semVersionRange is not a SemVersion (this can provide the single parsed version), it returns a range which has a start and end attribute. I've found the semVersionRange object doesn't provide a useful property for me to use. I'd be happy to discuss some more over a call or to provide a demo.

In that case, have the TryParseWilcard return a different representation of a version that does do what you want. Create your own WildcardVersion if needs be if there isn't something appropriate in the Semver library and you can put the parse on that, and then it can expose the Major, Minor and Patch as uint? values for example.

That's a great idea had a go at that, thanks again Lee

leeoades · 2025-01-28T11:42:06Z

I'll leave the proper review to someone who's familiar with the public API, but just a general comment: before this gets merged in, could you tidy up the commits with a reset please?

Hey @tomjonesdev - is there an option to squash merge the PR? I'm assuming that's what you mean by the reset?

src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/VersionUtilsTests.cs

src/GovUk.Education.ExploreEducationStatistics.Common/Utils/VersionUtils.cs

src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/VersionUtilsTests.cs

...on.ExploreEducationStatistics.Public.Data.Api.Tests/UnitTests/WildCardDatasetVersionTests.cs

...ducation.ExploreEducationStatistics.Public.Data.Api/Controllers/DataSetVersionsController.cs

src/explore-education-statistics-api-docs/README.md

leeoades · 2025-01-29T14:00:20Z

...ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionQueryableExtensions.cs

+        if (!VersionUtils.TryParseWildcard(wildcardedVersion, out var semVersionRange)) 
+            return new NotFoundResult();
+
+        var parts = wildcardedVersion.Trim('v').Split('.');


In that case, have the TryParseWilcard return a different representation of a version that does do what you want. Create your own WildcardVersion if needs be if there isn't something appropriate in the Semver library and you can put the parse on that, and then it can expose the Major, Minor and Patch as uint? values for example.

leeoades · 2025-01-29T14:05:13Z

...on.ExploreEducationStatistics.Public.Data.Api.Tests/UnitTests/WildCardDatasetVersionTests.cs

+
+namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests.UnitTests;
+
+public class WildCardDatasetVersionTests


Ah, interesting arrangement. OK, I'd not noticed that. 👍

src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/VersionUtilsTests.cs

ntsim · 2025-01-29T16:39:36Z

src/explore-education-statistics-api-docs/source/overview/versioning/index.html.md.erb

+
+### Automatically receiving minor updates
+
+New data set versions can automatically be retreived from the API without having to specify the new data set version numbers explicitly. This can be used by
+supplying endpoints with the parameter `dataSetVersion` an astrick `*` wildcard character in the place of the minor or major version. 
+
+For example: 
+
+- `dataSetVersion=*` or `dataSetVersion=v*` will return the latest major and minor version for the given data set.
+- `dataSetVersion=2.*.*`, `dataSetVersion=2.*` or `dataSetVersion=v2.*` will return the latest minor version of the v2 series 
+- `dataSetVersion=2.3.*` or `dataSetVersion=v2.3.*` will return the latest patch version of the v2.3 series.


Nice first draft of this 😄

On reflection, we should probably go into a bit more detail about the dataSetVersion parameter generally.

Would suggest pasting in the following:

### Requesting data set versions There are several API endpoints that accept a `dataSetVersion` query parameter e.g. the [Get a data set version](/reference-v1/endpoints/GetDataSetVersion/index.html) endpoint. The `dataSetVersion` parameter can be supplied with a version number (e.g. 1.0, 1.1, 2.0, etc) that controls the data set version that should be used in the request. For example: - `dataSetVersion=1.1` uses version 1.1 - `dataSetVersion=1.0` uses version 1.0 - `dataSetVersion=1` uses version 1.0 ### Automatically receiving minor version updates When using the `dataSetVersion` parameter, minor data set version updates can automatically be retrieved from the API without having to change the version number explicitly. This can be done by including an asterisk `*` wildcard character in place of a major or minor version. For example: - `dataSetVersion=*` uses the latest major and minor version - `dataSetVersion=2.*` uses the latest minor version of the v2 series

Note I've removed any mention of patch versions as end users aren't supposed to directly know these exist.

Currently, we only have patch versions as an internal concept. They've not really been implemented yet but exist at the DB level as we may need them later for types of 'data replacements' (one to explain another day).

To an end user, they should only be concerned with specifying major and minor versions. They should automatically receive the latest patch version (if there is one).

That makes sense, that's very helpful. thank you Nick!

leeoades · 2025-01-30T10:10:24Z

src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/VersionUtilsTests.cs

+
+            Assert.NotNull(wildcardVersion);
+            Assert.Equal((uint?)major, wildcardVersion.VersionMajor);
+            Assert.Equal((uint?)major, wildcardVersion.VersionMajor);


note: duplicated line

leeoades · 2025-01-30T10:11:37Z

src/GovUk.Education.ExploreEducationStatistics.Common/Utils/VersionUtils.cs

    {
-        versionString = versionString.Trim();
+        var parts = versionString.Trim('v').Trim().Split('.');


Just a question: do we want to support leading blank space before the v? If we do, then I don't think this does.

leeoades · 2025-01-30T10:12:21Z

src/GovUk.Education.ExploreEducationStatistics.Common/Utils/VersionUtils.cs

-    public static bool TryParseWildcard(string versionString,
-        [NotNullWhen(true)] out WildcardVersion? version)
+}
+public record WildcardVersion(uint? VersionMajor, uint? VersionMinor, uint? VersionPatch)


Forgot to mention yesterday - my bad. I think we can call these properties Major, Minor and Patch. I don't think we need a Version prefix on the name.

leeoades · 2025-01-30T10:18:24Z

src/GovUk.Education.ExploreEducationStatistics.Common.Tests/Utils/VersionUtilsTests.cs

+        [InlineData("v*", null, null, null)]
+        [InlineData("v1.*.*", 1, null, null)]
+        [InlineData("v1.*", 1, null, null)]
+        [InlineData("v1.2.*", 1, 2, null)]


As discussed - this currently supports whitespace around the numbers e.g. v1 . 2 . 3 which we probably don't want.

Good spot, covered this now in the tests and the logic.

…ating wildcard

mmoalin · 2025-01-30T16:18:54Z

src/GovUk.Education.ExploreEducationStatistics.Common/Utils/VersionUtils.cs

+        }
+
+        version = null;
+        if (parts.Length > 3 || !parts.All(part => part.Is("*") || uint.TryParse(part, System.Globalization.NumberStyles.None, null, out _)))


Me and Lee had a slack conversation about not parsing the uint multiple times in this function, he has recommended I make more efficient use of this so just on this now.

…xtra tests for parsing wildcard and nonwildcard versions

… add edge test cases and tidy up the docs

…e-analytical-services/explore-education-statistics into EES-5593-wildcard-ds-versions

mmoalin added 15 commits January 24, 2025 00:27

Initial untested changes for implemented wildcard parameter support f…

0ae4b1b

…or endpoints data-sets/{dataSetId}/meta & data-sets/{dataSetId}/query

Unit tests for the wildcard versioning filter query helper

bbd0f0a

Tidy up implementation, support "v*" & "*" dataset version wildcard s…

2973471

…elector.

Update swagger docs.

fbd2ad1

Openapi json update

5620d3f

final Openapi clean up

809d2a7

clean up

d5a2138

Add readme instructions for running API Docs ruby app

af5d2d3

Tidy up

c7015e7

Tidy up

f9d4e73

Add wildcard controller tests to dataset version

d8498ff

Refactor based on feedback from Lee & Mark

2ad8fb8

Tidy up refactor

c1d58bb

Tidy up

6bff9dd

Merge branch 'dev' into EES-5593-wildcard-ds-versions

d4f61ae

mmoalin changed the title ~~Ees 5593 wildcard ds versions~~ Ees 5593 Public API wildcard data set versions Jan 27, 2025

mmoalin changed the title ~~Ees 5593 Public API wildcard data set versions~~ EES-5593 Public API wildcard data set versions Jan 27, 2025

mmoalin changed the title ~~EES-5593 Public API wildcard data set versions~~ EES-5593 Public API wildcard data set versioning Jan 27, 2025

mmoalin added 8 commits January 27, 2025 23:56

final tidy up

596bea3

Merge branch 'EES-5593-wildcard-ds-versions' of https://github.com/df…

c216d3c

…e-analytical-services/explore-education-statistics into EES-5593-wildcard-ds-versions

final tidy up

86f8e0b

final tidy up

165772c

final tidy up

a6b9240

final tidy up

c7decd5

Support 0.* wildcard versioning

4417a34

Tidy up

6b8be33

mmoalin marked this pull request as ready for review January 28, 2025 09:27

Tidy up

eceb9bb

leeoades reviewed Jan 28, 2025

View reviewed changes

src/GovUk.Education.ExploreEducationStatistics.Common/Utils/VersionUtils.cs Outdated Show resolved Hide resolved

leeoades reviewed Jan 28, 2025

View reviewed changes

...on.ExploreEducationStatistics.Public.Data.Api.Tests/UnitTests/WildCardDatasetVersionTests.cs Outdated Show resolved Hide resolved

leeoades reviewed Jan 28, 2025

View reviewed changes

...on.ExploreEducationStatistics.Public.Data.Api.Tests/UnitTests/WildCardDatasetVersionTests.cs Outdated Show resolved Hide resolved

leeoades reviewed Jan 28, 2025

View reviewed changes

...ExploreEducationStatistics.Public.Data.Model/Extensions/DataSetVersionQueryableExtensions.cs Outdated Show resolved Hide resolved

leeoades reviewed Jan 28, 2025

View reviewed changes

ntsim reviewed Jan 28, 2025

View reviewed changes

mmoalin added 3 commits January 29, 2025 12:21

EES-5593 improve tests, api docs

f6d3f59

EES-5593 undo api-docs getting started modifications

1ef51a4

EES-5593 update last_reviewed_on api docs date

6bfc4da

leeoades reviewed Jan 29, 2025

View reviewed changes

mmoalin added 2 commits January 29, 2025 16:18

EES-5593 Simplify main logic, improve tests

cc42679

EES-5593 Simplify main logic, improve tests

2caceec

ntsim reviewed Jan 29, 2025

View reviewed changes

EES-5593 Simplify and improve main logic, tidy up tests

742a12c

leeoades reviewed Jan 30, 2025

View reviewed changes

mmoalin added 3 commits January 30, 2025 14:55

EES-5593 - Update api docs, capture and fix extra test case for valid…

8b8855f

…ating wildcard

EES-5593 - capture and fix extra test case for validating wildcard

4c46941

EES-5593 - capture and fix extra test case for validating wildcard

f514c4b

mmoalin commented Jan 30, 2025

View reviewed changes

mmoalin added 5 commits January 31, 2025 08:29

EES-5593 Simplify FindByVersion, DataSetVersionRecord.TryParse, add e…

95d3be9

…xtra tests for parsing wildcard and nonwildcard versions

EES-5593 tidyup

b796442

EES-5593 - Add edge cases and tidy up tests and record

003e449

EES-5593 - Add record to parse wildcard and normal data set versions,…

f9ca9bf

… add edge test cases and tidy up the docs

Merge branch 'EES-5593-wildcard-ds-versions' of https://github.com/df…

0fe8963

…e-analytical-services/explore-education-statistics into EES-5593-wildcard-ds-versions

mmoalin changed the title ~~EES-5593 Public API wildcard data set versioning~~ OLD EES-5593 Public API wildcard data set versioning Jan 31, 2025

mmoalin mentioned this pull request Jan 31, 2025

EES-5593 Public API wildcard data set versioning #5577

Merged

mmoalin closed this Jan 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OLD EES-5593 Public API wildcard data set versioning #5559

OLD EES-5593 Public API wildcard data set versioning #5559

mmoalin commented Jan 27, 2025 •

edited

Loading

tomjonesdev commented Jan 28, 2025

leeoades Jan 28, 2025

mmoalin Jan 28, 2025

leeoades Jan 29, 2025

leeoades Jan 28, 2025

mmoalin Jan 29, 2025

leeoades Jan 29, 2025

mmoalin Jan 29, 2025

leeoades Jan 28, 2025

mmoalin Jan 28, 2025

leeoades Jan 29, 2025

mmoalin Jan 29, 2025

leeoades commented Jan 28, 2025

leeoades Jan 29, 2025

leeoades Jan 29, 2025

ntsim Jan 29, 2025

mmoalin Jan 30, 2025

leeoades Jan 30, 2025

leeoades Jan 30, 2025

leeoades Jan 30, 2025

leeoades Jan 30, 2025

mmoalin Jan 30, 2025

mmoalin Jan 30, 2025


		namespace GovUk.Education.ExploreEducationStatistics.Public.Data.Api.Tests.UnitTests;

		public class WildCardDatasetVersionTests

OLD EES-5593 Public API wildcard data set versioning #5559

OLD EES-5593 Public API wildcard data set versioning #5559

Conversation

mmoalin commented Jan 27, 2025 • edited Loading

tomjonesdev commented Jan 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leeoades commented Jan 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmoalin commented Jan 27, 2025 •

edited

Loading