Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Add separate request/response views #32448

Merged
merged 14 commits into from
Apr 16, 2024
Next Next commit
chore: Add separate request/response views (#31781)
This PR gets finer control into what fields are allowed in
request-body-only, vs what's allowed in response-body-only. This leaves
the fields to separately controlled regarding what can go into the
database and what can't.

[Slack
thread](https://theappsmith.slack.com/archives/CPQNLFHTN/p1710125307810949).

✅ Server and Cypress **Sanity** tests pass on EE.

(cherry picked from commit 2617000)
  • Loading branch information
sharat87 committed Apr 5, 2024
commit 4b4d23eb7091eb8991dc2d21ccdcd9e98f7dda59
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
import com.appsmith.external.models.PluginType;
import com.appsmith.external.models.Policy;
import com.appsmith.external.models.Property;
import com.appsmith.external.views.ToResponse;
import com.appsmith.external.views.Views;
import com.fasterxml.jackson.annotation.JsonFormat;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonView;
import lombok.Getter;
import lombok.NoArgsConstructor;
Expand Down Expand Up @@ -88,9 +88,8 @@ public class ActionCE_DTO implements Identifiable, Executable {
ActionConfiguration actionConfiguration;

// this attribute carries error messages while processing the actionCollection
@JsonProperty(access = JsonProperty.Access.READ_ONLY)
@Transient
@JsonView(Views.Public.class)
@JsonView(ToResponse.class)
List<ErrorDTO> errorReports;

@JsonView(Views.Public.class)
Expand All @@ -106,23 +105,19 @@ public class ActionCE_DTO implements Identifiable, Executable {
@JsonView(Views.Public.class)
List<Property> dynamicBindingPathList;

@JsonProperty(access = JsonProperty.Access.READ_ONLY)
@JsonView(Views.Public.class)
@JsonView(ToResponse.class)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that we're using FromRequest to differentiate views that are restricted for client to edit. Could you please help me understand what the difference between ToResponse and Public is in this respect?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for asking. Good question. It likely will work fine if I keep it to ToResponse. I've done so many experiments I don't recall why I didn't revert this. Let me go check that and come back. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Removed ToResponse, and all Cypress tests pass on EE. Can you review/approve please? Thanks!

Boolean isValid;

@JsonProperty(access = JsonProperty.Access.READ_ONLY)
@JsonView(Views.Public.class)
@JsonView(ToResponse.class)
Set<String> invalids;

@Transient
@JsonProperty(access = JsonProperty.Access.READ_ONLY)
@JsonView(Views.Public.class)
@JsonView(ToResponse.class)
Set<String> messages = new HashSet<>();

// This is a list of keys that the client whose values the client needs to send during action execution.
// These are the Mustache keys that the server will replace before invoking the API
@JsonProperty(access = JsonProperty.Access.READ_ONLY)
@JsonView(Views.Public.class)
@JsonView(ToResponse.class)
Set<String> jsonPathKeys;

@JsonView(Views.Internal.class)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.appsmith.external.views;

/**
* Intended to annotate fields that can be set by HTTP request payloads, but should NOT be included
* in HTTP responses sent back to the client.
*/
public interface FromRequest extends Views.Public {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.appsmith.external.views;

/**
* Intended to mark entity/DTO fields that should be included as part of HTTP responses, but should
* be ignored as part of HTTP requests. For example, if a field is marked with this annotation, in
* a class used with {@code @RequestBody}, it's value will NOT be deserialized.
*/
public interface ToResponse extends Views.Public {}
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,19 @@
import com.appsmith.server.refactors.applications.RefactoringService;
import com.appsmith.server.services.LayoutActionService;
import com.appsmith.server.solutions.ActionExecutionSolution;
import io.micrometer.observation.ObservationRegistry;
import lombok.extern.slf4j.Slf4j;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping(Url.ACTION_URL)
@Slf4j
public class ActionController extends ActionControllerCE {

public ActionController(
LayoutActionService layoutActionService,
NewActionService newActionService,
RefactoringService refactoringService,
ActionExecutionSolution actionExecutionSolution,
ObservationRegistry observationRegistry) {
ActionExecutionSolution actionExecutionSolution) {

super(layoutActionService, newActionService, refactoringService, actionExecutionSolution, observationRegistry);
super(layoutActionService, newActionService, refactoringService, actionExecutionSolution);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.appsmith.external.models.ActionDTO;
import com.appsmith.external.models.ActionExecutionResult;
import com.appsmith.external.views.FromRequest;
import com.appsmith.external.views.ToResponse;
import com.appsmith.external.views.Views;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.constants.Url;
Expand All @@ -16,10 +18,9 @@
import com.appsmith.server.services.LayoutActionService;
import com.appsmith.server.solutions.ActionExecutionSolution;
import com.fasterxml.jackson.annotation.JsonView;
import io.micrometer.observation.ObservationRegistry;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.codec.multipart.Part;
Expand All @@ -42,47 +43,31 @@

@Slf4j
@RequestMapping(Url.ACTION_URL)
@RequiredArgsConstructor
public class ActionControllerCE {

private final LayoutActionService layoutActionService;
private final NewActionService newActionService;
private final RefactoringService refactoringService;
private final ActionExecutionSolution actionExecutionSolution;
private final ObservationRegistry observationRegistry;

@Autowired
public ActionControllerCE(
LayoutActionService layoutActionService,
NewActionService newActionService,
RefactoringService refactoringService,
ActionExecutionSolution actionExecutionSolution,
ObservationRegistry observationRegistry) {
this.layoutActionService = layoutActionService;
this.newActionService = newActionService;
this.refactoringService = refactoringService;
this.actionExecutionSolution = actionExecutionSolution;
this.observationRegistry = observationRegistry;
}

@JsonView(Views.Public.class)
@JsonView(ToResponse.class)
@PostMapping
@ResponseStatus(HttpStatus.CREATED)
public Mono<ResponseDTO<ActionDTO>> createAction(
@Valid @RequestBody ActionDTO resource,
@RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName,
@RequestHeader(name = "Origin", required = false) String originHeader,
ServerWebExchange exchange) {
@Valid @RequestBody @JsonView(FromRequest.class) ActionDTO resource,
@RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) {
log.debug("Going to create resource {}", resource.getClass().getName());
return layoutActionService
.createSingleActionWithBranch(resource, branchName)
.map(created -> new ResponseDTO<>(HttpStatus.CREATED.value(), created, null));
}

@JsonView(Views.Public.class)
@JsonView(ToResponse.class)
@PutMapping("/{defaultActionId}")
public Mono<ResponseDTO<ActionDTO>> updateAction(
@PathVariable String defaultActionId,
@Valid @RequestBody ActionDTO resource,
@Valid @RequestBody @JsonView(FromRequest.class) ActionDTO resource,
@RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) {
log.debug("Going to update resource with defaultActionId: {}, branch: {}", defaultActionId, branchName);
return layoutActionService
Expand Down Expand Up @@ -178,9 +163,6 @@ public Mono<ResponseDTO<ActionDTO>> deleteAction(
* <p>
* The controller function is primarily used with param applicationId by the client to fetch the actions in edit
* mode.
*
* @param params
* @return
*/
@JsonView(Views.Public.class)
@GetMapping("")
Expand Down
Loading