Merge pull request #178 from katharsis-project/client_post

#177 Properly support patch/post semantic in katharsis-client

katharsis-client/src/main/java/io/katharsis/client/KatharsisClient.java

@@ -62,6 +62,8 @@ public class KatharsisClient {
 
 	private ExceptionMapperRegistry exceptionMapperRegistry;
 
+	private boolean pushAlways = true;
+
 	/**
 	 * @param serviceUrl service url
 	 * @param resourceSearchPackage search package
@@ -86,6 +88,23 @@ public KatharsisClient(String serviceUrl, String resourceSearchPackage) {
 		objectMapper.registerModule(jsonApiModule);
 	}
 
+	/**
+	 * Older KatharsisClient implementation only supported a save() operation that POSTs the resource to the server. No difference is made
+	 * between insert and update. The server-implementation still does not make a difference. 
+	 * 
+	 * By default the flag is enabled to maintain backward compatibility. But it is strongly adviced to turn id on. It will become
+	 * the default in one of the subsequent releases.
+	 * 
+	 * @param pushAlways
+	 */
+	public void setPushAlways(boolean pushAlways) {
+		this.pushAlways = pushAlways;
+	}
+
+	public boolean getPushAlways() {
+		return pushAlways;
+	}
+
 	private static String normalize(String serviceUrl) {
 		if (serviceUrl.endsWith("/")) {
 			return serviceUrl.substring(0, serviceUrl.length() - 1);
@@ -124,9 +143,9 @@ private void initExceptionMapperRegistry() {
 	}
 
 	@SuppressWarnings({ "rawtypes", "unchecked" })
-	private <T, ID extends Serializable> void allocateRepository(Class<T> resourceClass) {
+	private <T, I extends Serializable> void allocateRepository(Class<T> resourceClass) {
 		ResourceInformation resourceInformation = moduleRegistry.getResourceInformationBuilder().build(resourceClass);
-		final ResourceRepositoryStub<T, ID> repositoryStub = new ResourceRepositoryStubImpl<>(this, resourceClass,
+		final ResourceRepositoryStub<T, I> repositoryStub = new ResourceRepositoryStubImpl<>(this, resourceClass,
 				resourceInformation, urlBuilder);
 
 		// create interface for it!
@@ -137,7 +156,7 @@ public Object buildRepository() {
 				return repositoryStub;
 			}
 		};
-		ResourceEntry<T, ID> resourceEntry = new DirectResponseResourceEntry<>(repositoryInstanceBuilder);
+		ResourceEntry<T, I> resourceEntry = new DirectResponseResourceEntry<>(repositoryInstanceBuilder);
 		Set<ResourceField> relationshipFields = resourceInformation.getRelationshipFields();
 		List<ResponseRelationshipEntry<T, ?>> relationshipEntries = new ArrayList<>();
 		RegistryEntry<T> registryEntry = new RegistryEntry<>(resourceInformation, resourceEntry, relationshipEntries);

katharsis-client/src/main/java/io/katharsis/client/QuerySpecResourceRepositoryStub.java

@@ -39,4 +39,25 @@ public interface QuerySpecResourceRepositoryStub<T, I extends Serializable> exte
 	 * @return persisted resource
 	 */
 	public <S extends T> S save(S entity, QuerySpec querySpec);
+
+
+	/**
+	 * Creates the given entity without any of its relationships.
+	 *
+	 * @param entity resource to be saved
+	 * @param <S> resource type
+	 * @return persisted resource
+	 */
+	public <S extends T> S create(S entity);
+
+	/**
+	 * Creates the given entity. {@link QueryParams} allows to specify which
+	 * relationships should be saved as well (just the relation, not the related resource).
+	 *
+	 * @param entity resource to be saved
+	 * @param querySpec querySpec
+	 * @param <S> resource type
+	 * @return persisted resource
+	 */
+	public <S extends T> S create(S entity, QuerySpec querySpec);
 }

katharsis-client/src/main/java/io/katharsis/client/ResourceRepositoryStub.java

@@ -37,4 +37,25 @@ public interface ResourceRepositoryStub<T, ID extends Serializable> extends Reso
 	 * @return persisted resource
 	 */
 	public <S extends T> S save(S entity, QueryParams queryParams);
+
+
+	/**
+	 * Saves the given entity without any of its relationships.
+	 *
+	 * @param entity resource to be saved
+	 * @param <S> resource type
+	 * @return persisted resource
+	 */
+	public <S extends T> S create(S entity);
+
+	/**
+	 * Saves the given entity. {@link QueryParams} allows to specify which
+	 * relationships should be saved as well.
+	 *
+	 * @param entity resource to be saved
+	 * @param queryParams query params
+	 * @param <S> resource type
+	 * @return persisted resource
+	 */
+	public <S extends T> S create(S entity, QueryParams queryParams);
 }

katharsis-client/src/main/java/io/katharsis/client/internal/ResourceRepositoryStubImpl.java

@@ -21,15 +21,17 @@
 import io.katharsis.queryspec.internal.QuerySpecAdapter;
 import io.katharsis.request.path.JsonPath;
 import io.katharsis.request.path.ResourcePath;
+import io.katharsis.resource.field.ResourceField;
 import io.katharsis.resource.information.ResourceInformation;
 import io.katharsis.response.BaseResponseContext;
 import io.katharsis.response.CollectionResponseContext;
 import io.katharsis.response.JsonApiResponse;
 import io.katharsis.response.ResourceResponseContext;
 import io.katharsis.utils.JsonApiUrlBuilder;
+import io.katharsis.utils.PropertyUtils;
 
-public class ResourceRepositoryStubImpl<T, ID extends Serializable> extends AbstractStub
-		implements ResourceRepositoryStub<T, ID>, QuerySpecResourceRepositoryStub<T, ID> {
+public class ResourceRepositoryStubImpl<T, I extends Serializable> extends AbstractStub
+		implements ResourceRepositoryStub<T, I>, QuerySpecResourceRepositoryStub<T, I> {
 
 	private ResourceInformation resourceInformation;
 
@@ -42,7 +44,7 @@ public ResourceRepositoryStubImpl(KatharsisClient client, Class<T> resourceClass
 		this.resourceInformation = resourceInformation;
 	}
 
-	private BaseResponseContext executePost(HttpUrl requestUrl, T resource, QueryAdapter queryAdapter) {
+	private BaseResponseContext executeUpdate(HttpUrl requestUrl, T resource, QueryAdapter queryAdapter, boolean create) {
 		JsonApiResponse response = new JsonApiResponse();
 		response.setEntity(resource);
 
@@ -60,13 +62,18 @@ private BaseResponseContext executePost(HttpUrl requestUrl, T resource, QueryAda
 
 		Builder builder = new Request.Builder().url(requestUrl);
 
-		builder = builder.post(RequestBody.create(null, requestBodyValue));
+		if (create || katharsis.getPushAlways()) {
+			builder = builder.post(RequestBody.create(null, requestBodyValue));
+		}
+		else {
+			builder = builder.patch(RequestBody.create(null, requestBodyValue));
+		}
 
 		return execute(builder, true);
 	}
 
 	@Override
-	public T findOne(ID id, QueryParams queryParams) {
+	public T findOne(I id, QueryParams queryParams) {
 		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, id, queryParams));
 		return findOne(url);
 	}
@@ -78,31 +85,75 @@ public List<T> findAll(QueryParams queryParams) {
 	}
 
 	@Override
-	public List<T> findAll(Iterable<ID> ids, QueryParams queryParams) {
+	public List<T> findAll(Iterable<I> ids, QueryParams queryParams) {
 		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, ids, queryParams));
 		return findAll(url);
 	}
 
 	@Override
 	public <S extends T> S save(S entity) {
-		return save(entity, new QueryParams());
+		return save(entity, new QuerySpec(resourceClass));
 	}
 
-	@SuppressWarnings("unchecked")
 	@Override
 	public <S extends T> S save(S entity, QueryParams queryParams) {
-		// TODO proper post vs patch
-		Object id = null;
+		return modify(entity, queryParams, false);
+	}
 
-		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, id, (QuerySpec) null));
-		BaseResponseContext context = executePost(url, entity, new QueryParamsAdapter(queryParams));
+	@SuppressWarnings("unchecked")
+	private <S extends T> S modify(S entity, QueryParams queryParams, boolean create) {
+		String strId = getStringId(entity, create);
+		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, strId, (QuerySpec) null));
+		BaseResponseContext context = executeUpdate(url, entity, new QueryParamsAdapter(queryParams), create);
 		return (S) context.getResponse().getEntity();
 	}
 
 	@Override
-	public void delete(ID id) {
-		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, id, (QuerySpec) null));
+	public <S extends T> S save(S entity, QuerySpec querySpec) {
+		return modify(entity, querySpec, false);
+	}
 
