nacx requested changes on this pull request.
Thanks @Etienne-Carriere! The patch approach LGTM. I've added some suggestions
to clean up the code a bit. Thanks!
> @@ -62,22 +64,40 @@
* in {@code auth/v1.0/}.
*/
public final class SwiftAuthenticationModule extends
KeystoneAuthenticationModule {
- private static final String STORAGE_USER = "X-Storage-User";
- private static final String STORAGE_PASS = "X-Storage-Pass";
+ public static final String TEMP_AUTH_HEADER_USER =
"jclouds.swift.tempAuth.headerUser";
+ public static final String TEMP_AUTH_HEADER_PASS =
"jclouds.swift.tempAuth.headerPass";
+
+ @com.google.inject.Inject(optional = true)
+ @Named(TEMP_AUTH_HEADER_USER)
+ private static String identityHeaderNameUser = "X-Storage-User";
+
+ @com.google.inject.Inject(optional = true)
+ @Named(TEMP_AUTH_HEADER_PASS)
+ private static String identityHeaderNamePass = "X-Storage-Pass";
Move these variables to the `TempAuthBinder ` class.
> + */
+package org.jclouds.openstack.swift.v1.binders;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import org.jclouds.domain.Credentials;
+import org.jclouds.http.HttpRequest;
+import org.jclouds.openstack.swift.v1.config.SwiftAuthenticationModule;
+import org.jclouds.rest.Binder;
+
+/**
+ * Binder to the tempAuthAuthentication
+ *
+ */
+public final class TempAuthBinder implements Binder{
[style] Remove the extra spaces and add a space before the last `{`.
> private static final String STORAGE_URL = "X-Storage-Url";
@Override
protected void configure() {
super.configure();
bindHttpApi(binder(), AuthenticationApi.class);
bindHttpApi(binder(), TempAuthApi.class);
+ requestStaticInjection(SwiftAuthenticationModule.class);
Once the injected variables are not here, this should not be needed.
> }
@Override protected Map<String, Function<Credentials, Access>>
authenticationMethods(Injector i) {
return ImmutableMap.<String, Function<Credentials, Access>>builder()
.putAll(super.authenticationMethods(i))
.put("tempAuthCredentials",
i.getInstance(TempAuth.class)).build();
}
+
+ public static String getIdentityHeaderName(){
+ return identityHeaderNameUser;
+ }
+
+ public static String getIdentityHeaderPass(){
+ return identityHeaderNamePass;
+ }
These getters should no longer be needed.
> @@ -153,7 +173,7 @@ private URI getURI(String headerValue) {
public AdaptTempAuthResponseToAccess setContext(HttpRequest request) {
String host = request.getEndpoint().getHost();
this.host = host;
- this.username = request.getFirstHeaderOrNull(STORAGE_USER);
+ this.username =
request.getFirstHeaderOrNull(SwiftAuthenticationModule.getIdentityHeaderName());
You can inject the header variable in this class too, to avoid dealing with
static methods.
> @@ -45,13 +46,24 @@
public void testTempAuthRequest() throws Exception {
- tempAuthServer.enqueue(new MockResponse().setResponseCode(204)
+ Properties overrides = null;
+ // with default values
+ test(overrides);
+ overrides = new Properties();
+ overrides.setProperty(SwiftAuthenticationModule.TEMP_AUTH_HEADER_USER ,
"X-Auth-User");
+ overrides.setProperty(SwiftAuthenticationModule.TEMP_AUTH_HEADER_PASS ,
"X-Auth-Pass");
+ // with specific Header Name values
+ test(overrides);
Better move this to another test named `testTempAuthRequestWithCustomHeaders`
or similar, to properly qualify and isolate the test that verifies this change.
> @@ -71,8 +83,10 @@ public void testTempAuthRequest() throws Exception {
assertEquals(listContainers.getHeader("X-Auth-Token"), "token");
}
- private SwiftApi api(String authUrl) throws IOException {
- Properties overrides = new Properties();
+ private SwiftApi api(String authUrl, Properties overrides) throws
IOException {
+ if (overrides == null){
+ overrides = new Properties();
+ }
Instead of passing null, remove this check and call this method with a new
Properties object.
> @@ -60,8 +72,8 @@ public void testTempAuthRequest() throws Exception {
RecordedRequest auth = tempAuthServer.takeRequest();
assertEquals(auth.getMethod(), "GET");
- assertEquals(auth.getHeader("X-Storage-User"), "user");
- assertEquals(auth.getHeader("X-Storage-Pass"), "password");
+
assertEquals(auth.getHeader(SwiftAuthenticationModule.getIdentityHeaderName()),
"user");
+
assertEquals(auth.getHeader(SwiftAuthenticationModule.getIdentityHeaderPass()),
"password");
Once there are two tests, it's better to keep the expected header name
hardcoded, to properly verify that the expected configuration is applied.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1046#pullrequestreview-15037991
