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

Exploration: Make Locale configurable for formatting of tags (not to be merged) #538

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
bab48fb
Initial work on configurable Locale, works for Longitude/Latitude and…
rdvdijk May 21, 2021
ed768a5
Restore Javadoc.
rdvdijk May 21, 2021
c594940
Fix missing import.
rdvdijk May 21, 2021
e93e7df
Add Nullable annotation to new Locale parameter.
rdvdijk May 21, 2021
bbf2cf9
Add Nullable annotation to new Locale parameter in interface.
rdvdijk May 21, 2021
2fe5f8f
Fix missing import.
rdvdijk May 21, 2021
60e67a6
Fix comment in test.
rdvdijk May 21, 2021
44aaa41
Introduce MetadataContext class to hold Locale to use for formatting …
rdvdijk May 28, 2021
63532a6
Replace Locale with MetadataContext class, which supplies the Locale.
rdvdijk May 28, 2021
9608080
Add missing Javadoc for new parameter.
rdvdijk May 28, 2021
88497e2
Apply Locale in Directory and TagDescriptor base classes.
rdvdijk May 28, 2021
aa71ef4
Add TODOs for two non-EXIF Locale use cases.
rdvdijk May 28, 2021
ffc5c93
Use context Locale in ExifDescriptorBase.
rdvdijk May 28, 2021
a0b3508
Fix locale passing to String.format.
rdvdijk Jun 18, 2021
06904a9
Provide Locale to all GpsDescriptor and related classes.
rdvdijk Jun 25, 2021
0e567cf
Pass Locale instead of full MetadataContext to Gps related classes.
rdvdijk Jun 25, 2021
e1bcae9
Provide Locale to all JpegReader and related classes.
rdvdijk Jun 25, 2021
4b51770
Provide MetadataContext to all JpegCommentReader and related classes.
rdvdijk Jun 25, 2021
0d1b49b
Initialize a default MetadataContext in JpegMetadataReader.
rdvdijk Jun 25, 2021
ffb1829
Provide MetadataContext to all JpegDhtReader and related classes.
rdvdijk Jun 25, 2021
dcae803
Provide MetadataContext to JpegDnlReader extract method.
rdvdijk Jun 25, 2021
88f8fa9
Provide MetadataContext to all JfifReader and related classes.
rdvdijk Jun 25, 2021
1b1ed9d
Provide MetadataContext to all IccReader and related classes.
rdvdijk Jun 25, 2021
15e6bb7
Provide MetadataContext to all JfxxReader and related classes.
rdvdijk Jun 25, 2021
a38da78
Provide MetadataContext to all XmpReader and related classes.
rdvdijk Jun 25, 2021
6d9c38d
Provide MetadataContext to all PhotoshopReader and related classes.
rdvdijk Jun 25, 2021
0e3655b
Make MetadataContext @NotNull instead of @Nullable in JpegSegmentMeta…
rdvdijk Jun 25, 2021
17f839c
Provide MetadataContext to all DuckyReader and related classes.
rdvdijk Jun 25, 2021
f8035fb
Provide MetadataContext to all IptcReader and related classes.
rdvdijk Jun 25, 2021
866081c
Remove unused imports.
rdvdijk Jun 25, 2021
3b6dbe7
Provide MetadataContext to all AdobeJpegReader and related classes.
rdvdijk Jun 25, 2021
dfea3db
Miscellaneous changes after self-review: remove or address TODOs, add…
rdvdijk Jun 25, 2021
a314058
Minor changes after self-review: inline variables, address or move TO…
rdvdijk Jun 25, 2021
37ff824
Merge remote-tracking branch 'drewnoakes/master' into configure-locale
rdvdijk Jun 25, 2021
02cf06f
Add TODOs to a few more default contexts. Remove unused constructor.
rdvdijk Jun 25, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 47 additions & 10 deletions Source/com/drew/imaging/jpeg/JpegMetadataReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.drew.lang.annotations.NotNull;
import com.drew.lang.annotations.Nullable;
import com.drew.metadata.Metadata;
import com.drew.metadata.MetadataContext;
import com.drew.metadata.adobe.AdobeJpegReader;
import com.drew.metadata.exif.ExifReader;
import com.drew.metadata.file.FileSystemMetadataReader;
Expand Down Expand Up @@ -71,45 +72,80 @@ public class JpegMetadataReader
);

