[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] libvirt-java



On Mon, Jul 07, 2008 at 08:26:53PM +0200, Toth Istvan wrote:
> 
> The changes to the API that I think of
> 
> - Make VirConnectCredential and VirConnectCredentialType into an inner 
> class of VirConnectAuth
> - and rename them to Credential and CredentialType

  Okay, I have tried to look at this I ended up with the enclosed patch.
I had a couple of assumptions to do:
  - CredentialType would still be a subtype of Credential
  - adressing from the GetMethodID() and FindClass() would need
    org/libvirt/VirConnectAuth$Credential instead of
    org/libvirt/VirConnectCredential and
    org/libvirt/VirConnectAuth$Credential$CredentialType instead of
    org/libvirt/VirConnectCredential$VirConnectCredentialType
    i.e. two $ lookups

> This gets rids of an extra top level class and encapsulates all auth 
> related data to VirConnectAuth.

  yeah that sounds fine to me hopefully patched accordinly this still works

> - Remove the "vir" prefix from most method names, and inner classes i.e:
> VirConnect.virNodeInfo() would become VirConnect.nodeInfo() and
> VirConnect.virNetworkLookupByName would become  
> VirConnect.networkLookupByName
> 
> Basically, I want to get get rid of the "Vir" prefix from all but the 
> top classes, and the enums.
> 
> I might even consider removing the Vir prefix from the top classes (i.e) 
> VirConnect -> Connect, but I'm not sure about it.

  yeah, I think we can get rid of the Vir prefix for everything, except the
enums, where it's probably better to keep them. This mimics what have been
done in other bindings too.

  I would come up with a patch too easilly but I first want an initial review
on the Credential refactoring if this looks good I will first commit that one
and then as a second step provide a Vir elimination patch (which will be big
since everything will be modified)

  Once that done, after a bit of testing I guess I will make a 0.2.0 release
and push that one to Fedora, sounds right ?

  thanks,

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/
Index: src/jni/VirConnectAuthCallbackBridge.c
===================================================================
RCS file: /data/cvs/libvirt-java/src/jni/VirConnectAuthCallbackBridge.c,v
retrieving revision 1.1
diff -u -p -r1.1 VirConnectAuthCallbackBridge.c
--- src/jni/VirConnectAuthCallbackBridge.c	30 Jun 2008 13:11:55 -0000	1.1
+++ src/jni/VirConnectAuthCallbackBridge.c	9 Jul 2008 13:51:33 -0000
@@ -12,10 +12,10 @@ int	VirConnectAuthCallbackBridge(virConn
 
 	jobject j_auth = ((CallBackStructType*)cbdata)->auth;
 	jclass j_auth_cls = (*env)->GetObjectClass(env, j_auth);
-	jmethodID j_auth_cb_id=(*env)->GetMethodID(env, (*env)->GetObjectClass(env, j_auth), "callback", "([Lorg/libvirt/VirConnectCredential;)I");
+	jmethodID j_auth_cb_id=(*env)->GetMethodID(env, (*env)->GetObjectClass(env, j_auth), "callback", "([Lorg/libvirt/VirConnectAuth$Credential;)I");
 
 
-	jclass j_cred_cls = (*env)->FindClass(env, "org/libvirt/VirConnectCredential");
+	jclass j_cred_cls = (*env)->FindClass(env, "org/libvirt/VirConnectAuth$Credential");
 	jmethodID j_cred_constructor = (*env)->GetMethodID(env, j_cred_cls, "<init>", "(ILjava/lang/String;Ljava/lang/String;Ljava/lang/String;)V");
 	jfieldID j_cred_result_id = (*env)->GetFieldID(env, j_cred_cls, "result", "Ljava/lang/String;");
 	
Index: src/jni/org_libvirt_VirConnect.c
===================================================================
RCS file: /data/cvs/libvirt-java/src/jni/org_libvirt_VirConnect.c,v
retrieving revision 1.2
diff -u -p -r1.2 org_libvirt_VirConnect.c
--- src/jni/org_libvirt_VirConnect.c	30 Jun 2008 13:11:55 -0000	1.2
+++ src/jni/org_libvirt_VirConnect.c	9 Jul 2008 13:51:33 -0000
@@ -200,9 +200,9 @@ JNIEXPORT jlong JNICALL Java_org_libvirt
 	jfieldID credTypeArray_id = (*env)->GetFieldID(env, 
 			(*env)->FindClass(env, "org/libvirt/VirConnectAuth"), 
 			"credType", 
-			"[Lorg/libvirt/VirConnectCredential$VirConnectCredentialType;");
+			"[Lorg/libvirt/VirConnectAuth$Credential$CredentialType;");
 	jmethodID credTypeMapToInt_id = (*env)->GetMethodID(env, 
-			(*env)->FindClass(env, "org/libvirt/VirConnectCredential$VirConnectCredentialType"), 
+			(*env)->FindClass(env, "org/libvirt/VirConnectAuth$Credential$CredentialType"), 
 			"mapToInt", 
 			"()I");
 	
