[ 
https://issues.apache.org/jira/browse/CAMEL-12933?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Maksym Shalak updated CAMEL-12933:
----------------------------------
    Description: 
After upgrade from Camel 2.14 to 2.22.1, I have noticed it no longer sets 
"CamelFileHost" header for Exchange input. After debugging and comparing 
differences between old and new execution, I have noticed that Camel stopped 
setting CamelFileHost header after changes in 
[https://github.com/apache/camel/commit/e3a1bdb6a278b5e4910ba4caf3ebe95751cceaee#diff-f9d7a01c99e5d4239aae6b6834cdfc65].

This happens because previously we had execution chain:
{code:java}
FtpConsumer->RemoteFileEndpoint.createExchange->GenericFile.bindToExchange->RemoteFile.populateHeaders(msg)
{code}
But after this change we have:
{code:java}
FtpConsumer->RemoteFileEndpoint.createExchange->GenericFile.bindToExchange->GenericFile.populateHeaders(msg,
 false)
{code}
This happens because bindToExchange method sets all the headers via 
populateHeaders. This method is overriden in RemoteFile, and it adds 
CamelFileHost header.
{code:java}
public void populateHeaders(GenericFileMessage<T> message) {
    if (message != null) {
        // because there is not probeContentType option 
        // in other file based components, false may be passed
        // as the second argument.
        super.populateHeaders(message, false);
        message.setHeader("CamelFileHost", getHostname());
    }
}
{code}
But after changes the signature of the parent method was changed from
{code:java}
public void populateHeaders(GenericFileMessage<T> message) {
{code}
to
{code:java}
public void populateHeaders(GenericFileMessage<T> message, boolean 
isProbeContentTypeFromEndpoint) {
{code}
But its signature was not changed in RemoteFile. Since it is missing the 
@Override annotation, it compiled well and was unnoticed. But now the 
overridden method in RemoteFile does not get executed, and we end up without 
CamelFileHost header. It may be unnoticed by those who do not use this header, 
but if we rely on it, it may be an issue.

My proposed changes will be:
 # Add @Override annotation to populateHeaders method in RemoteFile, so the 
same bug will not happen.
 # Change its signature to
{code:java}
public void populateHeaders(GenericFileMessage<T> message, boolean 
isProbeContentTypeFromEndpoint)
{code}
So it will be called instead of the parent method. If we assume someone already 
uses this method somehow and we want to make these changes backward compatible, 
we may just add a new method with this signature and @Override annotation, 
leaving existing populateHeaders(GenericFileMessage<T> message) without changes.

  was:
After upgrade from Camel 2.14 to 2.22.1, I have noticed it no longer sets 
"CamelFileHost" header for Exchange input. After debugging and comparing 
differences between old and new execution, I have noticed that Camel stopped 
setting CamelFileHost header after changes in 
[https://github.com/apache/camel/commit/e3a1bdb6a278b5e4910ba4caf3ebe95751cceaee#diff-f9d7a01c99e5d4239aae6b6834cdfc65].

This happens because previously we had execution chain:
{code:java}
FtpConsumer->RemoteFileEndpoint.createExchange->GenericFile.bindToExchange->RemoteFile.populateHeaders(msg)
{code}
But after this change we have:
{code:java}
FtpConsumer->RemoteFileEndpoint.createExchange->GenericFile.bindToExchange->GenericFile.populateHeaders(msg,
 false)
{code}
This happens because bindToExchange method sets all the headers via 
populateHeaders. This method is overriden in RemoteFile, and it adds 
CamelFileHost header.
{code:java}
public void populateHeaders(GenericFileMessage<T> message) {
    if (message != null) {
        // because there is not probeContentType option 
        // in other file based components, false may be passed
        // as the second argument.
        super.populateHeaders(message, false);
        message.setHeader("CamelFileHost", getHostname());
    }
}
{code}
But after changes the signature of the parent method was changed from
{code:java}
public void populateHeaders(GenericFileMessage<T> message) {
{code}
to
{code:java}
public void populateHeaders(GenericFileMessage<T> message, boolean 
isProbeContentTypeFromEndpoint) {
{code}
But its signature was not changed in RemoteFile. Since it is missing the 
@Override annotation, it compiled well and was unnoticed. But now the 
overridden method in RemoteFile does not get executed, and we end up without 
CamelFileHost header. It may be unnoticed by those who do not use this header, 
but if we rely on it it may be an issue.

My proposed changes will be:
 # Add @Override annotation to populateHeaders method in RemoteFile, so the 
same bug will not happen.
 # Change its signature to
{code:java}
public void populateHeaders(GenericFileMessage<T> message, boolean 
isProbeContentTypeFromEndpoint)
{code}
So it will be called instead of the parent method. If we assume someone already 
uses this method somehow and we want to make these changes backward compatible, 
we may just add a new method with this signature and @Override annotation, 
leaving existing populateHeaders(GenericFileMessage<T> message) without changes.


> Camel FTP regression: RemoteFile does not override populateHeaders method
> -------------------------------------------------------------------------
>
>                 Key: CAMEL-12933
>                 URL: https://issues.apache.org/jira/browse/CAMEL-12933
>             Project: Camel
>          Issue Type: Bug
>          Components: camel-ftp
>    Affects Versions: 2.22.1
>            Reporter: Maksym Shalak
>            Priority: Major
>
> After upgrade from Camel 2.14 to 2.22.1, I have noticed it no longer sets 
> "CamelFileHost" header for Exchange input. After debugging and comparing 
> differences between old and new execution, I have noticed that Camel stopped 
> setting CamelFileHost header after changes in 
> [https://github.com/apache/camel/commit/e3a1bdb6a278b5e4910ba4caf3ebe95751cceaee#diff-f9d7a01c99e5d4239aae6b6834cdfc65].
> This happens because previously we had execution chain:
> {code:java}
> FtpConsumer->RemoteFileEndpoint.createExchange->GenericFile.bindToExchange->RemoteFile.populateHeaders(msg)
> {code}
> But after this change we have:
> {code:java}
> FtpConsumer->RemoteFileEndpoint.createExchange->GenericFile.bindToExchange->GenericFile.populateHeaders(msg,
>  false)
> {code}
> This happens because bindToExchange method sets all the headers via 
> populateHeaders. This method is overriden in RemoteFile, and it adds 
> CamelFileHost header.
> {code:java}
> public void populateHeaders(GenericFileMessage<T> message) {
>     if (message != null) {
>         // because there is not probeContentType option 
>         // in other file based components, false may be passed
>         // as the second argument.
>         super.populateHeaders(message, false);
>         message.setHeader("CamelFileHost", getHostname());
>     }
> }
> {code}
> But after changes the signature of the parent method was changed from
> {code:java}
> public void populateHeaders(GenericFileMessage<T> message) {
> {code}
> to
> {code:java}
> public void populateHeaders(GenericFileMessage<T> message, boolean 
> isProbeContentTypeFromEndpoint) {
> {code}
> But its signature was not changed in RemoteFile. Since it is missing the 
> @Override annotation, it compiled well and was unnoticed. But now the 
> overridden method in RemoteFile does not get executed, and we end up without 
> CamelFileHost header. It may be unnoticed by those who do not use this 
> header, but if we rely on it, it may be an issue.
> My proposed changes will be:
>  # Add @Override annotation to populateHeaders method in RemoteFile, so the 
> same bug will not happen.
>  # Change its signature to
> {code:java}
> public void populateHeaders(GenericFileMessage<T> message, boolean 
> isProbeContentTypeFromEndpoint)
> {code}
> So it will be called instead of the parent method. If we assume someone 
> already uses this method somehow and we want to make these changes backward 
> compatible, we may just add a new method with this signature and @Override 
> annotation, leaving existing populateHeaders(GenericFileMessage<T> message) 
> without changes.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to