@NotNull
public static Metadata readMetadata(@NotNull InputStream inputStream, @Nullable Iterable<JpegSegmentMetadataReader> readers) throws JpegProcessingException, IOException
public static Metadata readMetadata(@NotNull InputStream inputStream, @Nullable Iterable<JpegSegmentMetadataReader> readers, @NotNull MetadataContext context) throws JpegProcessingException, IOException
{
Metadata metadata = new Metadata();
process(metadata, inputStream, readers);
process(metadata, inputStream, readers, context);
return metadata;
}

@NotNull
public static Metadata readMetadata(@NotNull InputStream inputStream, @Nullable Iterable<JpegSegmentMetadataReader> readers) throws JpegProcessingException, IOException
{
return readMetadata(inputStream, readers, null);
}

@NotNull
public static Metadata readMetadata(@NotNull InputStream inputStream, @NotNull MetadataContext context) throws JpegProcessingException, IOException
{
return readMetadata(inputStream, null, context);
}

@NotNull
public static Metadata readMetadata(@NotNull InputStream inputStream) throws JpegProcessingException, IOException
{
return readMetadata(inputStream, null);
// TODO document this default context?
Copy link
Author

@rdvdijk rdvdijk Jun 25, 2021

Choose a reason for hiding this comment

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

As explained in the PR discussion: I've added this comment throughout to mark where new MetadataContext() is called, which is done mostly for backwards compatibility.

return readMetadata(inputStream, null, new MetadataContext());
}

@NotNull
public static Metadata readMetadata(@NotNull File file, @Nullable Iterable<JpegSegmentMetadataReader> readers) throws JpegProcessingException, IOException
public static Metadata readMetadata(@NotNull File file, @Nullable Iterable<JpegSegmentMetadataReader> readers, @NotNull MetadataContext context) throws JpegProcessingException, IOException
{
InputStream inputStream = new FileInputStream(file);
Metadata metadata;
try {
metadata = readMetadata(inputStream, readers);
metadata = readMetadata(inputStream, readers, context);
} finally {
inputStream.close();
}
// TODO pass locale?
new FileSystemMetadataReader().read(file, metadata);
return metadata;
}

@NotNull
public static Metadata readMetadata(@NotNull File file, @Nullable Iterable<JpegSegmentMetadataReader> readers) throws JpegProcessingException, IOException
{
// TODO document this default context?
return readMetadata(file, readers, new MetadataContext());
}

@NotNull
public static Metadata readMetadata(@NotNull File file, @NotNull MetadataContext context) throws JpegProcessingException, IOException
{
return readMetadata(file, null, context);
}

@NotNull
public static Metadata readMetadata(@NotNull File file) throws JpegProcessingException, IOException
{
return readMetadata(file, null);
// TODO document this default context?
return readMetadata(file, null, new MetadataContext());
}

public static void process(@NotNull Metadata metadata, @NotNull InputStream inputStream) throws JpegProcessingException, IOException
{
process(metadata, inputStream, null);
// TODO document this default context?
process(metadata, inputStream, null, new MetadataContext());
}

public static void process(@NotNull Metadata metadata, @NotNull InputStream inputStream, @Nullable Iterable<JpegSegmentMetadataReader> readers) throws JpegProcessingException, IOException
{
// TODO document this default context?
process(metadata, inputStream, readers, new MetadataContext());
}

