Mailing-List: contact dev-help@camel.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@camel.apache.org
MIME-Version: 1.0
In-Reply-To: <CAGB5yNkjXth5zftC_5HHPHgBaVCdv=PjW2ftZPnbNDr9QUXy1Q@mail.gmail.com>
References: <0266d564db534dd38ad7122a00c20505@git.apache.org> <CAGB5yNkjXth5zftC_5HHPHgBaVCdv=PjW2ftZPnbNDr9QUXy1Q@mail.gmail.com>
From: Zoran Regvart <zoran@regvart.com>
Date: Wed, 1 Mar 2017 13:00:27 +0100
Message-ID: <CABD_Zr9fqRYuHhVvpg_sOk8ydZsTC8qMhZFRHSwiaChrc1f0pw@mail.gmail.com>
Subject: Re: camel git commit: CAMEL-10915 Add type conversion support for jav...
To: dev@camel.apache.org
Cc: commits@camel.apache.org
Content-Type: text/plain; charset=UTF-8
archived-at: Wed, 01 Mar 2017 12:00:54 -0000

Hi Claus,
thanks for reviewing :) Would this be better?

CharSequence toCharSequence(URI value)

and

URI toUri(CharSequence value)

or do you think that URI type converter support should be removed entirely?

zoran

On Wed, Mar 1, 2017 at 12:29 PM, Claus Ibsen <claus.ibsen@gmail.com> wrote:
> On Wed, Mar 1, 2017 at 12:21 PM,  <zregvart@apache.org> wrote:
>> Repository: camel
>> Updated Branches:
>>   refs/heads/master c6a15565a -> d569b80d8
>>
>>
>> CAMEL-10915 Add type conversion support for jav...
>>
>> ...a.net.URI
>>
>> Adds converter to and from `java.net.URI` and `java.lang.CharSequence`.
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/camel/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/d569b80d
>> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/d569b80d
>> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/d569b80d
>>
>> Branch: refs/heads/master
>> Commit: d569b80d81b200ca83a1f7e56d5e20c6f59f6427
>> Parents: c6a1556
>> Author: Zoran Regvart <zregvart@apache.org>
>> Authored: Wed Mar 1 12:17:52 2017 +0100
>> Committer: Zoran Regvart <zregvart@apache.org>
>> Committed: Wed Mar 1 12:20:49 2017 +0100
>>
>> ----------------------------------------------------------------------
>>  .../converter/CorePackageScanClassResolver.java |  1 +
>>  .../camel/impl/converter/UriTypeConverter.java  | 53 +++++++++++++++++++
>>  .../impl/converter/UriTypeConverterTest.java    | 54 ++++++++++++++++++++
>>  3 files changed, 108 insertions(+)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/camel/blob/d569b80d/camel-core/src/main/java/org/apache/camel/impl/converter/CorePackageScanClassResolver.java
>> ----------------------------------------------------------------------
>> diff --git a/camel-core/src/main/java/org/apache/camel/impl/converter/CorePackageScanClassResolver.java b/camel-core/src/main/java/org/apache/camel/impl/converter/CorePackageScanClassResolver.java
>> index ed11fa9..65ec445 100644
>> --- a/camel-core/src/main/java/org/apache/camel/impl/converter/CorePackageScanClassResolver.java
>> +++ b/camel-core/src/main/java/org/apache/camel/impl/converter/CorePackageScanClassResolver.java
>> @@ -77,6 +77,7 @@ public class CorePackageScanClassResolver implements PackageScanClassResolver {
>>          converters.add(GenericFileConverter.class);
>>          converters.add(DurationConverter.class);
>>          converters.add(AttachmentConverter.class);
>> +        converters.add(UriTypeConverter.class);
>>      }
>>
>>      @Override
>>
>> http://git-wip-us.apache.org/repos/asf/camel/blob/d569b80d/camel-core/src/main/java/org/apache/camel/impl/converter/UriTypeConverter.java
>> ----------------------------------------------------------------------
>> diff --git a/camel-core/src/main/java/org/apache/camel/impl/converter/UriTypeConverter.java b/camel-core/src/main/java/org/apache/camel/impl/converter/UriTypeConverter.java
>> new file mode 100644
>> index 0000000..aab8e14
>> --- /dev/null
>> +++ b/camel-core/src/main/java/org/apache/camel/impl/converter/UriTypeConverter.java
>> @@ -0,0 +1,53 @@
>> +/**
>> + * Licensed to the Apache Software Foundation (ASF) under one or more
>> + * contributor license agreements.  See the NOTICE file distributed with
>> + * this work for additional information regarding copyright ownership.
>> + * The ASF licenses this file to You under the Apache License, Version 2.0
>> + * (the "License"); you may not use this file except in compliance with
>> + * the License.  You may obtain a copy of the License at
>> + *
>> + *      http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +package org.apache.camel.impl.converter;
>> +
>> +import java.net.URI;
>> +import java.net.URISyntaxException;
>> +
>> +import org.apache.camel.Converter;
>> +import org.apache.camel.TypeConversionException;
>> +import org.apache.camel.TypeConverter;
>> +
>> +/**
>> + * A {@link TypeConverter} that converts to and from {@link URI}s.
>> + */
>> +public class UriTypeConverter {
>> +
>> +    @Converter
>> +    public static CharSequence toCharSequence(final Object value) {
>> +        final URI uri = toUri(value);
>> +
>> +        return uri.toString();
>> +    }
>
> -1 be careful to do such open ended conversion from Object -> Char
> which is not related to URI.
> That converter should be removed
>
>
>> +
>> +    @Converter
>> +    public static URI toUri(final Object value) {
>> +        if (value instanceof URI) {
>> +            return (URI)value;
>> +        }
>> +
>> +        final String stringValue = String.valueOf(value);
>> +
>> +        try {
>> +            return new URI(stringValue);
>> +        } catch (final URISyntaxException e) {
>> +            throw new TypeConversionException(value, URI.class, e);
>> +        }
>> +    }
>> +
>
> This should not be Object either, its a String -> URI
>
>
>
>
>> +}
>>
>> http://git-wip-us.apache.org/repos/asf/camel/blob/d569b80d/camel-core/src/test/java/org/apache/camel/impl/converter/UriTypeConverterTest.java
>> ----------------------------------------------------------------------
>> diff --git a/camel-core/src/test/java/org/apache/camel/impl/converter/UriTypeConverterTest.java b/camel-core/src/test/java/org/apache/camel/impl/converter/UriTypeConverterTest.java
>> new file mode 100644
>> index 0000000..9cb4289
>> --- /dev/null
>> +++ b/camel-core/src/test/java/org/apache/camel/impl/converter/UriTypeConverterTest.java
>> @@ -0,0 +1,54 @@
>> +/**
>> + * Licensed to the Apache Software Foundation (ASF) under one or more
>> + * contributor license agreements.  See the NOTICE file distributed with
>> + * this work for additional information regarding copyright ownership.
>> + * The ASF licenses this file to You under the Apache License, Version 2.0
>> + * (the "License"); you may not use this file except in compliance with
>> + * the License.  You may obtain a copy of the License at
>> + *
>> + *      http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +package org.apache.camel.impl.converter;
>> +
>> +import java.net.URI;
>> +
>> +import org.apache.camel.TypeConversionException;
>> +import org.junit.Test;
>> +
>> +import static org.junit.Assert.assertEquals;
>> +
>> +public class UriTypeConverterTest {
>> +
>> +    static final String EXAMPLE = "http://www.example.com";
>> +
>> +    static final URI EXAMPLE_URI = URI.create(EXAMPLE);
>> +
>> +    static final String INVALID = ":";
>> +
>> +    @Test(expected = TypeConversionException.class)
>> +    public void shouldComplainOnInvalidStringUrisConvertingToStrings() {
>> +        UriTypeConverter.toCharSequence(INVALID);
>> +    }
>> +
>> +    @Test(expected = TypeConversionException.class)
>> +    public void shouldComplainOnInvalidStringUrisConvertingToUri() {
>> +        UriTypeConverter.toUri(INVALID);
>> +    }
>> +
>> +    @Test
>> +    public void shouldConvertFromStringsToUris() {
>> +        assertEquals(EXAMPLE_URI, UriTypeConverter.toUri(EXAMPLE));
>> +    }
>> +
>> +    @Test
>> +    public void shouldConvertFromUrisToStrings() {
>> +        assertEquals(EXAMPLE, UriTypeConverter.toCharSequence(EXAMPLE_URI));
>> +    }
>> +
>> +}
>>
>
>
>
> --
> Claus Ibsen
> -----------------
> http://davsclaus.com @davsclaus
> Camel in Action 2: https://www.manning.com/ibsen2


-- 
Zoran Regvart