restlet / restlet-framework-java

The first REST API framework for Java
https://restlet.talend.com
Other
653 stars 284 forks source link

EntityClientHelper prevents Directory negotiation #908

Open jgustie opened 10 years ago

jgustie commented 10 years ago

I am seeing an issue where the FileClientHelper.handleEntityGet method is succeeding in a way that breaks negotiation through a Directory. When an exact file is not found, the helper tries locating a sibling file with the same base name and compatible metadata (using Variant.isCompatible), the directory is trying to perform a similar negotiation at a higher level but ends up with an incorrect view of what variants are available.

To reproduce this, you need a directory which contains two files: test.en.txt and test.fr.txt. Then run the following code against that directory:

public class LocalFileTest extends Application {
    public static void main(String[] args) throws Exception {
        Component component = new Component();
        component.getServers().add(Protocol.HTTP, 8080);
        component.getClients().add(Protocol.FILE);
        component.getDefaultHost().attach(new LocalFileTest());
        component.start();
    }

    public LocalFileTest() {
        getTunnelService().setExtensionsTunnel(true);
        // getMetadataService().setDefaultLanguage(null);
        // getMetadataService().setDefaultCharacterSet(null);
    }

    @Override
    public Restlet createInboundRoot() {
        return new Directory(getContext(), "file:///path/to/directory/");
    }
}

Requests for http://localhost:8080/test will negotiate fine based on language preference but requests for http://localhost:8080/test.txt will always return the same representation regardless of language preference (test.en and test.fr work as expected).

Initially I thought that using Variant.includes instead of Variant.isCompatible when determining what the "right representation" is would fix the issue, but it does not. The problem seems more related to the fact that handleEntityGet ends up picking the first of multiple potentially compatible variants (possibly influenced by the default language); this ends up bypassing the ability of the directory to perform negotiation.

One potential solution I see is to continue iterating over the file entries to see if there are more then one compatible variants; if there are, allow the helper to 404, otherwise assume it is safe to return the compatible match (if found). This solution would require unsetting the default language (which is fine because there is no test.txt which needs it): so hopefully the potential impact is minimal.

thboileau commented 10 years ago

Thanks a lot @jgustie for the sample test code! I 'm having a look at it.

thboileau commented 10 years ago

This has some side effects. when the Directory does not support content negotiation. I think that the main point is about taking into account the default metadata or not.

jlouvel commented 10 years ago

@thboileau , shall we give this more thought and postpone? Another idea is to make the changes in 2.3 branch as it seems to have potential for breaking backward compatibility with existing 2.2 applications.

jlouvel commented 10 years ago

Updated milestone. @jgustie Thierry will provide you additional feedback for the next steps.

thboileau commented 10 years ago

I think we can fix it for 2.2 also. Having said that, I see some wrong interactions between the behavior of the Directory/DirectoryServerResource on one side, and the client helper on the other. In addition, I see that the default metadata may interfer also in the way we interpret file names, in the case the directory supports content negotiation or not.

See the DirectoryTestCase#testUserPreferences(MyApplication, Directory) method, if spanish language is the default language, the test will fail.

jlouvel commented 9 years ago

Too late for 2.3.0, we'll fix this in 3.0 and work on backporting this to 2.3 (new "stable" soon).