Feature/1692 api v3 runs

[dk1844]

This PR implements Runs v3 API #1692.

• Pagination functionality is not included.
• Endpoints description below
• Endpoints are covered by an integTest, the RunRepositoryV3 is covered by a separate integTest, too.
• Special attention was given to efficiently aggregate the queries of the run collection

Endpoints details

GET /runs[?startDate=yyyy-MM-dd|sparkAppId=...|uniqueId=...]

• returns (wrapped) list of RunSummaries (Run without splineRef and controlMeasure)
• form of "last of each" (having runs grouped by datasetName and versionName - the Run with the latest runId is only returned

GET /runs/{datasetName}[?startDate=yyyy-MM-dd]

• same as above

GET /runs/{datasetName}/{datasetVersion}[?startDate=yyyy-MM-dd]

• returns (wrapped) list of RunSummaries (Run without splineRef and controlMeasure)
• no filtering, objects with all runIds should be returned

PUT /runs/{datasetName}/{datasetVersion}

• payload = Run
• response contains the Location header with url to the newly created entity

GET /runs/{datasetName}/{datasetVersion}/{runId}

• returns the Run object (contains splineRef and controlMeasure)
• allows latest for a runId instead of #.

PUT /runs/{datasetName}/{datasetVersion}/{runId}

• payload = RunStatus (logically updated state, optional error)
• there is currently no restrictions on the state transitions (details here)

GET /runs/{datasetName}/{datasetVersion}/{runId}/checkpoints

• allows latest for a runId instead of #.

POST /runs/{datasetName}/{datasetVersion}/{runId}/checkpoints

• payload = Checkpoint
• the new checkpoint is check for name duplicate and refused with a failed validation in such a case
• response contains the Location header with url to the newly created entity

GET /runs/{datasetName}/{datasetVersion}/{runId}/checkpoints/{checkpointName}

• allows latest for a runId instead of #

GET /runs/{datasetName}/{datasetVersion}/{runId}/metadata

• allows latest for a runId instead of #.
• there is no endpoint for GET .../metadata/{metadataName}, because it does not seem to make a lot of sence (but I am open to suggstions in this part)

Consideration for latest-of-each RunSummaries in MongoDB

The latest-of-each retrieval poses an approach conundrum. I have basically thought about these options to implement it:

1. grouping with max(runId) and joining on the original documents data - using lookup just as the V2 repository does it
2. using Mongo's aggregate, while storing the documents in the with the group and then filtering by the max(runId)
3. leverage a sort-of-all before grouping and then use grouping with first aggregate accumulator (guaranteed to be the max runId because of the pre-sorting)

The first option seems like overkill, but I admit I have not tried it. With options 2 and 3, I have written basic implementations of such aggregate queries and rudimentarily measured it on our UAT environment holding ~450k run records.

Option 3 appeared to take about two times (~30s) more time than option 2 (~15s), provided that there is an appropriate index present in the collection for the pre-sorting. Surprisingly, this index { $sort: { "runId": -1 } } was present on our UAT, but must have been added extra, because our default setup does not introduce this index to the runs_v1 collection. Thus, in the code, I have added it to be present for future MongoDB provisionings.

Option 2 mongo script used:

db.getCollection('run_v1').aggregate([
{ $sort: { "runId": -1 } }, // because of this sorting, the $first below yields max runId in the groups
{
    $group: {
      _id: {
           "dataset": "$dataset",
           "datasetVersion": "$datasetVersion"
      },
      datasetName: { $first : "$dataset"},
      datasetVersion: { $first : "$datasetVersion"},
      runId: { $first: "$runId" },
      status: { $first: "$runStatus.status" },
      startDateTime: { $first : "$startDateTime"},
      uniqueId: { $first: "$uniqueId" }

    },
},
{
    $project: {
        "_id" : 0 
    }
}
])

Option 3 mongo script used:

db.getCollection('run_v1').aggregate([
{
    $group: {
      _id: {
           "dataset": "$dataset",
           "datasetVersion": "$datasetVersion"
      },
      maxRunId: { $max: "$runId"},
      docs: {
          $push: '$$ROOT' // saving the documents
      }
    }
},
{
  $project: {
    _id: 0,
    doc: {
      "$setDifference": [{
          "$map": {
            "input": "$docs",
            "as": "mapped",
            "in": {
              "$cond": [{
                  "$eq": ["$maxRunId", "$$mapped.runId"] // filtering those with maxRunId here
                },
                "$$mapped",
                false
              ]
            }
          }
        },
        [false]
      ]
    }

  }
}, {
  $unwind: '$doc'
}, {
    $project: {
      datasetName: "$doc.dataset",
      datasetVersion: "$doc.datasetVersion",
      runId: "$doc.runId" ,
      status: "$doc.runStatus.status" ,
      startDateTime: "$doc.startDateTime",
      uniqueId:  "$doc.uniqueId" 
    }
}
  
], { allowDiskUse: true })

[benedeki]

Love the comment 👍

[benedeki]

If you would have time to explain this to me (educating 😉 )

[dk1844]

RunServiceV3 extends RunService. This has a few (expected) effects:

• Wherever a field of type RunServiceV3 is needed, @Autowired only has 1 option to use - RunServiceV3.
• Wherever a field of type RunService is needed, @Autowired has 2 options to use - RunService or RunServiceV3 and thus it fails with ambiguity exception (No qualifying bean of type 'za.co.absa.enceladus.rest_api.services.RunService' available: expected single matching bean but found 2: runService,runServiceV3). Therefore it needs to be disambiguated somehow (some details here, but basically this is summed up by By default, Spring resolves @Autowired entries by type.)

There are multiple ways how to hint the desired wiring to be used, e.g. mark one of the implementations by @Primary etc. The option I have used here is that I mark the bean with a qualifier -- in my case one that matches the field name where requested. So, I have:

@Service("runService") // by-name qualifier: making V2 autowiring un-ambiguous
class RunService()

@Service
class RunServiceV3() extends RunService

class LandingPageController @Autowired() (
  ...,
  runService: RunService, // matches the qualifier of RunService by field name => no ambiguity, RunService is wired.
)

In case that you could not match the name of the qualifier with the field name (unable to change either for some reason), this can be solved by adding the @Qualifier(qualifierName) to the property as shown with @Qualifier("fooFormatter") at https://www.baeldung.com/spring-autowire#1-autowiring-by-qualifier. So it would look like this:

@Service("runService")
class RunService()

@Service
class RunServiceV3() extends RunService

class LandingPageController @Autowired() (
  ...,
  @Qualifier("runService") // matches the qualifier of RunService => no ambiguity, RunService is wired.
  runServiceWithADifferentName: RunService, 
)

[benedeki]

Another education request, please.

[dk1844]

This is the same case as #2073 (comment)
A field of type RunMongoRepository can be wired with more than one class, so the qualifier here helps to resolve that -- when the field of this type is named runMongoRepository or accompanied by @Qualifier("runMongoRepository").

[Zejnilovic]

Question 1

On non-existing URLs I do not get a 404 but rather an empty array.
/api-v3/runs/DeleteMeXYZ -> the dataset does not exist
/api-v3/runs/DeleteMe/1223 -> the version doesn't exist
These return 200 and an empty array.

I can see why return an empty array if the dataset exists but has no runs for the Name or the Version, but if the dataset name or combination of name and version don't even exist in Dataset collection, then it is misleading.

Thoughts?

[benedeki]

Question 1

On non-existing URLs I do not get a 404 but rather an empty array. /api-v3/runs/DeleteMeXYZ -> the dataset does not exist /api-v3/runs/DeleteMe/1223 -> the version doesn't exist These return 200 and an empty array.

I can see why return an empty array if the dataset exists but has no runs for the Name or the Version, but if the dataset name or combination of name and version don't even exist in Dataset collection, then it is misleading.

Thoughts?

I agree. 404 in case Dataset & Version does not exists.
Empty array if no runs for existing Dataset & Version

[Zejnilovic]

Suggestion 2

on Update status, that is PUT /runs/{datasetName}/{datasetVersion}/{runId}, we return plaintext message. I would rather go with

{
  "status": "updated",
  "message": "message"
}

But this already smells of the JSON envelope. So maybe we keep it for the next PR

[Zejnilovic]

code reviewed
built
run

[AdrianOlosutean]

Code review

[dk1844]

Hi guys,

Question 1

On non-existing URLs I do not get a 404 but rather an empty array. /api-v3/runs/DeleteMeXYZ -> the dataset does not exist /api-v3/runs/DeleteMe/1223 -> the version doesn't exist These return 200 and an empty array.
I can see why return an empty array if the dataset exists but has no runs for the Name or the Version, but if the dataset name or combination of name and version don't even exist in Dataset collection, then it is misleading.
Thoughts?

I agree. 404 in case Dataset & Version does not exists. Empty array if no runs for existing Dataset & Version

This I have fixed as suggested. For /api-v3/runs/justDatasetNameWithoutVersion, I have added check for latest-version, so this option is covered as well to produce 404 if no version is found for this dataset.
Tests added.

Suggestion 2

on Update status, that is PUT /runs/{datasetName}/{datasetVersion}/{runId}, we return plaintext message. I would rather go with

{
  "status": "updated",
  "message": "message"
}

But this already smells of the JSON envelope. So maybe we keep it for the next PR

Yes, it does smell a bit as to be part of #2062, thus, so far, I have just wrapped the response in an object with a message field (so it is a valid JSON with the message, but no other fields like status), but for the rest of the effort, we should solve this in #2062

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
19 Code Smells

No Coverage information
9.3% Duplication

[AdrianOlosutean]

LGTM