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

[Daniel Veillard]

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.

  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

> 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();

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/