[libvirt] [PATCH libvirt-java 1/2] Return a byte[] array with secretGetValue

[Wido den Hollander]

On 05-07-12 10:20, Daniel Veillard wrote:
> On Tue, Jun 26, 2012 at 10:07:31AM +0200, Wido den Hollander wrote:
>> We break the API with this, but Java does not support multiple method signatures with different return types.
>>
>> The old method returned a String, but since a secret can be binary data this type is not suited.
>>
>> Users who now that their secret is in fact a String, can use cast with:
>>
>> Secret secret = conn.secretLookupByUUIDString("uuuuuuuid");
>> String value = new String(secret.getValue());
>
>    While it makes perfect sense to switch the jna layer to byte array as
> otherwise we can't guarantee functional operation in all cases, I aslo
> think we should not break compatibility, since Java doesn't allow
> overload with different value type, it just mean we need a new method
> name.

Yes, I get that. We don't want to break anything here.

>
>    So I'm simply renaming your method name to "getByteValue" but keeping
> getValue() implemented with the cast you suggest, that seems to work for
> me

I tested it and verified it's working for me with cephx authentication keys.

Wido

>
>> Signed-off-by: Wido den Hollander <wido at widodh.nl>
>> ---
>>   src/main/java/org/libvirt/Secret.java      |   13 ++++++++++---
>>   src/main/java/org/libvirt/jna/Libvirt.java |    2 +-
>>   2 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/main/java/org/libvirt/Secret.java b/src/main/java/org/libvirt/Secret.java
>> index 48f7895..39d9122 100644
>> --- a/src/main/java/org/libvirt/Secret.java
>> +++ b/src/main/java/org/libvirt/Secret.java
>> @@ -5,6 +5,9 @@ import org.libvirt.jna.SecretPointer;
>>
>>   import com.sun.jna.Native;
>>   import com.sun.jna.NativeLong;
>> +import com.sun.jna.ptr.LongByReference;
>> +import com.sun.jna.Pointer;
>> +import java.nio.ByteBuffer;
>>
>>   /**
>>    * A secret defined by libvirt
>> @@ -106,12 +109,16 @@ public class Secret {
>>
>>       /**
>>        * Fetches the value of the secret
>> -     *
>> +     *
>>        * @return the value of the secret, or null on failure.
>>        */
>> -    public String getValue() throws LibvirtException {
>> -        String returnValue = libvirt.virSecretGetValue(VSP, new NativeLong(), 0);
>> +    public byte[] getValue() throws LibvirtException {
>> +        LongByReference value_size = new LongByReference();
>> +        Pointer value = libvirt.virSecretGetValue(VSP, value_size, 0);
>>           processError();
>> +        ByteBuffer bb = value.getByteBuffer(0, value_size.getValue());
>> +        byte[] returnValue = new byte[bb.remaining()];
>> +        bb.get(returnValue);
>>           return returnValue;
>>       }
>>
>> diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java
>> index b1e53a2..f53199d 100644
>> --- a/src/main/java/org/libvirt/jna/Libvirt.java
>> +++ b/src/main/java/org/libvirt/jna/Libvirt.java
>> @@ -330,7 +330,7 @@ public interface Libvirt extends Library {
>>       public int virSecretGetUUID(SecretPointer virSecretPtr, byte[] uuidString);
>>       public int virSecretGetUUIDString(SecretPointer virSecretPtr, byte[] uuidString);
>>       public String virSecretGetUsageID(SecretPointer virSecretPtr);
>> -    public String virSecretGetValue(SecretPointer virSecretPtr, NativeLong value_size, int flags);
>> +    public Pointer virSecretGetValue(SecretPointer virSecretPtr, LongByReference value_size, int flags);
>>       public String virSecretGetXMLDesc(SecretPointer virSecretPtr, int flags);
>>       public SecretPointer virSecretLookupByUsage(ConnectionPointer virConnectPtr, int usageType, String usageID);
>>       public SecretPointer virSecretLookupByUUID(ConnectionPointer virConnectPtr, byte[] uuidBytes);
>> --
>> 1.7.9.5
>
>    So I'm squashing the following on top of your patch.
> I will reword the commit message slightly too. I'm sorry that this mean
> you will have to rename getValue() to getByteValue() but I prefer that
> than risking to break an unknown amount of existing users ... I hope you
> understand.
>
> diff --git a/src/main/java/org/libvirt/Secret.java b/src/main/java/org/libvirt/Secret.java
> index 39d9122..561c258 100644
> --- a/src/main/java/org/libvirt/Secret.java
> +++ b/src/main/java/org/libvirt/Secret.java
> @@ -108,11 +108,23 @@ public class Secret {
>       }
>
>       /**
> -     * Fetches the value of the secret
> +     * Fetches the value of the secret as a string (note that
> +     * this may not always work and getByteValue() is more reliable)
> +     * This is just kept for backward compatibility
>        *
>        * @return the value of the secret, or null on failure.
>        */
> -    public byte[] getValue() throws LibvirtException {
> +    public String getValue() throws LibvirtException {
> +        String returnValue = new String(getByteValue());
> +        return returnValue;
> +    }
> +
> +    /**
> +     * Fetches the value of the secret as a byte array
> +     *
> +     * @return the value of the secret, or null on failure.
> +     */
> +    public byte[] getByteValue() throws LibvirtException {
>           LongByReference value_size = new LongByReference();
>           Pointer value = libvirt.virSecretGetValue(VSP, value_size, 0);
>           processError();
>