+	@Override
+	public <S extends T> S create(S entity) {
+		return modify(entity, new QuerySpec(resourceClass), true);
+	}
+
+	@Override
+	public <S extends T> S create(S entity, QuerySpec querySpec) {
+		return modify(entity, querySpec, true);
+	}
+
+	@Override
+	public <S extends T> S create(S entity, QueryParams queryParams) {
+		return modify(entity, queryParams, true);
+	}
+
+	@SuppressWarnings("unchecked")
+	private <S extends T> S modify(S entity, QuerySpec querySpec, boolean create) {
+		String idString = getStringId(entity, create);
+		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, idString, (QuerySpec) null));
+		BaseResponseContext context = executeUpdate(url, entity, new QuerySpecAdapter(querySpec, katharsis.getRegistry()),
+				create);
+		return (S) context.getResponse().getEntity();
+	}
+
+	private <S extends T> String getStringId(S entity, boolean create) {
+		if (katharsis.getPushAlways()) {
+			return null;
+		}
+		if (create) {
+			return null;
+		}
+		else {
+			ResourceField idField = resourceInformation.getIdField();
+			Object objectId = PropertyUtils.getProperty(entity, idField.getUnderlyingName());
+			return resourceInformation.toIdString(objectId);
+		}
+	}
+
+	@Override
+	public void delete(I id) {
+		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, id, (QuerySpec) null));
 		executeDelete(url);
 	}
 
