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

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(a)widodh.nl&gt; > --- > 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();