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

Entity Provider for java.nio.file.Path #1275

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
////
*******************************************************************
* Copyright (c) 2019, 2024 Eclipse Foundation
* Copyright (c) 2019 Eclipse Foundation
*
* This specification document is made available under the terms
* of the Eclipse Foundation Specification License v1.0, which is
Expand All @@ -12,6 +12,8 @@
[[change-log]]
== Change Log

include::_changes-since-4.0-release.adoc[]

include::_changes-since-3.1-release.adoc[]

include::_changes-since-3.0-release.adoc[]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
////
*******************************************************************
* Copyright (c) 2024 Eclipse Foundation
*
* This specification document is made available under the terms
* of the Eclipse Foundation Specification License v1.0, which is
* available at https://www.eclipse.org/legal/efsl.php.
*******************************************************************
////

[[changes-since-4.0-release]]
=== Changes Since 4.0 Release

* <<standard_entity_providers>>: Added entity provider for `java.nio.file.Path`
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
////
*******************************************************************
* Copyright (c) 2019, 2021 Eclipse Foundation
* Copyright (c) 2019 Eclipse Foundation
*
* This specification document is made available under the terms
* of the Eclipse Foundation Specification License v1.0, which is
Expand Down Expand Up @@ -138,6 +138,8 @@ type combinations:
All media types (`\*/*`).
`java.io.File`::
All media types (`\*/*`).
`java.nio.file.Path`::
All media types (`\*/*`).
`jakarta.activation.DataSource`::
All media types (`\*/*`).
`javax.xml.transform.Source`::
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2020 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -133,6 +133,28 @@ public void sendFile(@Context SseEventSink sink, @Context Sse sse) {
}
}

@GET
@Path("path")
@Produces(MediaType.SERVER_SENT_EVENTS)
public void sendPath(@Context SseEventSink sink, @Context Sse sse) {
java.nio.file.Path p;
try (SseEventSink s = sink) {
try {
p = Files.createTempFile("tck", "temppath");
Files.write(p, MESSAGE.getBytes(), StandardOpenOption.CREATE,
StandardOpenOption.APPEND);
p.toFile().deleteOnExit();
s.send(sse.newEventBuilder().data(p).mediaType(MediaType.WILDCARD_TYPE)
.build());
} catch (IOException e) {
s.send(sse.newEvent(e.getMessage()));
throw new RuntimeException(e); // log to server log
}
} catch (IOException e) {
LOG.log(Level.WARNING, "Failed to close SseEventSink", e);
}
}

@GET
@Path("inputstream")
@Produces(MediaType.SERVER_SENT_EVENTS)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -22,6 +22,7 @@
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.InputStream;
import java.nio.file.Files;

import javax.xml.namespace.QName;

Expand Down Expand Up @@ -620,6 +621,110 @@ public void fileWriterClientInterceptorTest() throws Fault {
logMsg("JAXRS called registered writer interceptor for file provider");
}

// ------------------------- Path -----------------------------------

/*
* @testName: pathReaderContainerInterceptorTest
*
* @assertion_ids: JAXRS:SPEC:84;
*
* @test_Strategy: JAX-RS implementations are REQUIRED to call registered
* interceptors when mapping representations to Java types and vice versa.
*/
@Test
@Tag("xml_binding")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: All of these tests (including what was added here) have this @Tag. Do they really all require xml_binding? If not then this may be a problem (even for Jakarta Rest 4.0) since xml_binding is no longer required by the Platform, so none of these tests will run assuming tests tagged this way will be excluded. This applies to all new tests in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, given Jakarta XML Binding is no longer required, implementations won't need to run these effectively allowing implementations to pass without even implementing the feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually did a 1:1 copy from the "file" originals, just replacing "file" by "path". So I assume that why you mention was already wrong before. I hence kindly ask to separate that discussion from this PR, and address that issue in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've written Issue 1277 to document this. However, I don't see the point of adding additional tests that will not run. I understand that you may not have time to evaluate/address all of the tests in this class, but any new that you add should not have that dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what is the correct solution?

  • Simply removing @Tag("xml_binding") from the added test?
  • Not adding the test at all?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point Markus. As long as Issue 1277 is addressed prior to the next release. Can you confirm that the new TCK tests you've added have run successfully (since they currently would be skipped as part of a Jakarta Rest 4.0 TCK execution correct?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should be adding new TCK tests that don't run. It kind of defeats the purpose of a test. While I can understand that's how the old tests work, there's not reason to make new tests work the same way. There could very easily be a new jsonContent instance variable created that these tests use. I see no reason for it to to be XML content. It could even be plain text.

