Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic tests for the RDF and JSON Providers over QC #288

Merged
merged 10 commits into from
Feb 8, 2023

Conversation

berezovskyi
Copy link
Member

@berezovskyi berezovskyi commented Jan 28, 2023

Description

See #287

Checklist

  • This PR adds an entry to the CHANGELOG. See https://keepachangelog.com/en/1.0.0/ for instructions. Minor edits are exempt.
  • This PR was tested on at least one Lyo OSLC server or adds unit/integration tests.
  • This PR does NOT break the API

Issues

Closes #287

Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
@berezovskyi
Copy link
Member Author

Added an initial test matrix. The tests are failing due to the JSON Provider always returning query results for any array/collection response:

writeTo(true,
collection.toArray(new Object[collection.size()]),
mediaType,
map,
outputStream);

Note the true value of the first argument (queryResult). The tests look like this now (parameters are: (1) path, and (2) is a query response is expected):

image

@berezovskyi
Copy link
Member Author

While finishing #284, I didn't see a place to refactor out common code, thus I duplicated the code for Jena provider tests. We can refactor both tests independent of #284.

Now, to the results: it's a bit better than the OSLC JSON provider, but still rough edges (unexpected failures) exist. At least, now they are captured in tests:

image

All responses get valid RDF.

With the direct use of JMH, all tests pass. When we try to get JAX-RS to unmarshal the response, we see that all tests pass for the expected Query responses, whereas all but one tests fail for the expected raw collection of resources as a response.

Further investigation led me to org.eclipse.lyo.oslc4j.provider.jena.OslcRdfXmlProvider#isReadable(), which we simplified to always return true similar to isWritable(). Turns out Jersey has a check for the type of the Provider Entity. If it's Object, other checks are not done. Because OslcRdfXmlProvider implements MessageBodyReader<Object> and returns true, all other checks are skipped.

The PR has been updated to reject arrays and collections from being processed by OslcRdfXmlProvider.

NB! We should add the OSLC4JUtils.useBeanClassForParsing() into the test matrix.

@berezovskyi berezovskyi marked this pull request as ready for review February 4, 2023 23:28
@berezovskyi
Copy link
Member Author

berezovskyi commented Feb 4, 2023

@jad-elkhoury, I think the PR is ready for review. In process of writing an extensive test matrix, two important discoveries were made:

  1. The legacy OSLC JSON providers do not support non-Query collection responses. I simply added a description to the test that OSLC JSON provider does not support OslcNotQueryResult and OslcQueryCapability.
  2. The RDF Jena provider OslcRdfXmlProvider became too greedy (regression c65e91f), which made it override array and collection providers on reading. A fix is made and a regression fix entry was added to the CHANGELOG.

Minor discovery is that xUpdatedProvidersRegistry and xSimpleProvidersRegistry are next to useless as they silently wipe important data, such as extended properties and URIs. I could not get the tests pass with them. I would suggest to ask around on the mailing list for the use and deprecate those provider registries.

@jadelkhoury jadelkhoury merged commit 6d82cfd into master Feb 8, 2023
@jadelkhoury jadelkhoury deleted the b287-test-genericentity branch February 8, 2023 09:42
@berezovskyi berezovskyi changed the title Add a basic test for the JSON Provider over QC Add basic tests for the RDF and JSON Providers over QC Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support handling of a typed ResponseBuilder.entity reponse
2 participants