Deprecate XingApi's resource function (#245)

* Completely remove XingApi's resource function. Also removed factories and made all resource's constructors public. * Bump version, add changelog
xing · Jul 31, 2019 · 7f2314a · 7f2314a
1 parent bf61909
commit 7f2314a
Show file tree

Hide file tree

Showing 20 changed files with 45 additions and 288 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,4 +1,5 @@
 language: android
+dist: trusty
 
 android:
   components:
@@ -16,8 +17,6 @@ script:
   - ./gradlew clean check --stacktrace
 
 before_install:
-  - yes | sdkmanager "platforms;android-26"
-  - yes | sdkmanager "platforms;android-28"
   - export TZ=Europe/Berlin
 
 after_success:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,7 +1,20 @@
 Change Log
 ==========
 
-Version 3.0.7 *2019-01-31*
+## Version 3.1.0 *(2019-07-30)*
+
+### Breaking‼️:
+
+* [Remove XingApi's resource function](https://github.com/xing/xing-android-sdk/pull/245)
+  * See `GETTING_STARTED_ANDROID` for usage. The reasons for this change are:
+    * `XingApi` is a dependency of any `Resource`, therefore it doesn't make sense that the former provides or holds a
+    Resource in any way.
+    * `XingApi` also used to hold a cache of resources, this could potentially cause problems if an implementation would
+    contain any mutable state inside, as it would be propagated to further calls to the `resource` function.
+    * Finally, this function used to sometimes rely on reflection, which is not mentioned anywhere and leads to runtime
+    crashes when the target implementation Proguard's strips out the resources.
+
+## Version 3.0.7 *(2019-01-31)*
 
 * Update moshi to 1.8.0
 * Update minSdk to 21
@@ -10,18 +23,15 @@ Version 3.0.7 *2019-01-31*
 * Update build tools to 28.0.3
 * Update kotlin version to 1.3.20
 
-Version 3.0.6 *2019-01-10*
-----------------------------
+## Version 3.0.6 *(2019-01-10)*
 
 * [Allow setting a hostname verifier](https://github.com/xing/xing-android-sdk/pull/242)
 
-Version 2.0.1 *(2016-11-16)*
-----------------------------
+## Version 2.0.1 *(2016-11-16)*
 
  * Fix internal parsing mechanism to work with moshi 1.2.0 and higher.
  * Rename `XingApi#converter()` to `XingApi#moshi()`
 
-Version 2.0.0 *(2016-09-27)*
-----------------------------
+## Version 2.0.0 *(2016-09-27)*
 
  * Initial Release of the XING API Client Version 2.0.0 
diff --git a/GETTING_STARTED_ANDROID.md b/GETTING_STARTED_ANDROID.md
@@ -51,7 +51,7 @@ data. For example, let's assume that you need to access the users profile data:
 ```java
 
     // First you need to select the resource you want to access.
-    UserProfileResource resource = xingApi.resource(UserProfileResource.class);
+    UserProfileResource resource = new UserProfileResource(xingApi);
 
     // Get the spec for the user profile request.
     CallSpec<XingUser, HttpError> spec = resource.getOwnProfile();

diff --git a/api-client-android-sample/src/main/java/com/xing/api/sample/ui/ProfileActivity.java b/api-client-android-sample/src/main/java/com/xing/api/sample/ui/ProfileActivity.java
@@ -182,7 +182,7 @@ protected XingUser doInBackground(Activity... params) {
                   .accessToken(Prefs.getInstance(ProfileActivity.this).getOauthToken())
                   .accessSecret(Prefs.getInstance(ProfileActivity.this).getOauthSecret())
                   .build();
-            UserProfilesResource profilesResource = api.resource(UserProfilesResource.class);
+            UserProfilesResource profilesResource = new UserProfilesResource(api);
             Response<XingUser, HttpError> response = null;
             try {
                 response = profilesResource.getOwnProfile().execute();

diff --git a/api-client/src/main/java/com/xing/api/Resource.java b/api-client/src/main/java/com/xing/api/Resource.java
@@ -69,28 +69,4 @@ protected static Type first(Type searchFor, String... roots) {
         return Converter.first(searchFor, roots);
     }
 
-    /** Allows {@link Resource} creation via a non reflective api. */
-    public abstract static class Factory {
-        private final Class<? extends Resource> resourceCls;
-
-        public Factory(Class<? extends Resource> resourceCls) {
-            this.resourceCls = resourceCls;
-        }
-
-        /**
-         * Attempts to create a resource of {@code cls}. This returns the resource if one was created, or {@code null} if
-         * this factory isn't capable of creating such a resource.
-         */
-        final Resource create(Class<? extends Resource> cls, XingApi api) {
-            return cls == resourceCls ? create(api) : null;
-        }
-
-        /**
-         * Creates a {@linkplain Resource resource} instance of the class which <strong>this</strong> factory is attached
-         * to.
-         *
-         * <p>Implementations should <b>not</b> use {@link XingApi#resource(Class)} since it may result in an infinite loop.
-         */
-        public abstract Resource create(XingApi api);
-    }
 }
diff --git a/api-client/src/main/java/com/xing/api/XingApi.java b/api-client/src/main/java/com/xing/api/XingApi.java
@@ -18,7 +18,6 @@
 import com.serjltt.moshi.adapters.FallbackEnum;
 import com.serjltt.moshi.adapters.FallbackOnNull;
 import com.squareup.moshi.Moshi;
-import com.xing.api.Resource.Factory;
 import com.xing.api.internal.Experimental;
 import com.xing.api.internal.json.BirthDateJsonAdapter;
 import com.xing.api.internal.json.ContactPathJsonAdapter;
@@ -28,26 +27,9 @@
 import com.xing.api.internal.json.SafeCalendarJsonAdapter;
 import com.xing.api.internal.json.SafeEnumJsonAdapter;
 import com.xing.api.internal.json.TimeZoneJsonAdapter;
-import com.xing.api.resources.BookmarksResource;
-import com.xing.api.resources.ContactsResource;
-import com.xing.api.resources.GroupsResource;
-import com.xing.api.resources.JobsResource;
-import com.xing.api.resources.MessagesResource;
-import com.xing.api.resources.MiscellaneousResource;
-import com.xing.api.resources.ProfileEditingResource;
-import com.xing.api.resources.ProfileVisitsResource;
-import com.xing.api.resources.RecommendationsResource;
-import com.xing.api.resources.UserProfilesResource;
-
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Modifier;
-import java.util.ArrayList;
-import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
+
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.Executor;
 
 import okhttp3.HttpUrl;
@@ -58,16 +40,7 @@
 import static com.xing.api.Utils.stateNull;
 
 /**
- * Main access point for the XING API. Creates and holds instances of {@linkplain Response resources} that provide access
- * points and response/error handling for XING APIs.
- * <p>
- * Usage:
- * <pre>{@code
- *           // Will instantiate ContactsResource.
- *          ContactsResource resource = xingApi.resource(ContactsResource.class);
- *     }</pre>
- * <p>
- * Two states of XingApi are supported:
+ * Main access point for the XING API. Two states of XingApi are supported:
  * <dl>
  * <li>Logged in - Requires a user's access token and token secret (See {@linkplain XingApi.Builder}.)</li>
  * <li>Logged out - See {@linkplain Builder#loggedOut()}</li>
@@ -76,73 +49,21 @@
  * @since 2.0.0
  */
 public final class XingApi {
-    /** A list of built in factories for resources shipped with the library. */
-    private static final List<Resource.Factory> BUILT_IN_FACTORIES = new ArrayList<>();
-
-    static {
-        BUILT_IN_FACTORIES.add(BookmarksResource.FACTORY);
-        BUILT_IN_FACTORIES.add(ContactsResource.FACTORY);
-        BUILT_IN_FACTORIES.add(GroupsResource.FACTORY);
-        BUILT_IN_FACTORIES.add(JobsResource.FACTORY);
-        BUILT_IN_FACTORIES.add(MessagesResource.FACTORY);
-        BUILT_IN_FACTORIES.add(MiscellaneousResource.FACTORY);
-        BUILT_IN_FACTORIES.add(ProfileEditingResource.FACTORY);
-        BUILT_IN_FACTORIES.add(ProfileVisitsResource.FACTORY);
-        BUILT_IN_FACTORIES.add(RecommendationsResource.FACTORY);
-        BUILT_IN_FACTORIES.add(UserProfilesResource.FACTORY);
-    }
-
-    @SuppressWarnings("CollectionWithoutInitialCapacity")
-    private final Map<Class<? extends Resource>, Resource> resourcesCache = new LinkedHashMap<>();
     private final List<AuthErrorCallback> authErrorCallbacks = new LinkedList<>();
 
-    private final Set<Resource.Factory> resourceFactories;
     private final OkHttpClient client;
     private final HttpUrl apiEndpoint;
     private final Converter converter;
     private final CallbackAdapter callbackAdapter;
     private final Executor callbackExecutor;
 
     XingApi(OkHttpClient client, HttpUrl apiEndpoint, Converter converter, CallbackAdapter callbackAdapter,
-          Executor callbackExecutor, List<Resource.Factory> resourceFactories) {
+          Executor callbackExecutor) {
         this.client = client;
         this.apiEndpoint = apiEndpoint;
         this.converter = converter;
         this.callbackAdapter = callbackAdapter;
         this.callbackExecutor = callbackExecutor;
-
-        /* Initialise the factories and add custom ones. */
-        this.resourceFactories = new LinkedHashSet<>(resourceFactories);
-        this.resourceFactories.addAll(BUILT_IN_FACTORIES);
-    }
-
-    /** Return a {@link Resource} instance specified by the provided class. */
-    @SuppressWarnings("unchecked")
-    public <T extends Resource> T resource(Class<T> resource) {
-        Resource res = resourcesCache.get(checkNotNull(resource, "resource == null"));
-        if (res == null) {
-            // First try to create the resource via a factory
-            for (Resource.Factory factory : resourceFactories) {
-                res = factory.create(resource, this);
-
-                if (res != null) { // yay!
-                    resourcesCache.put(resource, res);
-                    return (T) res;
-                }
-            }
-
-            // Fallback to the reflection path
-            checkResourceClassDeclaration(resource);
-            try {
-                Constructor<? extends Resource> constructor = resource.getDeclaredConstructor(XingApi.class);
-                constructor.setAccessible(true);
-                res = constructor.newInstance(this);
-                resourcesCache.put(resource, res);
-            } catch (Exception ex) {
-                throw new IllegalArgumentException("Resource class malformed.", ex);
-            }
-        }
-        return (T) res;
     }
 
     /** Returns the api endpoint for <strong>this</strong> client instance. */
@@ -193,14 +114,6 @@ void notifyAuthError(Response<?, ResponseBody> rawResponse) {
         }
     }
 
-    /** Throws an exception if class was declared non-static or non-final. */
-    private static void checkResourceClassDeclaration(Class<? extends Resource> resource) {
-        int modifiers = resource.getModifiers();
-        if (resource.isLocalClass() || (resource.isMemberClass() && !Modifier.isStatic(modifiers))) {
-            throw new IllegalArgumentException("Resource class must be declared static.");
-        }
-    }
-
     /**
      * Build a new {@link XingApi}.
      * <p>
@@ -296,7 +209,6 @@ public static final class CustomStep extends BuildStep<CustomStep> {
      * @since 2.1.0
      */
     public static class BuildStep<T extends BuildStep> {
-        private final List<Factory> resourceFactory = new ArrayList<>();
         private OkHttpClient.Builder clientBuilder;
         private Moshi.Builder moshiBuilder;
         private Executor callbackExecutor;
@@ -306,12 +218,6 @@ public static class BuildStep<T extends BuildStep> {
             apiEndpoint = HttpUrl.parse("https://api.xing.com/");
         }
 
-        /** Add a resource factory for a specific {@linkplain Resource resource}. */
-        public final T addResourceFactory(Resource.Factory factory) {
-            resourceFactory.add(checkNotNull(factory, "factory == null"));
-            return self();
-        }
-
         /**
          * Change the api endpoint.
          * <p>
@@ -393,7 +299,7 @@ public final XingApi build() {
             CallbackAdapter adapter = Platform.get().callbackAdapter(callbackExecutor);
             Converter converter = new Converter(moshiBuilder.build());
 
-            return new XingApi(clientBuilder().build(), apiEndpoint, converter, adapter, callbackExecutor, resourceFactory);
+            return new XingApi(clientBuilder().build(), apiEndpoint, converter, adapter, callbackExecutor);
         }
     }
 }
diff --git a/api-client/src/main/java/com/xing/api/resources/BookmarksResource.java b/api-client/src/main/java/com/xing/api/resources/BookmarksResource.java
@@ -30,14 +30,8 @@
  * Provides methods which allow access to a {@linkplain com.xing.api.data.profile.XingUser user's} bookmarks.
  */
 public class BookmarksResource extends Resource {
-    public static final Resource.Factory FACTORY = new Resource.Factory(BookmarksResource.class) {
-        @Override public Resource create(XingApi api) {
-            return new BookmarksResource(api);
-        }
-    };
-
     /** Creates a resource instance. This should be the only constructor declared by child classes. */
-    BookmarksResource(XingApi api) {
+    public BookmarksResource(XingApi api) {
         super(api);
     }
 

diff --git a/api-client/src/main/java/com/xing/api/resources/ContactsResource.java b/api-client/src/main/java/com/xing/api/resources/ContactsResource.java
@@ -35,14 +35,8 @@
  * respond to contact requests.
  */
 public class ContactsResource extends Resource {
-    public static final Resource.Factory FACTORY = new Resource.Factory(ContactsResource.class) {
-        @Override public Resource create(XingApi api) {
-            return new ContactsResource(api);
-        }
-    };
-
     /** Creates a resource instance. This should be the only constructor declared by child classes. */
-    ContactsResource(XingApi api) {
+    public ContactsResource(XingApi api) {
         super(api);
     }
 

diff --git a/api-client/src/main/java/com/xing/api/resources/GroupsResource.java b/api-client/src/main/java/com/xing/api/resources/GroupsResource.java
@@ -34,14 +34,8 @@
  * Resource for all the possible groups calls.
  */
 public class GroupsResource extends Resource {
-    public static final Resource.Factory FACTORY = new Resource.Factory(GroupsResource.class) {
-        @Override public Resource create(XingApi api) {
-            return new GroupsResource(api);
-        }
-    };
-
     /** Creates a resource instance. This should be the only constructor declared by child classes. */
-    protected GroupsResource(XingApi api) {
+    public GroupsResource(XingApi api) {
         super(api);
     }
 

diff --git a/api-client/src/main/java/com/xing/api/resources/JobsResource.java b/api-client/src/main/java/com/xing/api/resources/JobsResource.java
@@ -31,14 +31,8 @@
  * criteria or recommended to a user by id.
  */
 public class JobsResource extends Resource {
-    public static final Resource.Factory FACTORY = new Resource.Factory(JobsResource.class) {
-        @Override public Resource create(XingApi api) {
-            return new JobsResource(api);
-        }
-    };
-
     /** Creates a resource instance. This should be the only constructor declared by child classes. */
-    JobsResource(XingApi api) {
+    public JobsResource(XingApi api) {
         super(api);
     }
 

diff --git a/api-client/src/main/java/com/xing/api/resources/MessagesResource.java b/api-client/src/main/java/com/xing/api/resources/MessagesResource.java
@@ -31,14 +31,8 @@
  * Provides methods which allow access to user's {@linkplain Conversation conversations}.
  */
 public class MessagesResource extends Resource {
-    public static final Resource.Factory FACTORY = new Resource.Factory(MessagesResource.class) {
-        @Override public Resource create(XingApi api) {
-            return new MessagesResource(api);
-        }
-    };
-
     /** Creates a resource instance. This should be the only constructor declared by child classes. */
-    MessagesResource(XingApi api) {
+    public MessagesResource(XingApi api) {
         super(api);
     }
 

diff --git a/api-client/src/main/java/com/xing/api/resources/MiscellaneousResource.java b/api-client/src/main/java/com/xing/api/resources/MiscellaneousResource.java
@@ -31,14 +31,8 @@
  * Represents the <a href="https://dev.xing.com/docs/resources#miscellaneous">Miscellaneous</a> resource.
  */
 public class MiscellaneousResource extends Resource {
-    public static final Resource.Factory FACTORY = new Resource.Factory(MiscellaneousResource.class) {
-        @Override public Resource create(XingApi api) {
-            return new MiscellaneousResource(api);
-        }
-    };
-
     /** Creates a resource instance. This should be the only constructor declared by child classes. */
-    MiscellaneousResource(XingApi api) {
+    public MiscellaneousResource(XingApi api) {
         super(api);
     }
 

diff --git a/api-client/src/main/java/com/xing/api/resources/ProfileEditingResource.java b/api-client/src/main/java/com/xing/api/resources/ProfileEditingResource.java
@@ -46,14 +46,8 @@
  * Provides methods which allow to edit the authorizing {@linkplain XingUser user's} profile information.
  */
 public class ProfileEditingResource extends Resource {
-    public static final Resource.Factory FACTORY = new Resource.Factory(ProfileEditingResource.class) {
-        @Override public Resource create(XingApi api) {
-            return new ProfileEditingResource(api);
-        }
-    };
-
     /** Creates a resource instance. This should be the only constructor declared by child classes. */
-    ProfileEditingResource(XingApi api) {
+    public ProfileEditingResource(XingApi api) {
         super(api);
     }