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

Changes in data model and implementation of image scaling #199

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

albertobucesGdtl
Copy link
Contributor

Changes in data model

  1. stm_tree
  • Added field tre_type
  • Added field tre_image_name
  1. stm_tree_nod
  • Added field tno_type
  • Added field tno_image
  • Added field tno_image_name
  • Added field tno_view_mode
  • Added field tno_taskid
  • Added field tno_filterable

Implementation of image scaling (ImageTransformer.java)


@JsonSetter("image")
public void imageDeserialize(String image) {
this.image = ImageTransformer.scaleImage(image, type);
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you ensure that the type field is set before the image field when the JSON is unmarshalled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the administration web, tree and tree node forms, the json sended when submit follows the defined structure in tree.model.ts and tree-node.model.ts respectively. So if type is defined before image in the model then the json will be equal.

Copy link
Contributor

@fjlopez fjlopez Jan 22, 2025

Choose a reason for hiding this comment

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

So, Do this behaviour require two calls, one for creating the TreeNode in the database with the type and a second to update the image field? Please confirm.

What happens when the user do the following path:

  • Creates the node with some type
  • Uploads the image
  • Later, decide that is better to assign a different role to the node and changes the type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The @JsonSetter tag will be changed to @PrePersist and @PreUpdate to avoid the need to have the type value before the image in the json.
By this way the method is executed after the java object is completely created and before being saved in the database.

If the user changes the node type or deletes the image in the form, the image field is saved as null.

111,application.type,APP Turística,false,false,app-turistica
112,treenode.folder.type,Cartografía,false,true,cartography
113,treenode.final.type,Cartografía,false,true,cartography
114,treenode.folder.type,Menú,false,false,menu
Copy link
Contributor

Choose a reason for hiding this comment

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

menu and list are mentioned in ImageTransformer, and hence they must have COD_SYSTEM set to true

}
result = scaledImage != null ? encodeImageToBase64(scaledImage, imageFormat.toLowerCase()) : image;
} catch (Exception e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the standard logger.

e.printStackTrace();
}
}
return result;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you confirm that the expected logic is this:

  • The client sends an url, and the url matches the size expected to the type -> keep the url
  • The client sends an url, and the url does not matches the size expected to the type -> retrieve, resize and encode as data
  • The client sends a base64 array which encodes an image that matches the size -> encode as data
  • The client sends a base64 array which encodes an image that does not match the size -> resize and encode as data

Any missing case?

Copy link
Contributor Author

@albertobucesGdtl albertobucesGdtl Jan 21, 2025

Choose a reason for hiding this comment

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

Those are the cases but I correct one of them:

  • The client sends a base64 array which encodes an image that matches the size -> keep the base64

When the base64 reaches the backend it is already in data protocol so it no needs any changes if it matches the size

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to confirm, the first time that an image is uploaded in the application, do it will be sent with or without the data: prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The image will be send with data: prefix

return reader.getFormatName(); // Retorna el formato, como "JPEG" o "PNG"
}
}
return DEFAULT_IMAGE_FORMAT; // No se pudo detectar
Copy link
Contributor

Choose a reason for hiding this comment

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

If the image cannot be detected, it could be anything (e.g. an excel file). This should be an error.

import javax.imageio.ImageReader;
import javax.imageio.stream.ImageInputStream;

public class ImageTransformer {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider to turn this into a @Converter / AttributeConverter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The converter does not have access to the type attribute and that attribute is necessary to select the appropriate size of the image

private static final int DEFAULT_ICON_HEIGHT = 25;
private static final int DEFAULT_ICON_WIDTH = 25;
private static final int DEFAULT_APP_IMAGE_HEIGHT = 125;
private static final int DEFAULT_APP_IMAGE_WIDTH = 125;
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider to use @Value annotations and allow to reconfigure externally

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternativelly, you can use a @ConfigurationProperties with the following structure:

  • defaultImageFormat: String (and this should be a list of valid image formats!)
  • defaultWidth: Int
  • defaultHeight: Int
  • sizes: List<ImageConstaintProperty>

where ImageConstaintProperty contains

  • property: String
  • width: Int
  • height: Int

In this way, the code becomes independent of the effective values and also get simpler and maintainable.

int width = DEFAULT_APP_IMAGE_WIDTH;
int height = DEFAULT_APP_IMAGE_HEIGHT;

if ("menu".equals(type)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Document that menu and list are values from code lists.

private static final int DEFAULT_ICON_HEIGHT = 25;
private static final int DEFAULT_ICON_WIDTH = 25;
private static final int DEFAULT_APP_IMAGE_HEIGHT = 125;
private static final int DEFAULT_APP_IMAGE_WIDTH = 125;
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternativelly, you can use a @ConfigurationProperties with the following structure:

  • defaultImageFormat: String (and this should be a list of valid image formats!)
  • defaultWidth: Int
  • defaultHeight: Int
  • sizes: List<ImageConstaintProperty>

where ImageConstaintProperty contains

  • property: String
  • width: Int
  • height: Int

In this way, the code becomes independent of the effective values and also get simpler and maintainable.

scaledImage = scaleImageFromBase64(image.split(",")[1], width, height);
imageFormat = image.substring(image.indexOf("/") + 1, image.indexOf(";"));
}
result = scaledImage != null ? encodeImageToBase64(scaledImage, imageFormat.toLowerCase()) : image;
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic that I see here is:

  • When the URL starts with http you will always try to retrieve it.
  • Then you will check size and then resize to a standardised size according type.
  • But only will be stored in the database if it has been uploaded by the administrator app or if its has been resized to fit the dimensions
  • If it does not matches the standardised size, you will keep the original URL, even if the image with its size breaks the UI.

Why not

  • you check that the uploaded content or the url is effectively an image (using ImageIO)
  • resize if required or possible, e.g. if the image is a svg it cannot be resized but it can be encoded in data
  • compute the image format and validate it against a list of supported formats
  • store the result always as data, in base64 if the image is binary or as is if the image is svg

Also, confirm with the developer of the mobile application:

  • formats supported
  • if there is no problem in send the images embebed in the JSON configuration in base64

@albertobucesGdtl
Copy link
Contributor Author

New changes:

  • Change tag @JsonSetter to @PrePersist and @PreUpdate in image attribute
  • ImageTransformer class is now a @service and includes image validations and errors control
  • Configurations for image scaling lie sizes and suported formats are obtains by ImageScalingProperties class (@ConfigurationProperties)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants