Feature/1693 api v3 disabled fails validation #2066

dk1844 · 2022-05-16T14:37:53Z

Implementation of PR review #2055 (comment), specifically:

disallow all PUT/POST calls except for PUT on /{name} to enable.

validate should return error disabled so it cannot be run in spark-jobs.

It has its own PR, because there are a lot of changes in the end (introducing VersionedModelServiceV3 - and changing some abstract classes to traits in order to multi-inherit in specific services both from VersionedModelServiceV3 (API V3 backing) and from V2 services that already hold a lot of reusable implementation.

Partial for #1693

…e contours, empty DatasetControllerV3 to test

- login allowed at /api/login & /api-v3/login - v2/v3 BaseRestApiTest distinquished

…st cases added

…der and no content -> IT updated

… IT updated

…version, includes a sample CR)

…st and post-import + IT to prove correct behavior

…mport + IT to prove correct behavior - fix with common location processing with segment stripping (+normalization)

… GET datasets/{name}/{version}

…/audit-trail added

…ion} now works for # or 'latest' (IT = regression test)

…{name}/{version} and /{name}/latest - improved

…- supports latest for as version-expression, impl for datasets improved by actual existence checking + IT test cases for non-existing/non-latest queries

…properties - supports latest for as version-expression; get impl is unvalidated, put impl checks validity - login is now common, under /api/login for both v2 and v3 (did not work previously)

…properties - supports latest for as version-expression; get impl is unvalidated, put impl checks validity - extended for different validation cases - login is now common, under /api/login for both v2 and v3 (did not work previously)

…ture -> CompletableFuture (mistake reverted)

… added. IT mostly adjusted, but there are todos - DatasetServiceV3 introduced to carry difference in behavior to DatasetService. original entity validation has been divided into create-validation and regular-entity validation. - buildfix for VersionedModelServiceTest

…sion}` in proper validations

…asets/dsName/version/rules, GET datasets/dsName/version/rules/# + IT

- v2 ignores this added information - needs

…api-v3-schema-etc

…defaults

…ts + IT

…, cleanup

…api-v3-disabled-fails-validation

rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/RunService.scala

AdrianOlosutean · 2022-05-24T06:22:54Z

rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/VersionedModelServiceV3.scala

+  }
+
+  // v3 has internal validation on importItem (because it is based on update), v2 only had it on import
+  override def importSingleItem(item: C, username: String, metadata: Map[String, String]): Future[Option[(C, Validation)]] = {


Why is the metadata passed here?

Thanks for noticing and making me realizing the problem! 🙇‍♂️
The metadata is passed there, because it should have been checked, too, but this implementation failed to do so.

Should be fixed now, I have updated the code and added test cases for both V2 and V3 import API to hold this together.

…s-validation % Conflicts: % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/exceptions/EndpointDisabled.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/exceptions/EndpointDisabledException.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/exceptions/EntityDisabledException.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/DatasetService.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/PropertyDefinitionService.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/VersionedModelService.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/DatasetServiceV3.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/MappingTableServiceV3.scala % rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/PropertyDefinitionServiceV3.scala % rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala % rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/MappingTableControllerV3IntegrationSuite.scala

…:1}` to be present on imports. + IT

AdrianOlosutean

Looks good, code review

rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/VersionedModelServiceV3.scala

…t fix

sonarcloud · 2022-05-27T12:43:32Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
11 Code Smells

No Coverage information
2.5% Duplication

AdrianOlosutean

LGTM

AdrianOlosutean

I was testing the API and I think DatasetControllerV3 should also have this validation on updating the properties: PUT on"/{name}/{version}/properties"

dk1844 · 2022-05-31T07:55:26Z

I was testing the API and I think DatasetControllerV3 should also have this validation on updating the properties: PUT on"/{name}/{version}/properties"

What do you mean? DatasetServiceV3.updateProperties() does validate properties being updated. Which part of validation do you consider missing, @AdrianOlosutean ?

AdrianOlosutean · 2022-05-31T07:57:50Z

When I tested this endpoint on a disabled dataset, I didn't see the disabled validation

I was testing the API and I think DatasetControllerV3 should also have this validation on updating the properties: PUT on"/{name}/{version}/properties"

What do you mean? DatasetServiceV3.updateProperties() does validate properties being updated. Which part of validation do you consider missing, @AdrianOlosutean ?

dk1844 · 2022-05-31T08:48:26Z

When I tested this endpoint on a disabled dataset, I didn't see the disabled validation

I was testing the API and I think DatasetControllerV3 should also have this validation on updating the properties: PUT on"/{name}/{version}/properties"

What do you mean? DatasetServiceV3.updateProperties() does validate properties being updated. Which part of validation do you consider missing, @AdrianOlosutean ?

I can see this covered in the (passing) integTests:
direct file
diff view

Zejnilovic

Build, run, code review

AdrianOlosutean

Tested locally as well

dk1844 added 30 commits March 7, 2022 15:35

#1693 API v3: VersionedModel v3 init - VersionedModelControllerV3 bas…

f1798b2

…e contours, empty DatasetControllerV3 to test

#1693 API v3: some basic VersionedModelControllerV3 integration test

0f62851

- login allowed at /api/login & /api-v3/login - v2/v3 BaseRestApiTest distinquished

#1693 API v3: VersionedModelControllerV3 integration test cont'd - te…

c01cfda

…st cases added

#1693 API v3: VersionedModelControllerV3 - post to yield location hea…

4fd08e2

…der and no content -> IT updated

#1693 API v3: VersionedModelControllerV3 - put to yield no content ->…

5c8e005

… IT updated

#1693 API v3: VersionedModelControllerV3 - IT for /ds/export (latest …

458bfed

…version, includes a sample CR)

#1693 API v3: VersionedModelControllerV3 - location header for put/po…

a7a6cf9

…st and post-import + IT to prove correct behavior

#1693 API v3: VersionedModelControllerV3 - location header for post-i…

af489b1

…mport + IT to prove correct behavior - fix with common location processing with segment stripping (+normalization)

#1693 API v3: VersionedModelControllerV3 -IT for GET datasets/{name},…

21f8008

… GET datasets/{name}/{version}

#1693 API v3: VersionedModelControllerV3 - IT for GET datasets/{name}…

6f63117

…/audit-trail added

#1693 API v3: VersionedModelControllerV3 - GET /datasets/{name}/{vers…

76e87ce

…ion} now works for # or 'latest' (IT = regression test)

#1693 API v3: VersionedModelControllerV3 - common implementation of /…

d62cd90

…{name}/{version} and /{name}/latest - improved

#1693 API v3: VersionedModelControllerV3 - /{name}/{version}/used-in …

fc4c359

…- supports latest for as version-expression, impl for datasets improved by actual existence checking + IT test cases for non-existing/non-latest queries

#1693 API v3: VersionedModelControllerV3 - GET/PUT /{name}/{version}/…

e6c8d8c

…properties - supports latest for as version-expression; get impl is unvalidated, put impl checks validity - login is now common, under /api/login for both v2 and v3 (did not work previously)

#1693 Fix: (v2) VersionedModelController.getLatestVersionNumber(): Fu…

219360a

…ture -> CompletableFuture (mistake reverted)

#1693 VersionedModelControllerV3 - IT extended for `POST /{name}/{ver…

101b37a

…sion}` in proper validations

#1693 VersionedModelControllerV3 - conformance rule mgmt GET+POST dat…

b934958

…asets/dsName/version/rules, GET datasets/dsName/version/rules/# + IT

#1693 PR review update - typos etc

51ad013

#1693 Validation-warnings as payload to caller in V3

ba5edce

- v2 ignores this added information - needs

#1693 Validation-warnings as payload to caller in V3 - IT covered

662119b

#1693 API v3 impl for MTs, no tests yet

b0fd955

#1693 import validation check added to IT.

d480445

Merge branch 'feature/1693-api-v3-versioned-model' into feature/1693-…

bffbf55

…api-v3-schema-etc

Merge branch 'develop-ver-3.0' into feature/1693-api-v3-versioned-model

3542a43

Merge branch 'feature/1693-api-v3-versioned-model' into feature/1693-…

51d68a8

…api-v3-schema-etc

#1693 api-v3 mapping table specific - IntegTest for GET/POST/PUT .../…

e1c904e

…defaults

#1693 api-v3 mapping table specific - MT now validates if schema exis…

489ae12

…ts + IT

#1693 schema validation for DSs + IntegTest update

d6ef401

dk1844 added 2 commits May 18, 2022 11:30

#1693 Future {throw x} replaced with Future.failed(x) in rest_api…

34292c4

…, cleanup

Merge branch 'feature/1693-api-v3-delete-recreate' into feature/1693-…

698086d

…api-v3-disabled-fails-validation

Zejnilovic reviewed May 20, 2022

View reviewed changes

rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/RunService.scala Show resolved Hide resolved

AdrianOlosutean reviewed May 24, 2022

View reviewed changes

Base automatically changed from feature/1693-api-v3-delete-recreate to develop-ver-3.0 May 24, 2022 09:44

dk1844 added 3 commits May 24, 2022 13:09

#1693 V2 /importItem integTest added

6bf7302

#1693 bugfix: V3 API now correctly checks `metadata":{"exportVersion"…