@@ -112,7 +163,7 @@ public Class<T> getResourceClass() {
 	}
 
 	@Override
-	public T findOne(ID id, QuerySpec querySpec) {
+	public T findOne(I id, QuerySpec querySpec) {
 		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, id, querySpec));
 		return findOne(url);
 	}
@@ -124,7 +175,7 @@ public ResourceList<T> findAll(QuerySpec querySpec) {
 	}
 
 	@Override
-	public ResourceList<T> findAll(Iterable<ID> ids, QuerySpec queryPaquerySpecrams) {
+	public ResourceList<T> findAll(Iterable<I> ids, QuerySpec queryPaquerySpecrams) {
 		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, ids, queryPaquerySpecrams));
 		return findAll(url);
 	}
@@ -146,15 +197,4 @@ private T findOne(HttpUrl url) {
 		return (T) responseContext.getResponse().getEntity();
 	}
 
-	@SuppressWarnings("unchecked")
-	@Override
-	public <S extends T> S save(S entity, QuerySpec querySpec) {
-		// TODO proper post vs patch
-		Object id = null;
-
-		HttpUrl url = HttpUrl.parse(urlBuilder.buildUrl(resourceClass, id, (QuerySpec) null));
-		BaseResponseContext context = executePost(url, entity, new QuerySpecAdapter(querySpec, katharsis.getRegistry()));
-		return (S) context.getResponse().getEntity();
-	}
-
 }

katharsis-client/src/test/java/io/katharsis/client/ExceptionTest.java

@@ -23,7 +23,7 @@ public void test() {
 		task.setId(10000L);
 		task.setName("test");
 		try {
-			taskRepo.save(task);
+			taskRepo.create(task);
 			Assert.fail();
 		}
 		catch (TestException e) {

katharsis-client/src/test/java/io/katharsis/client/ModuleTest.java

@@ -43,7 +43,7 @@ public void test() {
 		Task task = new Task();
 		task.setId(1L);
 		task.setName("task");
-		taskRepo.save(task);
+		taskRepo.create(task);
 
 		List<Task> tasks = taskRepo.findAll(new QuerySpec(Task.class));
 		Assert.assertEquals(1, tasks.size());

katharsis-client/src/test/java/io/katharsis/client/QueryParamsClientTest.java

@@ -55,7 +55,7 @@ public void testSaveAndFind() {
 		Task task = new Task();
 		task.setId(1L);
 		task.setName("test");
-		taskRepo.save(task);
+		taskRepo.create(task);
 
 		// check retrievable with findAll
 		List<Task> tasks = taskRepo.findAll(new QueryParams());
@@ -82,7 +82,7 @@ public void testGeneratedId() {
 		Task task = new Task();
 		task.setId(null);
 		task.setName("test");
-		Task savedTask = taskRepo.save(task);
+		Task savedTask = taskRepo.create(task);
 		Assert.assertNotNull(savedTask.getId());
 	}
 
@@ -91,7 +91,7 @@ public void testDelete() {
 		Task task = new Task();
 		task.setId(1L);
 		task.setName("test");
-		taskRepo.save(task);
+		taskRepo.create(task);
 
 		taskRepo.delete(1L);
 
@@ -129,12 +129,12 @@ public void testSetRelation() {
 		Project project = new Project();
 		project.setId(1L);
 		project.setName("project");
-		projectRepo.save(project);
+		projectRepo.create(project);
 
 		Task task = new Task();
 		task.setId(2L);
 		task.setName("test");
-		taskRepo.save(task);
+		taskRepo.create(task);
 
 		relRepo.setRelation(task, project.getId(), "project");
 
@@ -148,17 +148,17 @@ public void testAddSetRemoveRelations() {
 		Project project0 = new Project();
 		project0.setId(1L);
 		project0.setName("project0");
-		projectRepo.save(project0);
+		projectRepo.create(project0);
 
 		Project project1 = new Project();
 		project1.setId(2L);
 		project1.setName("project1");
-		projectRepo.save(project1);
+		projectRepo.create(project1);
 
 		Task task = new Task();
 		task.setId(3L);
 		task.setName("test");
-		taskRepo.save(task);
+		taskRepo.create(task);
 
 		relRepo.addRelations(task, Arrays.asList(project0.getId(), project1.getId()), "projects");
 		List<Project> relProjects = relRepo.findManyTargets(task.getId(), "projects", new QueryParams());