Index: src/org/libvirt/VirConnectAuth.java
===================================================================
RCS file: /data/cvs/libvirt-java/src/org/libvirt/VirConnectAuth.java,v
retrieving revision 1.1
diff -u -p -r1.1 VirConnectAuth.java
--- src/org/libvirt/VirConnectAuth.java	30 Jun 2008 13:11:55 -0000	1.1
+++ src/org/libvirt/VirConnectAuth.java	9 Jul 2008 13:51:33 -0000
@@ -3,20 +3,137 @@ package org.libvirt;
 /**
  * We diverge from the C implementation
  * There is no explicit cbdata field, you should just add any extra data to the child class's instance.
- * 
+ *
  * @author stoty
  *
  */
 public abstract class VirConnectAuth {
 	/**
+	 * @author stoty
+	 *
+	 */
+	public class Credential {
+
+		public static enum CredentialType {
+			// This is off by one, but we don't care, because we can't convert java Enum to C enum in a sane way anyway
+			/**
+			 * Identity to act as
+			 */
+			VIR_CRED_USERNAME,
+			/**
+			 * Identify to authorize as
+			 */
+			VIR_CRED_AUTHNAME,
+			/**
+			 * RFC 1766 languages, comma separated
+			 */
+			VIR_CRED_LANGUAGE,
+			/**
+			 * client supplies a nonce
+			 */
+			VIR_CRED_CNONCE,
+			/**
+			 * Passphrase secret
+			 */
+			VIR_CRED_PASSPHRASE,
+			/**
+			 * Challenge response
+			 */
+			VIR_CRED_ECHOPROMPT,
+			/**
+			 * Challenge response
+			 */
+			VIR_CRED_NOECHOPROMPT,
+			/**
+			 * Authentication realm
+			 */
+			VIR_CRED_REALM,
+			/**
+			 * Externally managed credential More may be added - expect the unexpected
+			 */
+			VIR_CRED_EXTERNAL;
+
+			/**
+			 * Maps the java VirConnectCredentialType Enum to libvirt's integer constant
+			 *
+			 * @return The integer equivalent
+			 */
+			private int mapToInt(){
+				switch(this){
+					case VIR_CRED_USERNAME: return 1;
+					case VIR_CRED_AUTHNAME: return 2;
+					case VIR_CRED_LANGUAGE: return 3;
+					case VIR_CRED_CNONCE: return 4;
+					case VIR_CRED_PASSPHRASE: return 5;
+					case VIR_CRED_ECHOPROMPT: return 6;
+					case VIR_CRED_NOECHOPROMPT: return 7;
+					case VIR_CRED_REALM: return 8;
+					case VIR_CRED_EXTERNAL: return 9;
+				}
+				//We may never reach this point
+				assert(false);
+				return 0;
+			}
+		}
+
+		/**
+		 * One of virConnectCredentialType constants
+		 */
+		public VirConnectCredentialType type;
+		/**
+		 * Prompt to show to user
+		 */
+		public String prompt;
+		/**
+		 * Additional challenge to show
+		 */
+		public String challenge;
+		/**
+		 * Optional default result
+		 */
+		public String defresult;
+		/**
+		 * Result to be filled with user response (or defresult)
+		 */
+		public String result;
+
+		/**
+		 * Convenience constructor to be called from the JNI side
+		 *
+		 * @param type
+		 * @param prompt
+		 * @param challenge
+		 * @param defresult
+		 */
+		Credential(int type, String prompt, String challenge, String defresult){
+			switch(type){
+				case 1: this.type=VirConnectCredentialType.VIR_CRED_USERNAME; break;
+				case 2: this.type=VirConnectCredentialType.VIR_CRED_AUTHNAME; break;
+				case 3: this.type=VirConnectCredentialType.VIR_CRED_LANGUAGE; break;
+				case 4: this.type=VirConnectCredentialType.VIR_CRED_CNONCE; break;
+				case 5: this.type=VirConnectCredentialType.VIR_CRED_PASSPHRASE; break;
+				case 6: this.type=VirConnectCredentialType.VIR_CRED_ECHOPROMPT; break;
+				case 7: this.type=VirConnectCredentialType.VIR_CRED_NOECHOPROMPT; break;
+				case 8: this.type=VirConnectCredentialType.VIR_CRED_REALM; break;
+				case 9: this.type=VirConnectCredentialType.VIR_CRED_EXTERNAL; break;
+				default: assert(false);
+			}
+			this.prompt = prompt;
+			this.challenge = challenge;
+			this.defresult = defresult;
+		}
+
+
+	}
+	/**
 	 * List of supported VirConnectCredential.VirConnectCredentialType values
 	 */
-	public  VirConnectCredential.VirConnectCredentialType credType[];
-	
+	public  VirConnectCredential.CredentialType credType[];
+
 	/**
 	 * The callback function that fills the credentials in
 	 * @param cred the array of credentials passed by libvirt
 	 * @return 0 if the defresult field contains a vailde response, -1 otherwise
 	 */
-	public abstract int callback(VirConnectCredential[] cred);
+	public abstract int callback(Credential[] cred);
 }