243f9e2

…:1}` to be present on imports. + IT

dk1844 requested review from AdrianOlosutean and Zejnilovic May 24, 2022 13:18

AdrianOlosutean previously approved these changes May 25, 2022

View reviewed changes

rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/VersionedModelServiceV3.scala Show resolved Hide resolved

#1693 PropertyDefinitionControllerV3IntegrationSuite - metadata impor…

cab3ae0

…t fix

dk1844 dismissed AdrianOlosutean’s stale review via cab3ae0 May 27, 2022 10:52

#1693 Incorrectly merged SchemaControllerV3IntegrationSuite fixed.

b3ffe43

dk1844 requested a review from AdrianOlosutean May 27, 2022 14:15

AdrianOlosutean approved these changes May 27, 2022

View reviewed changes

dk1844 requested a review from AdrianOlosutean May 30, 2022 07:24

AdrianOlosutean added the PR:reviewing Only for PR - PR is being reviewed by somebody; blocks merging label May 30, 2022

AdrianOlosutean reviewed May 30, 2022

View reviewed changes

AdrianOlosutean removed the PR:reviewing Only for PR - PR is being reviewed by somebody; blocks merging label May 30, 2022

Zejnilovic approved these changes May 31, 2022

View reviewed changes

Zejnilovic added the PR:tested Only for PR - PR was tested by a tester (person) label May 31, 2022

AdrianOlosutean approved these changes May 31, 2022

View reviewed changes

dk1844 merged commit 9cf521e into develop-ver-3.0 May 31, 2022

dk1844 deleted the feature/1693-api-v3-disabled-fails-validation branch May 31, 2022 13:13

dk1844 linked an issue Jun 7, 2022 that may be closed by this pull request

Experimental API 3 - Versioned Model #1693

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/1693 api v3 disabled fails validation #2066

Feature/1693 api v3 disabled fails validation #2066

dk1844 commented May 16, 2022 •

edited

Loading

AdrianOlosutean May 24, 2022

dk1844 May 24, 2022 •

edited

Loading

AdrianOlosutean left a comment

sonarcloud bot commented May 27, 2022

AdrianOlosutean left a comment

AdrianOlosutean left a comment

dk1844 commented May 31, 2022

AdrianOlosutean commented May 31, 2022

dk1844 commented May 31, 2022

Zejnilovic left a comment

AdrianOlosutean left a comment

Feature/1693 api v3 disabled fails validation #2066

Feature/1693 api v3 disabled fails validation #2066

Conversation

dk1844 commented May 16, 2022 • edited Loading

AdrianOlosutean May 24, 2022

Choose a reason for hiding this comment

dk1844 May 24, 2022 • edited Loading

Choose a reason for hiding this comment

AdrianOlosutean left a comment

Choose a reason for hiding this comment

sonarcloud bot commented May 27, 2022

AdrianOlosutean left a comment

Choose a reason for hiding this comment

AdrianOlosutean left a comment

Choose a reason for hiding this comment

dk1844 commented May 31, 2022

AdrianOlosutean commented May 31, 2022

dk1844 commented May 31, 2022

Zejnilovic left a comment

Choose a reason for hiding this comment

AdrianOlosutean left a comment

Choose a reason for hiding this comment

dk1844 commented May 16, 2022 •

edited

Loading

dk1844 May 24, 2022 •

edited

Loading