public static void process(@NotNull Metadata metadata, @NotNull InputStream inputStream, @Nullable Iterable<JpegSegmentMetadataReader> readers, @NotNull MetadataContext context) throws JpegProcessingException, IOException
{
if (readers == null)
readers = ALL_READERS;
Expand All @@ -123,15 +159,16 @@ public static void process(@NotNull Metadata metadata, @NotNull InputStream inpu

JpegSegmentData segmentData = JpegSegmentReader.readSegments(new StreamReader(inputStream), segmentTypes);

processJpegSegmentData(metadata, readers, segmentData);
processJpegSegmentData(metadata, readers, segmentData, context);
}

public static void processJpegSegmentData(Metadata metadata, Iterable<JpegSegmentMetadataReader> readers, JpegSegmentData segmentData)
// TODO create method with original signature for backwards compatibility
public static void processJpegSegmentData(@NotNull Metadata metadata, @NotNull Iterable<JpegSegmentMetadataReader> readers, @NotNull JpegSegmentData segmentData, @NotNull MetadataContext context)
{
// Pass the appropriate byte arrays to each reader.
for (JpegSegmentMetadataReader reader : readers) {
for (JpegSegmentType segmentType : reader.getSegmentTypes()) {
reader.readJpegSegments(segmentData.getSegments(segmentType), metadata, segmentType);
reader.readJpegSegments(segmentData.getSegments(segmentType), metadata, segmentType, context);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion Source/com/drew/imaging/jpeg/JpegSegmentMetadataReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.drew.lang.annotations.NotNull;
import com.drew.metadata.Metadata;
import com.drew.metadata.MetadataContext;

/**
* Defines an object that extracts metadata from in JPEG segments.
Expand All @@ -21,6 +22,7 @@ public interface JpegSegmentMetadataReader
* encountered in the original file.
* @param metadata The {@link Metadata} object into which extracted values should be merged.
* @param segmentType The {@link JpegSegmentType} being read.
* @param context The {@link MetadataContext} to use for parsing and formatting.
*/
void readJpegSegments(@NotNull final Iterable<byte[]> segments, @NotNull final Metadata metadata, @NotNull final JpegSegmentType segmentType);
void readJpegSegments(@NotNull final Iterable<byte[]> segments, @NotNull final Metadata metadata, @NotNull final JpegSegmentType segmentType, @NotNull MetadataContext context);
}
27 changes: 24 additions & 3 deletions Source/com/drew/lang/GeoLocation.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import com.drew.lang.annotations.Nullable;

import java.text.DecimalFormat;
import java.text.DecimalFormatSymbols;
import java.util.Locale;

/**
* Represents a latitude and longitude pair, giving a position on earth in spherical coordinates.
Expand All @@ -37,17 +39,20 @@ public final class GeoLocation
{
private final double _latitude;
private final double _longitude;
private final Locale _locale;

/**
* Instantiates a new instance of {@link GeoLocation}.
*
* @param latitude the latitude, in degrees
* @param longitude the longitude, in degrees
* @param locale the locale to use
*/
public GeoLocation(double latitude, double longitude)
public GeoLocation(double latitude, double longitude, Locale locale)
{
_latitude = latitude;
_longitude = longitude;
_locale = locale;
}

/**
Expand Down Expand Up @@ -77,12 +82,26 @@ public boolean isZero()
/**
* Converts a decimal degree angle into its corresponding DMS (degrees-minutes-seconds) representation as a string,
* of format: {@code -1° 23' 4.56"}
*
* @deprecated Use {@link #decimalToDegreesMinutesSecondsString(double, Locale)} instead
*/
@NotNull
@Deprecated
public static String decimalToDegreesMinutesSecondsString(double decimal)
{
return decimalToDegreesMinutesSecondsString(decimal, null);
}

/**
* Converts a decimal degree angle into its corresponding DMS (degrees-minutes-seconds) representation as a string,
* of format: {@code -1° 23' 4.56"}
*/
@NotNull
public static String decimalToDegreesMinutesSecondsString(double decimal, Locale locale)
{
double[] dms = decimalToDegreesMinutesSeconds(decimal);
DecimalFormat format = new DecimalFormat("0.##");
DecimalFormatSymbols symbols = locale == null ? DecimalFormatSymbols.getInstance() : DecimalFormatSymbols.getInstance(locale);
DecimalFormat format = new DecimalFormat("0.##", symbols);
return String.format("%s\u00B0 %s' %s\"", format.format(dms[0]), format.format(dms[1]), format.format(dms[2]));
}

Expand Down Expand Up @@ -158,6 +177,8 @@ public String toString()
@NotNull
public String toDMSString()
{
return decimalToDegreesMinutesSecondsString(_latitude) + ", " + decimalToDegreesMinutesSecondsString(_longitude);
return decimalToDegreesMinutesSecondsString(_latitude, _locale)
+ ", "
+ decimalToDegreesMinutesSecondsString(_longitude, _locale);
}
}
24 changes: 19 additions & 5 deletions Source/com/drew/metadata/Directory.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.lang.reflect.Array;
import java.text.DateFormat;
import java.text.DecimalFormat;
import java.text.DecimalFormatSymbols;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.*;
Expand Down Expand Up @@ -192,6 +193,12 @@ public void setParent(@NotNull Directory parent)
_parent = parent;
}

protected Locale getLocale()
{
// TODO discuss: is it acceptable to use a default here?
return _descriptor != null ? _descriptor.getContext().locale() : new MetadataContext().locale();
}

// TAG SETTERS

/**
Expand Down Expand Up @@ -1024,15 +1031,15 @@ public String getString(int tagType)
string.append(Array.getLong(o, i));
}
} else if (componentType.getName().equals("float")) {
DecimalFormat format = new DecimalFormat(_floatFormatPattern);
DecimalFormat format = getFloatFormat();
for (int i = 0; i < arrayLength; i++) {
if (i != 0)
string.append(' ');
String s = format.format(Array.getFloat(o, i));
string.append(s.equals("-0") ? "0" : s);
}
} else if (componentType.getName().equals("double")) {
DecimalFormat format = new DecimalFormat(_floatFormatPattern);
DecimalFormat format = getFloatFormat();
for (int i = 0; i < arrayLength; i++) {
if (i != 0)
string.append(' ');
Expand All @@ -1053,10 +1060,10 @@ public String getString(int tagType)
}

if (o instanceof Double)
return new DecimalFormat(_floatFormatPattern).format(((Double)o).doubleValue());
return getFloatFormat().format(((Double) o).doubleValue());

if (o instanceof Float)
return new DecimalFormat(_floatFormatPattern).format(((Float)o).floatValue());
return getFloatFormat().format(((Float) o).floatValue());

// Note that several cameras leave trailing spaces (Olympus, Nikon) but this library is intended to show
// the actual data within the file. It is not inconceivable that whitespace may be significant here, so we
Expand All @@ -1065,6 +1072,12 @@ public String getString(int tagType)
return o.toString();
}

private DecimalFormat getFloatFormat()
{
DecimalFormatSymbols symbols = DecimalFormatSymbols.getInstance(getLocale());
return new DecimalFormat(_floatFormatPattern, symbols);
}

@Nullable
public String getString(int tagType, String charset)
{
Expand Down Expand Up @@ -1150,7 +1163,8 @@ public String getDescription(int tagType)
@Override
public String toString()
{
return String.format("%s Directory (%d %s)",
return String.format(getLocale(),
"%s Directory (%d %s)",
getName(),
_tagMap.size(),
_tagMap.size() == 1
Expand Down
49 changes: 49 additions & 0 deletions Source/com/drew/metadata/MetadataContext.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package com.drew.metadata;

import com.drew.lang.annotations.NotNull;

import java.util.Locale;

/**
* Context class for metadata. Contains settings used while extracting and formatting metadata.
*
* @author Roel van Dijk
*/
public class MetadataContext
{
/**
* Locale used to format metadata.
*/
private Locale _locale;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make MetadataContext immutable so that it can be passed around without worrying about threads. Thus, this, and any other fields in this class should be final in my opinion.

Copy link
Author

Choose a reason for hiding this comment

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

I fully agree with that! Since we only have a Locale for now, I could create a constructor on MetadataContext.

However, using the builder pattern would make this more future proof (albeit more verbose).


/**
* Initialize a context using the system default {@link Locale}.
*/
public MetadataContext()
{
_locale = Locale.getDefault();
}

/**
* Gets the configured {@link Locale}.
*
* @return the configured locale.
*/
public Locale locale()
{
return _locale;
}

/**
* Configure the {@link Locale} to use for extracting and formatting metadata.
*
* @param locale the locale to use
* @return this context
*/
public MetadataContext locale(@NotNull final Locale locale)
{
_locale = locale;
return this;
}

}
Loading