There would be new interceptors needed too, but again IMO that's not really a big deal.

Copy link
Contributor Author

@mkarg mkarg Aug 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jim-krueger I cannot confirm that they run, as there is not yet any implementation that fulfils the new API (or maybe I misunderstood your question).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jamezp For me it is a big deal, as it imposes unnecessary work for me now and it imposes a temporary deviation from the original code, which I want to prevent as good as possible. OTOH as you feel this is not a big deal, maybe you like to chime in and fix #1277 upfront, so I can rebase on the already fixed code base in a second step? We can leave open this PR for the time being. WDYT?

Copy link
Contributor Author

@mkarg mkarg Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we proceed? Apparently nobody chimed in to fix #1277, so could we please merge this feature and solve #1277 later?

public void pathReaderContainerInterceptorTest() throws Fault {
addInterceptors(EntityReaderInterceptor.class);
setProperty(Property.REQUEST, buildRequest(Request.POST, "postpath"));
setRequestContentEntity(content);
setProperty(Property.SEARCH_STRING,
EntityReaderInterceptor.class.getName());
setProperty(Property.UNEXPECTED_RESPONSE_MATCH, Resource.DIRECTION);
invoke();
logMsg("JAXRS called registered reader interceptor for path provider");
}

/*
* @testName: pathReaderNoInterceptorTest
*
* @assertion_ids: JAXRS:SPEC:84;
*
* @test_Strategy: JAX-RS implementations are REQUIRED to call registered
* interceptors when mapping representations to Java types and vice versa.
*/
@Test
@Tag("xml_binding")
public void pathReaderNoInterceptorTest() throws Fault {
setProperty(Property.REQUEST, buildRequest(Request.POST, "postpath"));
setRequestContentEntity(content);
setProperty(Property.SEARCH_STRING, content);
invoke();
}

/*
* @testName: pathWriterContainerInterceptorTest
*
* @assertion_ids: JAXRS:SPEC:84;
*
* @test_Strategy: JAX-RS implementations are REQUIRED to call registered
* interceptors when mapping representations to Java types and vice versa.
*/
@Test
@Tag("xml_binding")
public void pathWriterContainerInterceptorTest() throws Fault {
addInterceptors(EntityWriterInterceptor.class);
setProperty(Property.REQUEST, buildRequest(Request.GET, "getpath"));
setProperty(Property.SEARCH_STRING,
EntityWriterInterceptor.class.getName());
setProperty(Property.SEARCH_STRING, Resource.DIRECTION);
invoke();
logMsg("JAXRS called registered writer interceptor for path provider");
}

/*
* @testName: pathWriterNoInterceptorTest
*
* @assertion_ids: JAXRS:SPEC:84;
*
* @test_Strategy: JAX-RS implementations are REQUIRED to call registered
* interceptors when mapping representations to Java types and vice versa.
*/
@Test
@Tag("xml_binding")
public void pathWriterNoInterceptorTest() throws Fault {
setProperty(Property.REQUEST, buildRequest(Request.GET, "getpath"));
setProperty(Property.SEARCH_STRING, Resource.getName());
invoke();
}

/*
* @testName: pathWriterClientInterceptorTest
*
* @assertion_ids: JAXRS:SPEC:84;
*
* @test_Strategy: JAX-RS implementations are REQUIRED to call registered
* interceptors when mapping representations to Java types and vice versa.
*/
@Test
@Tag("xml_binding")
public void pathWriterClientInterceptorTest() throws Fault {
try {
java.nio.file.Path path = Files.createTempFile("temp", "tmp");
Files.writeString(path, content);
setRequestContentEntity(path);
} catch (IOException e) {
throw new Fault(e);
}
addProvider(EntityWriterInterceptor.class);
addInterceptors(EntityWriterInterceptor.class);
setProperty(Property.REQUEST, buildRequest(Request.POST, "poststring"));
setProperty(Property.SEARCH_STRING,
EntityWriterInterceptor.class.getName());
setProperty(Property.UNEXPECTED_RESPONSE_MATCH, Resource.DIRECTION);
invoke();
logMsg("JAXRS called registered writer interceptor for path provider");
}

// ------------------------- DataSource -----------------------------------

/*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -23,6 +23,7 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import java.nio.file.Files;
import java.util.List;

import javax.xml.namespace.QName;
Expand Down Expand Up @@ -133,6 +134,21 @@ public Response postFile(File file) throws IOException {
return buildResponse(text);
}

@GET
@Path("getpath")
public Response getPath() throws IOException {
java.nio.file.Path path = Files.createTempFile("filter", "tmp");
Files.writeString(path, getName());
return buildResponse(path);
}

@POST
@Path("postpath")
public Response postPath(java.nio.file.Path path) throws IOException {
String text = Files.readString(path);
return buildResponse(text);
}

@GET
@Path("getdatasource")
public Response getDataSource() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -82,7 +82,7 @@ void setPropertyAndInvokeEncoded(String resourceMethod) throws Fault {
}

String[] methodsAll = { "bytearray", "string", "inputstream", "reader",
"file", "datasource", "source", "jaxb", "streamingoutput" };
"file", "path", "datasource", "source", "jaxb", "streamingoutput" };


@Deployment(testable = false)
Expand All @@ -94,7 +94,7 @@ public static WebArchive createDeployment() throws IOException{
AbstractProvider.class, TckBooleanProvider.class,
TckByteArrayProvider.class, TckCharacterProvider.class,
TckDataSourceProvider.class, TckDataSourceProvider.class,
TckFileProvider.class, TckInputStreamProvider.class,
TckFileProvider.class, TckPathProvider.class, TckInputStreamProvider.class,
TckJaxbProvider.class, TckMapProvider.class,
TckNumberProvider.class, TckReaderProvider.class,
TckSourceProvider.class, TckStreamingOutputProvider.class,
Expand Down Expand Up @@ -195,6 +195,22 @@ public void readWriteFileProviderTest() throws Fault {
setPropertyAndInvoke("file", MediaType.APPLICATION_XML_TYPE);
}

/*
* @testName: readWritePathProviderTest
*
* @assertion_ids: JAXRS:SPEC:35
*
* @test_Strategy: An implementation MUST support application-provided entity
* providers and MUST use those in preference to its own pre-packaged
* providers when either could handle the same request.
*
*/
@Test
@Tag("xml_binding")
public void readWritePathProviderTest() throws Fault {
setPropertyAndInvoke("path", MediaType.APPLICATION_XML_TYPE);
}

/*
* @testName: readWriteDataSourceProviderTest
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -63,6 +63,12 @@ public File file(File file) {
return file;
}

@Path("path")
@POST
public java.nio.file.Path path(java.nio.file.Path path) {
return path;
}

@Path("datasource")
@POST
public DataSource datasource(DataSource datasource) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public java.util.Set<java.lang.Class<?>> getClasses() {
resources.add(TckByteArrayProvider.class);
resources.add(TckDataSourceProvider.class);
resources.add(TckFileProvider.class);
resources.add(TckPathProvider.class);
resources.add(TckInputStreamProvider.class);
resources.add(TckJaxbProvider.class);
resources.add(TckMapProvider.class);
Expand Down
Loading