Index: src/org/libvirt/VirConnectCredential.java
===================================================================
RCS file: src/org/libvirt/VirConnectCredential.java
diff -N src/org/libvirt/VirConnectCredential.java
--- src/org/libvirt/VirConnectCredential.java	30 Jun 2008 13:11:55 -0000	1.1
+++ /dev/null	1 Jan 1970 00:00:00 -0000
@@ -1,120 +0,0 @@
-package org.libvirt;
-
-/**
- * @author stoty
- *
- */
-public class VirConnectCredential {
-	
-	public static enum VirConnectCredentialType {
-		//This is off by one, but we don't care, because we can't convert java Enum to C enum in a sane way anyway 
-		/**
-		 * Identity to act as
-		 */
-		VIR_CRED_USERNAME,
-		/**
-		 * Identify to authorize as
-		 */
-		VIR_CRED_AUTHNAME,
-		/**
-		 * RFC 1766 languages, comma separated
-		 */
-		VIR_CRED_LANGUAGE,
-		/**
-		 * client supplies a nonce
-		 */
-		VIR_CRED_CNONCE,
-		/**
-		 * Passphrase secret
-		 */
-		VIR_CRED_PASSPHRASE,
-		/**
-		 * Challenge response
-		 */
-		VIR_CRED_ECHOPROMPT,
-		/**
-		 * Challenge response
-		 */
-		VIR_CRED_NOECHOPROMPT,
-		/**
-		 * Authentication realm
-		 */
-		VIR_CRED_REALM,
-		/**
-		 * Externally managed credential More may be added - expect the unexpected
-		 */
-		VIR_CRED_EXTERNAL;
-		
-		/**
-		 * Maps the java VirConnectCredentialType Enum to libvirt's integer constant
-		 * 
-		 * @return The integer equivalent
-		 */
-		private int mapToInt(){
-			switch(this){
-				case VIR_CRED_USERNAME: return 1;
-				case VIR_CRED_AUTHNAME: return 2;
-				case VIR_CRED_LANGUAGE: return 3;
-				case VIR_CRED_CNONCE: return 4;
-				case VIR_CRED_PASSPHRASE: return 5;
-				case VIR_CRED_ECHOPROMPT: return 6;
-				case VIR_CRED_NOECHOPROMPT: return 7;
-				case VIR_CRED_REALM: return 8;
-				case VIR_CRED_EXTERNAL: return 9;
-			}
-			//We may never reach this point
-			assert(false);
-			return 0;
-		}
-	}
-
-	/**
-	 * One of virConnectCredentialType constants
-	 */
-	public VirConnectCredentialType type;
-	/**
-	 * Prompt to show to user
-	 */
-	public String prompt;
-	/**
-	 * Additional challenge to show
-	 */
-	public String challenge;
-	/**
-	 * Optional default result
-	 */
-	public String defresult;
-	/**
-	 * Result to be filled with user response (or defresult)
-	 */
-	public String result;
-
-	/**
-	 * Convenience constructor to be called from the JNI side
-	 * 
-	 * @param type
-	 * @param prompt
-	 * @param challenge
-	 * @param defresult
-	 */
-	VirConnectCredential(int type, String prompt, String challenge, String defresult){
-		switch(type){
-			case 1: this.type=VirConnectCredentialType.VIR_CRED_USERNAME; break;
-			case 2: this.type=VirConnectCredentialType.VIR_CRED_AUTHNAME; break;
-			case 3: this.type=VirConnectCredentialType.VIR_CRED_LANGUAGE; break;
-			case 4: this.type=VirConnectCredentialType.VIR_CRED_CNONCE; break;
-			case 5: this.type=VirConnectCredentialType.VIR_CRED_PASSPHRASE; break;
-			case 6: this.type=VirConnectCredentialType.VIR_CRED_ECHOPROMPT; break;
-			case 7: this.type=VirConnectCredentialType.VIR_CRED_NOECHOPROMPT; break;
-			case 8: this.type=VirConnectCredentialType.VIR_CRED_REALM; break;
-			case 9: this.type=VirConnectCredentialType.VIR_CRED_EXTERNAL; break;
-			default: assert(false);
-		}
-		this.prompt = prompt;
-		this.challenge = challenge;
-		this.defresult = defresult;
-	}
-	
-
-}
-

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]